From 0ae14afc17d12a4301a8171c71fc98031dd6c913 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:34:17 -0700 Subject: [PATCH 01/16] ci: gate GitHub workflows with pinned actionlint (#2517) * fix(lint): catch malformed GitHub workflows before merge A self-broken ci.yml cannot report its own breakage, so parse every workflow in the local lint path that no-mistakes already runs. * fix(lint): pin actionlint instead of Ruby for workflow lint A self-broken ci.yml still has to fail in the local lint path, and the named tool for that gate is actionlint, not a new Ruby runtime. * no-mistakes(document): Clarify pinned workflow lint documentation --- .../firstmate-coding-guidelines/SKILL.md | 3 +- .github/workflows/ci.yml | 28 +- .no-mistakes.yaml | 5 +- CONTRIBUTING.md | 8 +- bin/fm-install-actionlint.sh | 36 +++ bin/fm-lint-workflows.sh | 137 ++++++++ bin/fm-lint.sh | 32 +- bin/fm-test-run.sh | 4 +- tests/fm-lint-workflows.test.sh | 298 ++++++++++++++++++ tests/fm-lint.test.sh | 2 + 10 files changed, 537 insertions(+), 16 deletions(-) create mode 100755 bin/fm-install-actionlint.sh create mode 100755 bin/fm-lint-workflows.sh create mode 100755 tests/fm-lint-workflows.test.sh diff --git a/.agents/skills/firstmate-coding-guidelines/SKILL.md b/.agents/skills/firstmate-coding-guidelines/SKILL.md index 2d43493299..a9e2154307 100644 --- a/.agents/skills/firstmate-coding-guidelines/SKILL.md +++ b/.agents/skills/firstmate-coding-guidelines/SKILL.md @@ -118,7 +118,8 @@ Run `bin/fm-doc-audience-check.sh`; it enforces classification, README setup rou - Plain dash `-`, never an em dash. - Never add an agent name as a commit co-author. - `bin/*.sh` and `bin/backends/*.sh` must pass `shellcheck`. -- Run `bin/fm-lint.sh` before treating a script change as done; it is the single owner of the lint definition (file set, config, and pinned shellcheck version) that CI and the no-mistakes pre-push gate both invoke, and it refuses to run under any other shellcheck version. +- Run `bin/fm-lint.sh` before treating a script change as done; it is the single owner of the lint definition (file set, config, pinned shellcheck version, and pinned actionlint workflow lint) that CI and the no-mistakes pre-push gate both invoke, and it refuses to run under any other version of either linter. +- When a task names a specific tool, implement the work with that tool, or explicitly flag the substitution and its new dependency footprint for review before shipping. - Colocate tests with the existing pattern in `tests/`, name them `.test.sh`, and extend an existing script rather than inventing a new runner. - Tests must exercise behavior through an executable or public interface and must never assert implementation-source bytes, including through parsers, regexes, snapshots, or indirect wrappers. - A maintainer-verification record under `docs/verification/` records active empirical facts, not assumptions or task chronology. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90c5d6e895..fcfc4cb2df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ permissions: jobs: lint: - name: Lint shell scripts + name: Lint runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 @@ -20,8 +20,15 @@ jobs: set -eu bin/fm-install-shellcheck.sh "$RUNNER_TEMP/bin" echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" - # Single owner of the lint definition (file set + config + version). Do not - # re-spell the shellcheck command here; keep CI and the pre-push gate on it. + - name: Install pinned actionlint + run: | + set -eu + bin/fm-install-actionlint.sh "$RUNNER_TEMP/bin" + echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" + # Single owner of the lint definition (shell file set, config, version, + # and GitHub workflow lint). Do not re-spell the checks here; keep CI + # and the pre-push gate on this script so a self-broken ci.yml still + # fails locally before merge. - run: bin/fm-lint.sh # Deterministic proof that portable parallel shards + portable serial + Herdr @@ -52,6 +59,11 @@ jobs: set -eu bin/fm-install-shellcheck.sh "$RUNNER_TEMP/bin" echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" + - name: Install pinned actionlint + run: | + set -eu + bin/fm-install-actionlint.sh "$RUNNER_TEMP/bin" + echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" - name: Install tasks-axi run: | set -eu @@ -84,6 +96,11 @@ jobs: set -eu bin/fm-install-shellcheck.sh "$RUNNER_TEMP/bin" echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" + - name: Install pinned actionlint + run: | + set -eu + bin/fm-install-actionlint.sh "$RUNNER_TEMP/bin" + echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" - name: Install tasks-axi run: | set -eu @@ -130,6 +147,11 @@ jobs: set -eu bin/fm-install-shellcheck.sh "$RUNNER_TEMP/bin" echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" + - name: Install pinned actionlint + run: | + set -eu + bin/fm-install-actionlint.sh "$RUNNER_TEMP/bin" + echo "$RUNNER_TEMP/bin" >> "$GITHUB_PATH" - name: Require tmux for e2e tests run: | set -eu diff --git a/.no-mistakes.yaml b/.no-mistakes.yaml index 62bb9e7284..219c39b794 100644 --- a/.no-mistakes.yaml +++ b/.no-mistakes.yaml @@ -24,9 +24,10 @@ document: # Pin lint to the same owner CI runs instead of leaving it to no-mistakes' # default handling, which does not invoke the repository's canonical lint gate. -# `bin/fm-lint.sh` owns the complete lint definition and +# `bin/fm-lint.sh` owns the complete lint definition, including GitHub workflow +# lint via pinned actionlint in `bin/fm-lint-workflows.sh`, and # `.github/workflows/ci.yml` invokes it directly, with parity asserted by -# `tests/fm-lint.test.sh`. +# `tests/fm-lint.test.sh` and `tests/fm-lint-workflows.test.sh`. # # Do not set commands.test to a complete tests/*.test.sh walk. Local no-mistakes # Test is intent-targeted validation of whether the change meets its brief; diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 65305797be..c9431bc59e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -45,8 +45,10 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star - Helper scripts in `bin/` are plain bash. Each starts with a usage header comment; keep it accurate when you change behavior. Test scripts and helpers in `tests/` are plain bash too. - `bin/fm-lint.sh` must pass: it is the single owner of the lint definition (the shellcheck file set, config, and pinned shellcheck version), and both CI and the no-mistakes pre-push gate run it, so local and CI can never diverge. - It pins one exact shellcheck version and refuses to run under any other; print it with `bin/fm-lint.sh --required-version` and install that build locally. + `bin/fm-lint.sh` must pass: it is the single owner of the lint definition (the shellcheck file set, config, pinned shellcheck version, and pinned actionlint workflow lint), and both CI and the no-mistakes pre-push gate run it, so local and CI can never diverge. + A malformed `.github/workflows/*.yml`, including a self-broken `ci.yml`, fails that local lint path before merge because a broken workflow cannot report its own breakage. + It pins one exact shellcheck version and one exact actionlint version and refuses to run under any other. + Print the shellcheck pin with `bin/fm-lint.sh --required-version` and the actionlint pin with `bin/fm-lint-workflows.sh --required-version`, then install those builds locally. - Harness-adapter ownership spans detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, semantic busy sources and trust gates in `bin/fm-busy-lib.sh`, delivery-only rendered guards in `bin/fm-composer-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in `.agents/skills/harness-adapters/SKILL.md`; the `firstmate-coding-guidelines` skill owns the validation policy for checks that depend on those harnesses. - Changes to runtime session backends (`bin/fm-backend.sh`, `bin/backends/`, and the scripts that dispatch through them) keep current setup and limits in the relevant backend guide and active empirical evidence in [`docs/verification/runtime-backends.md`](docs/verification/runtime-backends.md). - [`docs/documentation-audiences.md`](docs/documentation-audiences.md) and its machine-consumed inventory own prose classification; run `bin/fm-doc-audience-check.sh` after documentation changes. @@ -72,7 +74,7 @@ Check and test the toolbelt before pushing: ```sh while IFS= read -r script; do /bin/bash -n "$script" || exit; done < <(bin/fm-lint.sh --list-files) # syntax-check the shell surface fm-lint.sh will cover (changed files locally, full set in CI/on main) -bin/fm-lint.sh # lint that same surface; the single owner CI and the no-mistakes gate both run, full set in CI +bin/fm-lint.sh # lint that shell surface plus GitHub workflows via pinned actionlint; the single owner CI and the no-mistakes gate both run bin/fm-test-run.sh tests/.test.sh # one script (primary local focus path, timed) bin/fm-test-run.sh --family pure-contract-unit # ordinary family-scoped local path (serial, timed) bin/fm-test-run.sh --changed # conservative changed-file-informed set (never silent full suite) diff --git a/bin/fm-install-actionlint.sh b/bin/fm-install-actionlint.sh new file mode 100755 index 0000000000..d313d50639 --- /dev/null +++ b/bin/fm-install-actionlint.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# fm-install-actionlint.sh - install CI's pinned, verified actionlint build. +# +# Usage: +# fm-install-actionlint.sh +set -eu + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +VERSION="$("$ROOT/bin/fm-lint-workflows.sh" --required-version)" +SHA256=8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 +ARCHIVE="actionlint_${VERSION}_linux_amd64.tar.gz" +URL="https://github.com/rhysd/actionlint/releases/download/v${VERSION}/${ARCHIVE}" +DESTINATION=${1:?usage: fm-install-actionlint.sh } +TMP=$(mktemp -d "${RUNNER_TEMP:-${TMPDIR:-/tmp}}/fm-actionlint.XXXXXX") +trap 'rm -rf "$TMP"' EXIT + +DOWNLOAD_ATTEMPTS=6 +download_attempt=1 +while ! curl -fsSL "$URL" -o "$TMP/$ARCHIVE"; do + [ "$download_attempt" -lt "$DOWNLOAD_ATTEMPTS" ] || { + printf 'fm-install-actionlint.sh: download failed after %s attempts\n' "$DOWNLOAD_ATTEMPTS" >&2 + exit 1 + } + printf 'fm-install-actionlint.sh: download attempt %s failed; retrying\n' "$download_attempt" >&2 + sleep $((1 << (download_attempt - 1))) + download_attempt=$((download_attempt + 1)) +done +ACTUAL_SHA256=$(sha256sum "$TMP/$ARCHIVE" | awk '{print $1}') +[ "$ACTUAL_SHA256" = "$SHA256" ] || { + printf 'fm-install-actionlint.sh: checksum mismatch for %s\n' "$ARCHIVE" >&2 + exit 1 +} +tar -xzf "$TMP/$ARCHIVE" -C "$TMP" +mkdir -p "$DESTINATION" +install -m 0755 "$TMP/actionlint" "$DESTINATION/actionlint" +"$DESTINATION/actionlint" -version diff --git a/bin/fm-lint-workflows.sh b/bin/fm-lint-workflows.sh new file mode 100755 index 0000000000..0e2d7b07e1 --- /dev/null +++ b/bin/fm-lint-workflows.sh @@ -0,0 +1,137 @@ +#!/usr/bin/env bash +# fm-lint-workflows.sh - owner of firstmate's GitHub workflow lint. +# +# Runs pinned actionlint on every .github/workflows/*.{yml,yaml} so a malformed +# workflow, including a self-broken ci.yml, fails in the local and no-mistakes +# lint lane before merge. A broken ci.yml cannot report its own breakage, so +# this check must not live only as a step inside that workflow. bin/fm-lint.sh +# invokes this owner on its default (no explicit-path) path, which CI and +# commands.lint both use. +# +# Usage: +# fm-lint-workflows.sh lint workflows under this repo +# fm-lint-workflows.sh --root lint workflows under +# fm-lint-workflows.sh ... lint explicit workflow files +# fm-lint-workflows.sh --required-version +# fm-lint-workflows.sh --help +set -eu + +REQUIRED_ACTIONLINT=1.7.12 +SELF_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SELF="$SELF_DIR/fm-lint-workflows.sh" +ROOT="$(cd "$SELF_DIR/.." && pwd)" + +if [ "${1:-}" = "--required-version" ]; then + printf '%s\n' "$REQUIRED_ACTIONLINT" + exit 0 +fi + +fm_lint_workflows_usage() { + sed -n '2,16{s/^# \{0,1\}//;p;}' "$SELF" +} + +EXPLICIT_ROOT= +while [ "$#" -gt 0 ]; do + case "$1" in + --root) + [ "$#" -ge 2 ] || { + printf 'fm-lint-workflows.sh: --root requires a directory.\n' >&2 + exit 2 + } + EXPLICIT_ROOT=$2 + shift 2 + ;; + --root=*) + EXPLICIT_ROOT=${1#*=} + shift + ;; + --help|-h) + fm_lint_workflows_usage + exit 0 + ;; + --) + shift + break + ;; + -*) + printf 'fm-lint-workflows.sh: unknown option: %s\n' "$1" >&2 + exit 2 + ;; + *) + break + ;; + esac +done + +if [ -n "$EXPLICIT_ROOT" ]; then + [ -d "$EXPLICIT_ROOT" ] || { + printf 'fm-lint-workflows.sh: --root is not a directory: %s\n' "$EXPLICIT_ROOT" >&2 + exit 2 + } + ROOT="$(cd "$EXPLICIT_ROOT" && pwd)" +fi + +collect_workflow_files() { + local dir=$1 + [ -d "$dir" ] || return 0 + find "$dir" -maxdepth 1 \( -name '*.yml' -o -name '*.yaml' \) -type f \ + | LC_ALL=C sort +} + +FILES=() +if [ "$#" -gt 0 ]; then + for path in "$@"; do + case "$path" in + *.yml|*.yaml) ;; + *) + printf 'fm-lint-workflows.sh: not a workflow YAML file: %s\n' "$path" >&2 + exit 2 + ;; + esac + [ -f "$path" ] || { + printf 'fm-lint-workflows.sh: workflow file not found: %s\n' "$path" >&2 + exit 2 + } + FILES+=("$path") + done +else + workflow_dir="$ROOT/.github/workflows" + while IFS= read -r path; do + [ -n "$path" ] || continue + FILES+=("$path") + done < <(collect_workflow_files "$workflow_dir") + if [ "${#FILES[@]}" -eq 0 ]; then + printf 'fm-lint-workflows.sh: no GitHub workflow files found under %s\n' \ + "$workflow_dir" >&2 + exit 1 + fi +fi + +if ! command -v actionlint >/dev/null 2>&1; then + printf 'fm-lint-workflows.sh: actionlint not found; install actionlint %s for CI parity.\n' \ + "$REQUIRED_ACTIONLINT" >&2 + exit 127 +fi +ACTIONLINT_BIN=$(command -v actionlint) +resolved=$("$ACTIONLINT_BIN" -version | awk 'NR==1 {print; exit}') +printf 'fm-lint-workflows.sh: actionlint %s (pinned %s)\n' "$resolved" "$REQUIRED_ACTIONLINT" >&2 +if [ "$resolved" != "$REQUIRED_ACTIONLINT" ]; then + printf 'fm-lint-workflows.sh: actionlint %s required for CI parity, found %s. Install %s.\n' \ + "$REQUIRED_ACTIONLINT" "$resolved" "$REQUIRED_ACTIONLINT" >&2 + exit 1 +fi + +# fm-lint.sh owns ShellCheck of the canonical shell set. Disable actionlint's +# extra shell and Python subprocess linters so this gate is the named workflow +# linter, not a second shell lint of `run:` blocks. +set +e +"$ACTIONLINT_BIN" -no-color -shellcheck= -pyflakes= -- "${FILES[@]}" +rc=$? +set -e + +if [ "$rc" -ne 0 ]; then + exit "$rc" +fi + +printf 'fm-lint-workflows.sh: %s workflow files valid\n' "${#FILES[@]}" +exit 0 diff --git a/bin/fm-lint.sh b/bin/fm-lint.sh index 5c3bebcb21..d848a2ac83 100755 --- a/bin/fm-lint.sh +++ b/bin/fm-lint.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# fm-lint.sh - the single owner of firstmate's shell-lint definition. +# fm-lint.sh - the single owner of firstmate's lint definition. # # Runs its file set with ShellCheck's default severity, extended analysis, # ambient configuration disabled, and one exact ShellCheck version. CI and @@ -7,6 +7,9 @@ # version, bounded execution, and diagnostics ordering cannot drift. # Tests stop source analysis at imported production modules because every # production shell is already a canonical, source-aware root of this same run. +# The default (no explicit-path) path also runs bin/fm-lint-workflows.sh so a +# malformed GitHub workflow, including a self-broken ci.yml, fails locally +# before merge instead of only failing to run as CI. # # With no explicit paths, the file set depends on context: # - In CI (GITHUB_ACTIONS=true or CI=true), on the main branch, or when no @@ -16,10 +19,10 @@ # - Otherwise (an ordinary local branch with a real merge-base) it lints # only the canonical-set files changed since that merge-base, including # uncommitted local edits, via plain local `git diff` (no network, no -# `gh`). A branch with zero matching changed files exits 0 and prints a -# "no changed lint targets" note instead of running ShellCheck. +# `gh`). A branch with zero matching changed files skips ShellCheck and +# prints a "no changed lint targets" note, then still validates workflows. # Explicit paths always bypass this file-set selection and lint exactly the -# given paths, matching the same config. +# given paths, matching the same config, without the workflow YAML check. # # Canonical lint defaults to two bounded workers over two stable logical shards. # Each shard writes separate diagnostics, and the parent replays those outputs in @@ -97,7 +100,14 @@ if [ "${1:-}" = "--required-version" ]; then fi fm_lint_usage() { - sed -n '2,39{s/^# \{0,1\}//;p;}' "$SELF" + sed -n '2,42{s/^# \{0,1\}//;p;}' "$SELF" +} + +# Default no-args lint also validates GitHub workflows. Explicit paths stay a +# ShellCheck-only override so callers can target one shell root. +fm_lint_run_workflows() { + [ "$EXPLICIT_PATHS" -eq 0 ] || return 0 + "$SELF_DIR/fm-lint-workflows.sh" } JOBS=${FM_LINT_JOBS:-2} @@ -180,7 +190,9 @@ fm_lint_is_canonical_root() { } CHANGED_MODE=0 +EXPLICIT_PATHS=0 if [ "$#" -gt 0 ]; then + EXPLICIT_PATHS=1 ROOTS=("$@") else full_lint=1 @@ -238,7 +250,9 @@ fi if [ "$CHANGED_MODE" -eq 1 ] && [ "$ROOT_COUNT" -eq 0 ]; then printf 'fm-lint.sh: no changed lint targets\n' - exit 0 + overall_rc=0 + fm_lint_run_workflows || overall_rc=$? + exit "$overall_rc" fi if [ -n "$TELEMETRY" ]; then @@ -538,4 +552,10 @@ EOF fi fi +if [ "$overall_rc" -eq 0 ]; then + fm_lint_run_workflows || overall_rc=$? +else + fm_lint_run_workflows || true +fi + exit "$overall_rc" diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 4ca26c865e..24ced99088 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -140,6 +140,7 @@ family_for_basename() { fm-crew-state.test.sh|fm-decision-hold-lifecycle.test.sh|\ fm-documentation-audiences.test.sh|fm-ensure-agents-md.test.sh|fm-grok-harness.test.sh|\ fm-kimi-harness.test.sh|fm-muse-harness.test.sh|fm-herdr-lab.test.sh|fm-lint.test.sh|\ + fm-lint-workflows.test.sh|\ fm-operational-input.test.sh|fm-pi-primary-types.test.sh|\ fm-send-popup-settle.test.sh|fm-send-settle.test.sh|\ fm-subagent-pretool-check.test.sh|\ @@ -960,7 +961,8 @@ families_for_changed_path() { # lane's contract coverage re-runs. printf '%s\n' real-herdr-gated ;; - bin/fm-lint.sh|bin/fm-install-shellcheck.sh|\ + bin/fm-lint.sh|bin/fm-lint-workflows.sh|bin/fm-install-shellcheck.sh|\ + bin/fm-install-actionlint.sh|\ bin/fm-brief.sh|bin/fm-ensure-agents-md.sh|bin/fm-crew-state.sh|\ bin/fm-decision-hold.sh|bin/fm-supervision*|bin/fm-transition-lib.sh|\ bin/fm-tmux-lib.sh|bin/fm-marker-lib.sh|bin/fm-operational-input.sh|bin/fm-tasks-axi-lib.sh|\ diff --git a/tests/fm-lint-workflows.test.sh b/tests/fm-lint-workflows.test.sh new file mode 100755 index 0000000000..c7e918d678 --- /dev/null +++ b/tests/fm-lint-workflows.test.sh @@ -0,0 +1,298 @@ +#!/usr/bin/env bash +# GitHub workflow lint gate owned by bin/fm-lint-workflows.sh. +# +# A malformed .github/workflows/*.yml, including a self-broken ci.yml, must fail +# in the local/no-mistakes lint path before merge. Regression origin: #2512 put +# a column-0 heredoc body inside a `run: |` block in ci.yml; there was no +# workflow YAML lint, and the broken workflow could not report its own breakage. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +LINT_WF="$ROOT/bin/fm-lint-workflows.sh" +LINT="$ROOT/bin/fm-lint.sh" +INSTALLER="$ROOT/bin/fm-install-actionlint.sh" +REQUIRED=$("$LINT_WF" --required-version) + +write_valid_workflow() { + local path=$1 + cat > "$path" <<'YAML' +name: CI +on: push +jobs: + x: + runs-on: ubuntu-latest + steps: + - run: | + set -eu + echo ok +YAML +} + +# #2512-class breakage: a heredoc body at column 0 inside a `run: |` block. +write_col0_heredoc_workflow() { + local path=$1 + cat > "$path" <<'YAML' +name: CI +on: push +jobs: + x: + runs-on: ubuntu-latest + steps: + - name: Compatibility pointers must stay intact + run: | + set -eu + cmp -s CLAUDE.md - <<'EOF' || exit 1 + +@AGENTS.md +EOF + echo ok +YAML +} + +test_current_workflows_pass() { + local out rc + rc=0 + out=$("$LINT_WF" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "current workflows must parse, got $rc"$'\n'"$out" + assert_contains "$out" "workflow files valid" \ + "current-workflow lint did not report a valid count" + pass "current .github/workflows YAML files parse" +} + +test_col0_heredoc_fails_with_clear_error() { + local tmp out rc + tmp=$(fm_test_tmproot fm-lint-wf-col0) + mkdir -p "$tmp/.github/workflows" + write_col0_heredoc_workflow "$tmp/.github/workflows/ci.yml" + rc=0 + out=$("$LINT_WF" --root "$tmp" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "column-0 heredoc workflow unexpectedly passed"$'\n'"$out" + assert_contains "$out" "could not parse as YAML" \ + "column-0 heredoc failure did not report actionlint's YAML syntax error" + assert_contains "$out" "ci.yml" \ + "column-0 heredoc failure did not name the workflow file" + pass "column-0 heredoc workflow fails validation with a clear error" +} + +test_valid_fixture_passes() { + local tmp out rc + tmp=$(fm_test_tmproot fm-lint-wf-ok) + mkdir -p "$tmp/.github/workflows" + write_valid_workflow "$tmp/.github/workflows/ci.yml" + rc=0 + out=$("$LINT_WF" --root "$tmp" 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "valid fixture workflow failed"$'\n'"$out" + assert_contains "$out" "1 workflow files valid" \ + "valid fixture did not report one valid file" + pass "valid fixture workflow passes" +} + +test_empty_workflows_dir_fails() { + local tmp out rc + tmp=$(fm_test_tmproot fm-lint-wf-empty) + mkdir -p "$tmp/.github/workflows" + rc=0 + out=$("$LINT_WF" --root "$tmp" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "empty workflows dir unexpectedly passed"$'\n'"$out" + assert_contains "$out" "no GitHub workflow files found" \ + "empty workflows dir did not report the missing files" + pass "empty workflows directory fails closed" +} + +test_explicit_broken_path_fails() { + local tmp broken out rc + tmp=$(fm_test_tmproot fm-lint-wf-path) + broken="$tmp/broken.yml" + write_col0_heredoc_workflow "$broken" + rc=0 + out=$("$LINT_WF" "$broken" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "explicit broken path unexpectedly passed"$'\n'"$out" + assert_contains "$out" "could not parse as YAML" \ + "explicit broken path did not report actionlint's YAML syntax error" + pass "explicit malformed workflow path fails validation" +} + +test_non_mapping_root_fails() { + local tmp out rc + tmp=$(fm_test_tmproot fm-lint-wf-scalar) + mkdir -p "$tmp/.github/workflows" + printf 'just-a-string\n' > "$tmp/.github/workflows/ci.yml" + rc=0 + out=$("$LINT_WF" --root "$tmp" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "scalar YAML root unexpectedly passed"$'\n'"$out" + assert_contains "$out" "mapping node is expected" \ + "scalar YAML root did not report actionlint's mapping-node error" + pass "non-mapping workflow YAML root fails" +} + +test_missing_actionlint_fails_closed() { + local tmp fakebin out rc tool + tmp=$(fm_test_tmproot fm-lint-wf-noactionlint) + fakebin=$(fm_fakebin "$tmp") + mkdir -p "$tmp/.github/workflows" + write_valid_workflow "$tmp/.github/workflows/ci.yml" + for tool in bash dirname find sort awk; do + ln -s "$(command -v "$tool")" "$fakebin/$tool" + done + rc=0 + out=$(PATH="$fakebin" "$LINT_WF" --root "$tmp" 2>&1) || rc=$? + [ "$rc" -eq 127 ] || fail "missing actionlint expected exit 127, got $rc"$'\n'"$out" + assert_contains "$out" "actionlint not found" \ + "missing actionlint did not name the required linter" + assert_contains "$out" "$REQUIRED" \ + "missing actionlint did not name the pinned version" + pass "missing actionlint fails closed" +} + +test_pins_an_explicit_version() { + [ -n "$REQUIRED" ] || fail "fm-lint-workflows.sh --required-version printed nothing" + assert_contains "$REQUIRED" "1.7.12" "fm-lint-workflows.sh must pin actionlint 1.7.12" + pass "fm-lint-workflows.sh pins an explicit actionlint version ($REQUIRED)" +} + +test_rejects_wrong_actionlint_version() { + local tmp fakebin out rc + tmp=$(fm_test_tmproot fm-lint-wf-ver) + fakebin=$(fm_fakebin "$tmp") + mkdir -p "$tmp/.github/workflows" + write_valid_workflow "$tmp/.github/workflows/ci.yml" + cat > "$fakebin/actionlint" <<'SH' +#!/usr/bin/env bash +if [ "$1" = "-version" ]; then + printf '0.0.0\n' + exit 0 +fi +exit 0 +SH + chmod +x "$fakebin/actionlint" + rc=0 + out=$(PATH="$fakebin:$PATH" "$LINT_WF" --root "$tmp" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "fm-lint-workflows.sh accepted an actionlint version other than the pin"$'\n'"$out" + assert_contains "$out" "$REQUIRED" "fm-lint-workflows.sh did not name the required version on mismatch" + assert_contains "$out" "0.0.0" "fm-lint-workflows.sh did not report the resolved (wrong) version" + pass "fm-lint-workflows.sh refuses to lint under a non-pinned actionlint version" +} + +test_installer_retries_transient_download_failure() { + local tmp fakebin destination out + tmp=$(fm_test_tmproot fm-actionlint-download) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + + cat > "$fakebin/curl" <<'SH' +#!/usr/bin/env bash +count=0 +[ ! -f "$CURL_COUNT" ] || count=$(cat "$CURL_COUNT") +count=$((count + 1)) +printf '%s\n' "$count" > "$CURL_COUNT" +[ "$count" -gt 3 ] || exit 22 +while [ "$#" -gt 0 ]; do + if [ "$1" = "-o" ]; then + : > "$2" + exit 0 + fi + shift +done +exit 2 +SH + cat > "$fakebin/sha256sum" <<'SH' +#!/usr/bin/env bash +printf '8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 %s\n' "$1" +SH + cat > "$fakebin/tar" <<'SH' +#!/usr/bin/env bash +while [ "$#" -gt 0 ]; do + if [ "$1" = "-C" ]; then + cat > "$2/actionlint" <<'EOF' +#!/usr/bin/env bash +printf '1.7.12\n' +EOF + chmod +x "$2/actionlint" + exit 0 + fi + shift +done +exit 2 +SH + cat > "$fakebin/sleep" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fakebin/curl" "$fakebin/sha256sum" "$fakebin/tar" "$fakebin/sleep" + + out=$(CURL_COUNT="$tmp/curl-count" PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ + || fail "installer did not recover from a transient download failure"$'\n'"$out" + [ "$(cat "$tmp/curl-count")" -eq 4 ] || fail "installer did not recover after three failed downloads" + assert_contains "$out" "download attempt 3 failed; retrying" "installer did not disclose its third retry" + [ -x "$destination/actionlint" ] || fail "installer did not install actionlint after retrying" + pass "actionlint installer retries a transient download failure" +} + +# Prove the no-mistakes/local owner (bin/fm-lint.sh with no paths) catches a +# self-broken ci.yml. Copy the lint scripts into a fake repo so the default +# workflow root is the fixture, not this worktree. +test_fm_lint_default_path_catches_broken_ci_yml() { + local tmp fakebin log diff_file out rc + tmp=$(fm_test_tmproot fm-lint-wf-default) + mkdir -p "$tmp/bin" "$tmp/.github/workflows" + cp "$LINT" "$tmp/bin/fm-lint.sh" + cp "$LINT_WF" "$tmp/bin/fm-lint-workflows.sh" + chmod +x "$tmp/bin/fm-lint.sh" "$tmp/bin/fm-lint-workflows.sh" + write_col0_heredoc_workflow "$tmp/.github/workflows/ci.yml" + + fakebin=$(fm_fakebin "$tmp") + log="$tmp/shellcheck.log" + cat > "$fakebin/git" <<'SH' +#!/usr/bin/env bash +case "$*" in + "rev-parse --is-inside-work-tree") printf 'true\n'; exit 0 ;; + "rev-parse --abbrev-ref HEAD") printf 'feature\n'; exit 0 ;; + "rev-parse --verify -q origin/main") exit 0 ;; + "merge-base "*) printf 'fakebase123\n'; exit 0 ;; + "diff --name-only --diff-filter=ACMR -z fakebase123 --") + [ -n "${FM_TEST_GIT_DIFF_FILE:-}" ] && cat "${FM_TEST_GIT_DIFF_FILE}" + exit 0 + ;; + *) exit 0 ;; +esac +SH + chmod +x "$fakebin/git" + : > "$log" + cat > "$fakebin/shellcheck" <> "$log" +exit 0 +SH + chmod +x "$fakebin/shellcheck" + diff_file="$tmp/diff.nul" + : > "$diff_file" + + rc=0 + out=$(PATH="$fakebin:$PATH" GITHUB_ACTIONS='' CI='' FM_LINT_JOBS=1 \ + FM_TEST_GIT_DIFF_FILE="$diff_file" "$tmp/bin/fm-lint.sh" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "fm-lint.sh default path missed a broken ci.yml"$'\n'"$out" + assert_contains "$out" "could not parse as YAML" \ + "fm-lint.sh default path did not surface the workflow YAML error" + assert_contains "$out" "ci.yml" \ + "fm-lint.sh default path did not name the broken workflow" + pass "fm-lint.sh default path catches a self-broken ci.yml" +} + +test_pins_an_explicit_version +test_current_workflows_pass +test_col0_heredoc_fails_with_clear_error +test_valid_fixture_passes +test_empty_workflows_dir_fails +test_explicit_broken_path_fails +test_non_mapping_root_fails +test_missing_actionlint_fails_closed +test_rejects_wrong_actionlint_version +test_installer_retries_transient_download_failure +test_fm_lint_default_path_catches_broken_ci_yml diff --git a/tests/fm-lint.test.sh b/tests/fm-lint.test.sh index 46e5d3178d..99eba0f8ab 100755 --- a/tests/fm-lint.test.sh +++ b/tests/fm-lint.test.sh @@ -208,6 +208,8 @@ test_zero_changed_files_exits_clean() { [ "$rc" -eq 0 ] || fail "zero changed lint targets must exit 0, got $rc"$'\n'"$out" assert_contains "$out" "ShellCheck 0.11.0" "zero-changed run did not print the ShellCheck version line" assert_contains "$out" "no changed lint targets" "zero-changed run did not note the empty target set" + assert_contains "$out" "workflow files valid" \ + "zero-changed run skipped workflow YAML validation" pass "fm-lint.sh exits 0 with a note when the local branch has no changed lint targets" } From ac55d39a5bb355308f608b9ff2d033592232826c Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:42:37 -0700 Subject: [PATCH 02/16] fix: install pinned lint tools across supported platforms (#2546) * fix: install pinned shellcheck and actionlint on macOS and linux arm64 The installers were hardcoded to linux amd64 and sha256sum, so a Mac dev could not satisfy the refuse-on-mismatch lint gate. Select the official per-platform archive and checksum, and fall back to shasum -a 256. * no-mistakes(document): Document cross-platform pinned lint installers --- CONTRIBUTING.md | 3 +- bin/fm-install-actionlint.sh | 58 +++++- bin/fm-install-shellcheck.sh | 58 +++++- tests/fm-lint-workflows.test.sh | 314 ++++++++++++++++++++++++++---- tests/fm-lint.test.sh | 330 +++++++++++++++++++++++++++----- 5 files changed, 666 insertions(+), 97 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c9431bc59e..97442a450f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -48,7 +48,8 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star `bin/fm-lint.sh` must pass: it is the single owner of the lint definition (the shellcheck file set, config, pinned shellcheck version, and pinned actionlint workflow lint), and both CI and the no-mistakes pre-push gate run it, so local and CI can never diverge. A malformed `.github/workflows/*.yml`, including a self-broken `ci.yml`, fails that local lint path before merge because a broken workflow cannot report its own breakage. It pins one exact shellcheck version and one exact actionlint version and refuses to run under any other. - Print the shellcheck pin with `bin/fm-lint.sh --required-version` and the actionlint pin with `bin/fm-lint-workflows.sh --required-version`, then install those builds locally. + Print the shellcheck pin with `bin/fm-lint.sh --required-version` and the actionlint pin with `bin/fm-lint-workflows.sh --required-version`. + Use `bin/fm-install-shellcheck.sh` and `bin/fm-install-actionlint.sh` to install those exact builds locally; each installer's header owns its destination usage and supported platforms. - Harness-adapter ownership spans detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, semantic busy sources and trust gates in `bin/fm-busy-lib.sh`, delivery-only rendered guards in `bin/fm-composer-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in `.agents/skills/harness-adapters/SKILL.md`; the `firstmate-coding-guidelines` skill owns the validation policy for checks that depend on those harnesses. - Changes to runtime session backends (`bin/fm-backend.sh`, `bin/backends/`, and the scripts that dispatch through them) keep current setup and limits in the relevant backend guide and active empirical evidence in [`docs/verification/runtime-backends.md`](docs/verification/runtime-backends.md). - [`docs/documentation-audiences.md`](docs/documentation-audiences.md) and its machine-consumed inventory own prose classification; run `bin/fm-doc-audience-check.sh` after documentation changes. diff --git a/bin/fm-install-actionlint.sh b/bin/fm-install-actionlint.sh index d313d50639..77eaf7e269 100755 --- a/bin/fm-install-actionlint.sh +++ b/bin/fm-install-actionlint.sh @@ -1,16 +1,56 @@ #!/usr/bin/env bash # fm-install-actionlint.sh - install CI's pinned, verified actionlint build. # +# Downloads the official GitHub release archive for the host OS/arch, verifies +# its per-archive SHA-256 pin, and installs the binary into the destination +# directory. Supported platforms: linux amd64/x86_64, linux arm64/aarch64, +# darwin amd64/x86_64, darwin arm64/aarch64. Pins come from the official +# actionlint release checksums file. Verification uses sha256sum when present, +# otherwise shasum -a 256. An unsupported OS/arch or a missing pin fails +# without downloading. +# # Usage: # fm-install-actionlint.sh set -eu ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" VERSION="$("$ROOT/bin/fm-lint-workflows.sh" --required-version)" -SHA256=8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 -ARCHIVE="actionlint_${VERSION}_linux_amd64.tar.gz" -URL="https://github.com/rhysd/actionlint/releases/download/v${VERSION}/${ARCHIVE}" + +die() { + printf 'fm-install-actionlint.sh: %s\n' "$*" >&2 + exit 1 +} + DESTINATION=${1:?usage: fm-install-actionlint.sh } + +os=$(uname -s) +arch=$(uname -m) +# SHA-256 pins are from actionlint_1.7.12_checksums.txt on the official +# v1.7.12 release (https://github.com/rhysd/actionlint/releases/tag/v1.7.12). +case "${os}-${arch}" in + Linux-x86_64|Linux-amd64) + ARCHIVE="actionlint_${VERSION}_linux_amd64.tar.gz" + SHA256=8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 + ;; + Linux-aarch64|Linux-arm64) + ARCHIVE="actionlint_${VERSION}_linux_arm64.tar.gz" + SHA256=325e971b6ba9bfa504672e29be93c24981eeb1c07576d730e9f7c8805afff0c6 + ;; + Darwin-x86_64|Darwin-amd64) + ARCHIVE="actionlint_${VERSION}_darwin_amd64.tar.gz" + SHA256=5b44c3bc2255115c9b69e30efc0fecdf498fdb63c5d58e17084fd5f16324c644 + ;; + Darwin-arm64|Darwin-aarch64) + ARCHIVE="actionlint_${VERSION}_darwin_arm64.tar.gz" + SHA256=aba9ced2dee8d27fecca3dc7feb1a7f9a52caefa1eb46f3271ea66b6e0e6953f + ;; + *) + die "unsupported platform ${os}-${arch}; need linux or darwin on amd64/x86_64 or arm64/aarch64" + ;; +esac +[ -n "$SHA256" ] || die "no pinned checksum for ${os}-${arch}" + +URL="https://github.com/rhysd/actionlint/releases/download/v${VERSION}/${ARCHIVE}" TMP=$(mktemp -d "${RUNNER_TEMP:-${TMPDIR:-/tmp}}/fm-actionlint.XXXXXX") trap 'rm -rf "$TMP"' EXIT @@ -25,9 +65,17 @@ while ! curl -fsSL "$URL" -o "$TMP/$ARCHIVE"; do sleep $((1 << (download_attempt - 1))) download_attempt=$((download_attempt + 1)) done -ACTUAL_SHA256=$(sha256sum "$TMP/$ARCHIVE" | awk '{print $1}') + +if command -v sha256sum >/dev/null 2>&1; then + ACTUAL_SHA256=$(sha256sum "$TMP/$ARCHIVE" | awk '{print $1}') +elif command -v shasum >/dev/null 2>&1; then + ACTUAL_SHA256=$(shasum -a 256 "$TMP/$ARCHIVE" | awk '{print $1}') +else + die "need sha256sum or shasum to verify the actionlint archive" +fi [ "$ACTUAL_SHA256" = "$SHA256" ] || { - printf 'fm-install-actionlint.sh: checksum mismatch for %s\n' "$ARCHIVE" >&2 + printf 'fm-install-actionlint.sh: checksum mismatch for %s (expected %s, got %s)\n' \ + "$ARCHIVE" "$SHA256" "$ACTUAL_SHA256" >&2 exit 1 } tar -xzf "$TMP/$ARCHIVE" -C "$TMP" diff --git a/bin/fm-install-shellcheck.sh b/bin/fm-install-shellcheck.sh index b947b3faab..694211e4d2 100755 --- a/bin/fm-install-shellcheck.sh +++ b/bin/fm-install-shellcheck.sh @@ -1,16 +1,56 @@ #!/usr/bin/env bash # fm-install-shellcheck.sh - install CI's pinned, verified ShellCheck build. # +# Downloads the official GitHub release archive for the host OS/arch, verifies +# its per-archive SHA-256 pin, and installs the binary into the destination +# directory. Supported platforms: linux amd64/x86_64, linux arm64/aarch64, +# darwin amd64/x86_64, darwin arm64/aarch64. Pins come from the official +# ShellCheck release asset digests. Verification uses sha256sum when present, +# otherwise shasum -a 256. An unsupported OS/arch or a missing pin fails +# without downloading. +# # Usage: # fm-install-shellcheck.sh set -eu ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" VERSION="$("$ROOT/bin/fm-lint.sh" --required-version)" -SHA256=8c3be12b05d5c177a04c29e3c78ce89ac86f1595681cab149b65b97c4e227198 -ARCHIVE="shellcheck-v${VERSION}.linux.x86_64.tar.xz" -URL="https://github.com/koalaman/shellcheck/releases/download/v${VERSION}/${ARCHIVE}" + +die() { + printf 'fm-install-shellcheck.sh: %s\n' "$*" >&2 + exit 1 +} + DESTINATION=${1:?usage: fm-install-shellcheck.sh } + +os=$(uname -s) +arch=$(uname -m) +# SHA-256 pins are the GitHub release asset digests for shellcheck v0.11.0 +# .tar.xz archives (https://github.com/koalaman/shellcheck/releases/tag/v0.11.0). +case "${os}-${arch}" in + Linux-x86_64|Linux-amd64) + ARCHIVE="shellcheck-v${VERSION}.linux.x86_64.tar.xz" + SHA256=8c3be12b05d5c177a04c29e3c78ce89ac86f1595681cab149b65b97c4e227198 + ;; + Linux-aarch64|Linux-arm64) + ARCHIVE="shellcheck-v${VERSION}.linux.aarch64.tar.xz" + SHA256=12b331c1d2db6b9eb13cfca64306b1b157a86eb69db83023e261eaa7e7c14588 + ;; + Darwin-x86_64|Darwin-amd64) + ARCHIVE="shellcheck-v${VERSION}.darwin.x86_64.tar.xz" + SHA256=3c89db4edcab7cf1c27bff178882e0f6f27f7afdf54e859fa041fca10febe4c6 + ;; + Darwin-arm64|Darwin-aarch64) + ARCHIVE="shellcheck-v${VERSION}.darwin.aarch64.tar.xz" + SHA256=56affdd8de5527894dca6dc3d7e0a99a873b0f004d7aabc30ae407d3f48b0a79 + ;; + *) + die "unsupported platform ${os}-${arch}; need linux or darwin on amd64/x86_64 or arm64/aarch64" + ;; +esac +[ -n "$SHA256" ] || die "no pinned checksum for ${os}-${arch}" + +URL="https://github.com/koalaman/shellcheck/releases/download/v${VERSION}/${ARCHIVE}" TMP=$(mktemp -d "${RUNNER_TEMP:-${TMPDIR:-/tmp}}/fm-shellcheck.XXXXXX") trap 'rm -rf "$TMP"' EXIT @@ -25,9 +65,17 @@ while ! curl -fsSL "$URL" -o "$TMP/$ARCHIVE"; do sleep $((1 << (download_attempt - 1))) download_attempt=$((download_attempt + 1)) done -ACTUAL_SHA256=$(sha256sum "$TMP/$ARCHIVE" | awk '{print $1}') + +if command -v sha256sum >/dev/null 2>&1; then + ACTUAL_SHA256=$(sha256sum "$TMP/$ARCHIVE" | awk '{print $1}') +elif command -v shasum >/dev/null 2>&1; then + ACTUAL_SHA256=$(shasum -a 256 "$TMP/$ARCHIVE" | awk '{print $1}') +else + die "need sha256sum or shasum to verify the ShellCheck archive" +fi [ "$ACTUAL_SHA256" = "$SHA256" ] || { - printf 'fm-install-shellcheck.sh: checksum mismatch for %s\n' "$ARCHIVE" >&2 + printf 'fm-install-shellcheck.sh: checksum mismatch for %s (expected %s, got %s)\n' \ + "$ARCHIVE" "$SHA256" "$ACTUAL_SHA256" >&2 exit 1 } tar -xJf "$TMP/$ARCHIVE" -C "$TMP" diff --git a/tests/fm-lint-workflows.test.sh b/tests/fm-lint-workflows.test.sh index c7e918d678..ef611fbaa8 100755 --- a/tests/fm-lint-workflows.test.sh +++ b/tests/fm-lint-workflows.test.sh @@ -15,6 +15,121 @@ LINT="$ROOT/bin/fm-lint.sh" INSTALLER="$ROOT/bin/fm-install-actionlint.sh" REQUIRED=$("$LINT_WF" --required-version) +# Official sha256 values from actionlint_1.7.12_checksums.txt on the v1.7.12 +# release (https://github.com/rhysd/actionlint/releases/tag/v1.7.12). Tests +# compare installer behavior against these published digests, not script source. +ACTIONLINT_SHA_LINUX_AMD64=8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 +ACTIONLINT_SHA_LINUX_ARM64=325e971b6ba9bfa504672e29be93c24981eeb1c07576d730e9f7c8805afff0c6 +ACTIONLINT_SHA_DARWIN_AMD64=5b44c3bc2255115c9b69e30efc0fecdf498fdb63c5d58e17084fd5f16324c644 +ACTIONLINT_SHA_DARWIN_ARM64=aba9ced2dee8d27fecca3dc7feb1a7f9a52caefa1eb46f3271ea66b6e0e6953f + +fm_install_stub_uname() { + local fakebin=$1 + cat > "$fakebin/uname" <<'SH' +#!/usr/bin/env bash +case "${1:-}" in + -s) printf '%s\n' "${FM_TEST_UNAME_S:-Linux}" ;; + -m) printf '%s\n' "${FM_TEST_UNAME_M:-x86_64}" ;; + *) printf '%s\n' "${FM_TEST_UNAME_S:-Linux}" ;; +esac +SH + chmod +x "$fakebin/uname" +} + +fm_install_stub_curl() { + local fakebin=$1 + cat > "$fakebin/curl" <<'SH' +#!/usr/bin/env bash +count=0 +[ ! -f "${CURL_COUNT:-}" ] || count=$(cat "$CURL_COUNT") +count=$((count + 1)) +[ -z "${CURL_COUNT:-}" ] || printf '%s\n' "$count" > "$CURL_COUNT" +url= +out= +while [ "$#" -gt 0 ]; do + case "$1" in + -o) + out=$2 + shift 2 + ;; + -*) + shift + ;; + *) + url=$1 + shift + ;; + esac +done +[ -z "${CURL_URL_LOG:-}" ] || printf '%s\n' "$url" >> "$CURL_URL_LOG" +fail_until=${CURL_FAIL_UNTIL:-0} +[ "$count" -gt "$fail_until" ] || exit 22 +: > "$out" +exit 0 +SH + chmod +x "$fakebin/curl" +} + +fm_install_stub_hasher() { + local fakebin=$1 name=$2 + cat > "$fakebin/$name" <<'SH' +#!/usr/bin/env bash +self=${0##*/} +if [ -n "${HASHER_LOG:-}" ]; then + printf '%s\n' "$self $*" >> "$HASHER_LOG" +fi +file=$1 +if [ "$self" = shasum ]; then + algo= + file= + while [ "$#" -gt 0 ]; do + case "$1" in + -a) + algo=$2 + shift 2 + ;; + *) + file=$1 + shift + ;; + esac + done + [ "$algo" = 256 ] || exit 1 +fi +printf '%s %s\n' "${SHA256_STUB_HASH:?}" "$file" +SH + chmod +x "$fakebin/$name" +} + +fm_install_stub_tar_actionlint() { + local fakebin=$1 + cat > "$fakebin/tar" <<'SH' +#!/usr/bin/env bash +while [ "$#" -gt 0 ]; do + if [ "$1" = "-C" ]; then + cat > "$2/actionlint" <<'EOF' +#!/usr/bin/env bash +printf '1.7.12\n' +EOF + chmod +x "$2/actionlint" + exit 0 + fi + shift +done +exit 2 +SH + chmod +x "$fakebin/tar" +} + +fm_install_stub_sleep() { + local fakebin=$1 + cat > "$fakebin/sleep" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fakebin/sleep" +} + write_valid_workflow() { local path=$1 cat > "$path" <<'YAML' @@ -181,48 +296,16 @@ test_installer_retries_transient_download_failure() { fakebin=$(fm_fakebin "$tmp") destination="$tmp/bin" - cat > "$fakebin/curl" <<'SH' -#!/usr/bin/env bash -count=0 -[ ! -f "$CURL_COUNT" ] || count=$(cat "$CURL_COUNT") -count=$((count + 1)) -printf '%s\n' "$count" > "$CURL_COUNT" -[ "$count" -gt 3 ] || exit 22 -while [ "$#" -gt 0 ]; do - if [ "$1" = "-o" ]; then - : > "$2" - exit 0 - fi - shift -done -exit 2 -SH - cat > "$fakebin/sha256sum" <<'SH' -#!/usr/bin/env bash -printf '8aca8db96f1b94770f1b0d72b6dddcb1ebb8123cb3712530b08cc387b349a3d8 %s\n' "$1" -SH - cat > "$fakebin/tar" <<'SH' -#!/usr/bin/env bash -while [ "$#" -gt 0 ]; do - if [ "$1" = "-C" ]; then - cat > "$2/actionlint" <<'EOF' -#!/usr/bin/env bash -printf '1.7.12\n' -EOF - chmod +x "$2/actionlint" - exit 0 - fi - shift -done -exit 2 -SH - cat > "$fakebin/sleep" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$fakebin/curl" "$fakebin/sha256sum" "$fakebin/tar" "$fakebin/sleep" + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" sha256sum + fm_install_stub_tar_actionlint "$fakebin" + fm_install_stub_sleep "$fakebin" - out=$(CURL_COUNT="$tmp/curl-count" PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ + out=$(CURL_COUNT="$tmp/curl-count" CURL_FAIL_UNTIL=3 \ + SHA256_STUB_HASH="$ACTIONLINT_SHA_LINUX_AMD64" \ + FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=x86_64 \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ || fail "installer did not recover from a transient download failure"$'\n'"$out" [ "$(cat "$tmp/curl-count")" -eq 4 ] || fail "installer did not recover after three failed downloads" assert_contains "$out" "download attempt 3 failed; retrying" "installer did not disclose its third retry" @@ -230,6 +313,150 @@ SH pass "actionlint installer retries a transient download failure" } +test_installer_selects_platform_archive_url_and_checksum() { + local tmp fakebin destination out url_log uname_s uname_m archive sha + tmp=$(fm_test_tmproot fm-actionlint-platform) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + url_log="$tmp/curl-url.log" + + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" sha256sum + fm_install_stub_tar_actionlint "$fakebin" + fm_install_stub_sleep "$fakebin" + + while IFS=$'\t' read -r uname_s uname_m archive sha; do + [ -n "$uname_s" ] || continue + rm -rf "$destination" + : > "$url_log" + out=$(CURL_URL_LOG="$url_log" SHA256_STUB_HASH="$sha" \ + FM_TEST_UNAME_S="$uname_s" FM_TEST_UNAME_M="$uname_m" \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ + || fail "installer failed for ${uname_s}/${uname_m}"$'\n'"$out" + assert_contains "$(cat "$url_log")" "$archive" \ + "installer did not download $archive for ${uname_s}/${uname_m}" + assert_contains "$(cat "$url_log")" \ + "https://github.com/rhysd/actionlint/releases/download/v${REQUIRED}/${archive}" \ + "installer used the wrong URL for ${uname_s}/${uname_m}" + [ -x "$destination/actionlint" ] || fail "installer did not install actionlint for ${uname_s}/${uname_m}" + done <&1) || rc=$? + [ "$rc" -ne 0 ] || fail "installer accepted a wrong checksum"$'\n'"$out" + assert_contains "$out" "checksum mismatch" "installer did not report a checksum mismatch" + assert_contains "$out" "actionlint_${REQUIRED}_linux_amd64.tar.gz" \ + "mismatch did not name the selected archive" + assert_contains "$out" "$ACTIONLINT_SHA_LINUX_AMD64" \ + "mismatch did not name the pinned linux/amd64 checksum" + [ ! -e "$destination/actionlint" ] || fail "installer installed actionlint after a checksum mismatch" + pass "actionlint installer rejects a wrong checksum" +} + +test_installer_falls_back_to_shasum() { + local tmp fakebin destination out hasher_log tool + tmp=$(fm_test_tmproot fm-actionlint-shasum) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + hasher_log="$tmp/hasher.log" + + for tool in bash dirname mktemp rm awk mkdir install cat chmod; do + ln -s "$(command -v "$tool")" "$fakebin/$tool" + done + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" shasum + fm_install_stub_tar_actionlint "$fakebin" + fm_install_stub_sleep "$fakebin" + + : > "$hasher_log" + out=$(CURL_URL_LOG="$tmp/curl-url.log" HASHER_LOG="$hasher_log" \ + SHA256_STUB_HASH="$ACTIONLINT_SHA_LINUX_AMD64" \ + FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=x86_64 \ + PATH="$fakebin" "$INSTALLER" "$destination" 2>&1) \ + || fail "installer did not fall back to shasum -a 256"$'\n'"$out" + assert_grep 'shasum -a 256' "$hasher_log" "installer did not invoke shasum -a 256" + [ -x "$destination/actionlint" ] || fail "installer did not install actionlint via shasum" + pass "actionlint installer falls back to shasum -a 256 when sha256sum is absent" +} + +test_installer_prefers_sha256sum_over_shasum() { + local tmp fakebin destination hasher_log + tmp=$(fm_test_tmproot fm-actionlint-sha256sum-pref) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + hasher_log="$tmp/hasher.log" + + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" sha256sum + fm_install_stub_hasher "$fakebin" shasum + fm_install_stub_tar_actionlint "$fakebin" + fm_install_stub_sleep "$fakebin" + + : > "$hasher_log" + PATH="$fakebin:$PATH" HASHER_LOG="$hasher_log" \ + SHA256_STUB_HASH="$ACTIONLINT_SHA_LINUX_AMD64" \ + FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=x86_64 \ + "$INSTALLER" "$destination" >/dev/null \ + || fail "installer failed when both hashers were present" + assert_grep 'sha256sum' "$hasher_log" "installer did not prefer sha256sum" + if grep -q 'shasum' "$hasher_log"; then + fail "installer invoked shasum even though sha256sum was present"$'\n'"$(cat "$hasher_log")" + fi + pass "actionlint installer prefers sha256sum when both hashers are present" +} + +test_installer_rejects_unsupported_platform() { + local tmp fakebin destination out rc + tmp=$(fm_test_tmproot fm-actionlint-unsupported) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + + rc=0 + out=$(FM_TEST_UNAME_S=FreeBSD FM_TEST_UNAME_M=amd64 \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "installer accepted an unsupported OS"$'\n'"$out" + assert_contains "$out" "unsupported platform" "installer did not name the unsupported platform" + assert_contains "$out" "FreeBSD-amd64" "installer did not report the detected OS/arch" + + rc=0 + out=$(FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=ppc64le \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "installer accepted an unsupported architecture"$'\n'"$out" + assert_contains "$out" "unsupported platform" "installer did not reject linux/ppc64le" + pass "actionlint installer rejects an unsupported OS or architecture" +} + # Prove the no-mistakes/local owner (bin/fm-lint.sh with no paths) catches a # self-broken ci.yml. Copy the lint scripts into a fake repo so the default # workflow root is the fixture, not this worktree. @@ -295,4 +522,9 @@ test_non_mapping_root_fails test_missing_actionlint_fails_closed test_rejects_wrong_actionlint_version test_installer_retries_transient_download_failure +test_installer_selects_platform_archive_url_and_checksum +test_installer_rejects_wrong_checksum +test_installer_falls_back_to_shasum +test_installer_prefers_sha256sum_over_shasum +test_installer_rejects_unsupported_platform test_fm_lint_default_path_catches_broken_ci_yml diff --git a/tests/fm-lint.test.sh b/tests/fm-lint.test.sh index 99eba0f8ab..e7fd94fc95 100755 --- a/tests/fm-lint.test.sh +++ b/tests/fm-lint.test.sh @@ -22,6 +22,128 @@ INSTALLER="$ROOT/bin/fm-install-shellcheck.sh" # The pinned version, read from the single source (the one owner itself). REQUIRED=$("$LINT" --required-version) +# Official GitHub release asset sha256 values for shellcheck v0.11.0 .tar.xz +# archives (https://github.com/koalaman/shellcheck/releases/tag/v0.11.0). Tests +# compare installer behavior against these published digests, not script source. +SHELLCHECK_SHA_LINUX_X86_64=8c3be12b05d5c177a04c29e3c78ce89ac86f1595681cab149b65b97c4e227198 +SHELLCHECK_SHA_LINUX_AARCH64=12b331c1d2db6b9eb13cfca64306b1b157a86eb69db83023e261eaa7e7c14588 +SHELLCHECK_SHA_DARWIN_X86_64=3c89db4edcab7cf1c27bff178882e0f6f27f7afdf54e859fa041fca10febe4c6 +SHELLCHECK_SHA_DARWIN_AARCH64=56affdd8de5527894dca6dc3d7e0a99a873b0f004d7aabc30ae407d3f48b0a79 + +# fm_install_stub_uname : uname -s / uname -m from FM_TEST_UNAME_S/M. +fm_install_stub_uname() { + local fakebin=$1 + cat > "$fakebin/uname" <<'SH' +#!/usr/bin/env bash +case "${1:-}" in + -s) printf '%s\n' "${FM_TEST_UNAME_S:-Linux}" ;; + -m) printf '%s\n' "${FM_TEST_UNAME_M:-x86_64}" ;; + *) printf '%s\n' "${FM_TEST_UNAME_S:-Linux}" ;; +esac +SH + chmod +x "$fakebin/uname" +} + +# fm_install_stub_curl : log the URL, fail CURL_FAIL_UNTIL times, then +# write an empty file at -o. CURL_COUNT and CURL_URL_LOG are paths the stub +# updates when invoked. +fm_install_stub_curl() { + local fakebin=$1 + cat > "$fakebin/curl" <<'SH' +#!/usr/bin/env bash +count=0 +[ ! -f "${CURL_COUNT:-}" ] || count=$(cat "$CURL_COUNT") +count=$((count + 1)) +[ -z "${CURL_COUNT:-}" ] || printf '%s\n' "$count" > "$CURL_COUNT" +url= +out= +while [ "$#" -gt 0 ]; do + case "$1" in + -o) + out=$2 + shift 2 + ;; + -*) + shift + ;; + *) + url=$1 + shift + ;; + esac +done +[ -z "${CURL_URL_LOG:-}" ] || printf '%s\n' "$url" >> "$CURL_URL_LOG" +fail_until=${CURL_FAIL_UNTIL:-0} +[ "$count" -gt "$fail_until" ] || exit 22 +: > "$out" +exit 0 +SH + chmod +x "$fakebin/curl" +} + +# fm_install_stub_hasher : sha256sum or shasum stub that prints +# SHA256_STUB_HASH and records the invocation on HASHER_LOG. shasum requires -a 256. +fm_install_stub_hasher() { + local fakebin=$1 name=$2 + cat > "$fakebin/$name" <<'SH' +#!/usr/bin/env bash +self=${0##*/} +if [ -n "${HASHER_LOG:-}" ]; then + printf '%s\n' "$self $*" >> "$HASHER_LOG" +fi +file=$1 +if [ "$self" = shasum ]; then + algo= + file= + while [ "$#" -gt 0 ]; do + case "$1" in + -a) + algo=$2 + shift 2 + ;; + *) + file=$1 + shift + ;; + esac + done + [ "$algo" = 256 ] || exit 1 +fi +printf '%s %s\n' "${SHA256_STUB_HASH:?}" "$file" +SH + chmod +x "$fakebin/$name" +} + +fm_install_stub_tar_shellcheck() { + local fakebin=$1 + cat > "$fakebin/tar" <<'SH' +#!/usr/bin/env bash +while [ "$#" -gt 0 ]; do + if [ "$1" = "-C" ]; then + mkdir -p "$2/shellcheck-v0.11.0" + cat > "$2/shellcheck-v0.11.0/shellcheck" <<'EOF' +#!/usr/bin/env bash +printf 'ShellCheck - shell script analysis tool\nversion: 0.11.0\n' +EOF + chmod +x "$2/shellcheck-v0.11.0/shellcheck" + exit 0 + fi + shift +done +exit 2 +SH + chmod +x "$fakebin/tar" +} + +fm_install_stub_sleep() { + local fakebin=$1 + cat > "$fakebin/sleep" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fakebin/sleep" +} + # True only when the resolved shellcheck is exactly the pinned version, so the # lint-running tests below match what CI enforces instead of a runner default. pinned_ready() { @@ -247,51 +369,19 @@ test_installer_retries_transient_download_failure() { fakebin=$(fm_fakebin "$tmp") destination="$tmp/bin" - cat > "$fakebin/curl" <<'SH' -#!/usr/bin/env bash -count=0 -[ ! -f "$CURL_COUNT" ] || count=$(cat "$CURL_COUNT") -count=$((count + 1)) -printf '%s\n' "$count" > "$CURL_COUNT" -# Reproduce the CI incident: the release endpoint returned 503 for all three -# formerly configured attempts before recovering. -[ "$count" -gt 3 ] || exit 22 -while [ "$#" -gt 0 ]; do - if [ "$1" = "-o" ]; then - : > "$2" - exit 0 - fi - shift -done -exit 2 -SH - cat > "$fakebin/sha256sum" <<'SH' -#!/usr/bin/env bash -printf '8c3be12b05d5c177a04c29e3c78ce89ac86f1595681cab149b65b97c4e227198 %s\n' "$1" -SH - cat > "$fakebin/tar" <<'SH' -#!/usr/bin/env bash -while [ "$#" -gt 0 ]; do - if [ "$1" = "-C" ]; then - mkdir -p "$2/shellcheck-v0.11.0" - cat > "$2/shellcheck-v0.11.0/shellcheck" <<'EOF' -#!/usr/bin/env bash -printf 'ShellCheck - shell script analysis tool\nversion: 0.11.0\n' -EOF - chmod +x "$2/shellcheck-v0.11.0/shellcheck" - exit 0 - fi - shift -done -exit 2 -SH - cat > "$fakebin/sleep" <<'SH' -#!/usr/bin/env bash -exit 0 -SH - chmod +x "$fakebin/curl" "$fakebin/sha256sum" "$fakebin/tar" "$fakebin/sleep" - - out=$(CURL_COUNT="$tmp/curl-count" PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" sha256sum + fm_install_stub_tar_shellcheck "$fakebin" + fm_install_stub_sleep "$fakebin" + + # Reproduce the CI incident: the release endpoint returned 503 for all three + # formerly configured attempts before recovering. Force linux/x86_64 so the + # retry path stays the CI archive even when this suite runs on macOS. + out=$(CURL_COUNT="$tmp/curl-count" CURL_FAIL_UNTIL=3 \ + SHA256_STUB_HASH="$SHELLCHECK_SHA_LINUX_X86_64" \ + FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=x86_64 \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ || fail "installer did not recover from a transient download failure"$'\n'"$out" [ "$(cat "$tmp/curl-count")" -eq 4 ] || fail "installer did not recover after three failed downloads" assert_contains "$out" "download attempt 3 failed; retrying" "installer did not disclose its third retry" @@ -299,6 +389,151 @@ SH pass "ShellCheck installer retries a transient download failure" } +test_installer_selects_platform_archive_url_and_checksum() { + local tmp fakebin destination out url_log uname_s uname_m archive sha + tmp=$(fm_test_tmproot fm-shellcheck-platform) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + url_log="$tmp/curl-url.log" + + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" sha256sum + fm_install_stub_tar_shellcheck "$fakebin" + fm_install_stub_sleep "$fakebin" + + while IFS=$'\t' read -r uname_s uname_m archive sha; do + [ -n "$uname_s" ] || continue + rm -rf "$destination" + : > "$url_log" + out=$(CURL_URL_LOG="$url_log" SHA256_STUB_HASH="$sha" \ + FM_TEST_UNAME_S="$uname_s" FM_TEST_UNAME_M="$uname_m" \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) \ + || fail "installer failed for ${uname_s}/${uname_m}"$'\n'"$out" + assert_contains "$(cat "$url_log")" "$archive" \ + "installer did not download $archive for ${uname_s}/${uname_m}" + assert_contains "$(cat "$url_log")" \ + "https://github.com/koalaman/shellcheck/releases/download/v${REQUIRED}/${archive}" \ + "installer used the wrong URL for ${uname_s}/${uname_m}" + [ -x "$destination/shellcheck" ] || fail "installer did not install ShellCheck for ${uname_s}/${uname_m}" + done <&1) || rc=$? + [ "$rc" -ne 0 ] || fail "installer accepted a wrong checksum"$'\n'"$out" + assert_contains "$out" "checksum mismatch" "installer did not report a checksum mismatch" + assert_contains "$out" "shellcheck-v${REQUIRED}.linux.x86_64.tar.xz" \ + "mismatch did not name the selected archive" + assert_contains "$out" "$SHELLCHECK_SHA_LINUX_X86_64" \ + "mismatch did not name the pinned linux/x86_64 checksum" + [ ! -e "$destination/shellcheck" ] || fail "installer installed ShellCheck after a checksum mismatch" + pass "ShellCheck installer rejects a wrong checksum" +} + +test_installer_falls_back_to_shasum() { + local tmp fakebin destination out hasher_log tool + tmp=$(fm_test_tmproot fm-shellcheck-shasum) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + hasher_log="$tmp/hasher.log" + + for tool in bash dirname mktemp rm awk mkdir install cat chmod; do + ln -s "$(command -v "$tool")" "$fakebin/$tool" + done + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" shasum + fm_install_stub_tar_shellcheck "$fakebin" + fm_install_stub_sleep "$fakebin" + + # Restricted PATH: shasum is present, sha256sum is not. + : > "$hasher_log" + out=$(CURL_URL_LOG="$tmp/curl-url.log" HASHER_LOG="$hasher_log" \ + SHA256_STUB_HASH="$SHELLCHECK_SHA_LINUX_X86_64" \ + FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=x86_64 \ + PATH="$fakebin" "$INSTALLER" "$destination" 2>&1) \ + || fail "installer did not fall back to shasum -a 256"$'\n'"$out" + assert_grep 'shasum -a 256' "$hasher_log" "installer did not invoke shasum -a 256" + [ -x "$destination/shellcheck" ] || fail "installer did not install ShellCheck via shasum" + pass "ShellCheck installer falls back to shasum -a 256 when sha256sum is absent" +} + +test_installer_prefers_sha256sum_over_shasum() { + local tmp fakebin destination hasher_log + tmp=$(fm_test_tmproot fm-shellcheck-sha256sum-pref) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + hasher_log="$tmp/hasher.log" + + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + fm_install_stub_hasher "$fakebin" sha256sum + fm_install_stub_hasher "$fakebin" shasum + fm_install_stub_tar_shellcheck "$fakebin" + fm_install_stub_sleep "$fakebin" + + : > "$hasher_log" + PATH="$fakebin:$PATH" HASHER_LOG="$hasher_log" \ + SHA256_STUB_HASH="$SHELLCHECK_SHA_LINUX_X86_64" \ + FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=x86_64 \ + "$INSTALLER" "$destination" >/dev/null \ + || fail "installer failed when both hashers were present" + assert_grep 'sha256sum' "$hasher_log" "installer did not prefer sha256sum" + if grep -q 'shasum' "$hasher_log"; then + fail "installer invoked shasum even though sha256sum was present"$'\n'"$(cat "$hasher_log")" + fi + pass "ShellCheck installer prefers sha256sum when both hashers are present" +} + +test_installer_rejects_unsupported_platform() { + local tmp fakebin destination out rc + tmp=$(fm_test_tmproot fm-shellcheck-unsupported) + fakebin=$(fm_fakebin "$tmp") + destination="$tmp/bin" + + fm_install_stub_uname "$fakebin" + fm_install_stub_curl "$fakebin" + + rc=0 + out=$(FM_TEST_UNAME_S=FreeBSD FM_TEST_UNAME_M=amd64 \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "installer accepted an unsupported OS"$'\n'"$out" + assert_contains "$out" "unsupported platform" "installer did not name the unsupported platform" + assert_contains "$out" "FreeBSD-amd64" "installer did not report the detected OS/arch" + + rc=0 + out=$(FM_TEST_UNAME_S=Linux FM_TEST_UNAME_M=ppc64le \ + PATH="$fakebin:$PATH" "$INSTALLER" "$destination" 2>&1) || rc=$? + [ "$rc" -ne 0 ] || fail "installer accepted an unsupported architecture"$'\n'"$out" + assert_contains "$out" "unsupported platform" "installer did not reject linux/ppc64le" + pass "ShellCheck installer rejects an unsupported OS or architecture" +} + test_rejects_wrong_shellcheck_version() { # Version-independent: a fake shellcheck reporting a different version must be # refused before any lint, proving local and CI cannot silently diverge. @@ -626,6 +861,11 @@ SH test_list_files_reports_the_shell_inventory test_pins_an_explicit_version test_installer_retries_transient_download_failure +test_installer_selects_platform_archive_url_and_checksum +test_installer_rejects_wrong_checksum +test_installer_falls_back_to_shasum +test_installer_prefers_sha256sum_over_shasum +test_installer_rejects_unsupported_platform test_rejects_wrong_shellcheck_version test_catches_a_real_lint_defect test_ignores_ambient_shellcheck_opts From 312871d1c768c2252630d4f5f2737664e2fb3345 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:47:45 -0700 Subject: [PATCH 03/16] docs: reconcile test-evidence docs with store_in_repo: true (#2548) .no-mistakes.yaml has set test.evidence.store_in_repo: true since #2355, but CONTRIBUTING.md, docs/configuration.md, and docs/architecture.md still described the old policy of keeping evidence out of the repo in a temp directory. The current no-mistakes behavior for store_in_repo: true is to publish each run's test evidence to the orphan no-mistakes/evidence branch and link it from the PR body. That branch shares no history with code branches, so evidence never enters a pushed feature branch or the default branch, and CI's tracked personal fleet paths rule stays accurate. Docs only. No change to .no-mistakes.yaml or any workflow. --- CONTRIBUTING.md | 4 ++-- docs/architecture.md | 4 ++-- docs/configuration.md | 5 +++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97442a450f..cef1f1180f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -67,9 +67,9 @@ A crewmate picking up such a brief should load the skill even if the brief preda When supervising live crewmates, keep firstmate's own long validation or build commands in the background so watcher wakes can still be handled. Crewmate validation follows the installed no-mistakes version's SKILL.md and live `axi` help instead of duplicating gate mechanics in firstmate docs. Firstmate's wrapper still matters: crewmates route every `ask-user` finding to firstmate, which applies the authority contract in `AGENTS.md`, and crewmates avoid `--yes` because it would bypass that check and any required captain escalation. -Local `.no-mistakes/` state and test evidence stay out of this repo; `.no-mistakes.yaml` keeps evidence in a temp directory and pins the gate's lint command to `bin/fm-lint.sh`, matching the Linux CI lint job. +`.no-mistakes.yaml` publishes test evidence to the orphan `no-mistakes/evidence` branch, which shares no history with code branches, and pins the gate's lint command to `bin/fm-lint.sh`, matching the Linux CI lint job. Local no-mistakes Test is intent-targeted and must not re-run every `tests/*.test.sh`; `.github/workflows/ci.yml` owns the broad behavior suite plus platform-specific compatibility lanes. -That is firstmate-specific; do not commit `.no-mistakes/evidence/` here even when another no-mistakes-managed target project keeps committed PR evidence. +The pipeline publishes that evidence itself, so never hand-commit `.no-mistakes/` paths onto a feature branch; CI rejects them as tracked personal fleet paths. Check and test the toolbelt before pushing: diff --git a/docs/architecture.md b/docs/architecture.md index b735685822..b07da27b98 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -244,8 +244,8 @@ A ship brief records its mode as a fixed machine-readable line and the spawn ref `data/projects.md` records each project's standing posture and optional `+yolo` flag as the captain's default and as context for that decision, including the conditional `no-mistakes-prod-only` policy; a ship spawn that drops below the registered rigor prints a deviation notice and continues. `bin/fm-project-mode.sh` remains the one registry parser for the mechanical consumers that have no task in hand: fleet sync's `local-only` skip and home seeding's refusal and no-mistakes initialization. When a selected delivery path calls for a diff, `bin/fm-review-diff.sh` refreshes the authoritative base and, when task meta records `pr=`, always fetches and compares against `refs/pull//head` by default (recorded `pr_head=` is only an offline fallback) before falling back to the local branch with a warning. -For target project repos shipped through their own no-mistakes pipeline, commits under `.no-mistakes/evidence/` are the pipeline's PR-viewable validation evidence and are expected to stay in the crew branch until the evidence-hosting design changes. -The firstmate repo itself is the exception: its `.no-mistakes/` directory is local state, stays gitignored, and is rejected by CI if tracked. +Where a no-mistakes pipeline stores evidence in the repo, it publishes that PR-viewable validation evidence to an orphan evidence branch that shares no history with code branches, so it never enters the crew branch or the default branch. +This repo uses that setting, and its own `.no-mistakes/` directory remains local state that stays gitignored and is rejected by CI if tracked; [`configuration.md`](configuration.md) owns the setting. PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling `gh-axi pr merge`. The helper requires a full `https://github.com///pull/` URL, invokes `gh-axi pr merge --repo /`, defaults to `--squash`, preserves explicit merge-method flags, and rejects malformed URLs or repo override flags before recording merge state; a well-formed GitLab merge request URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) is refused too, explicitly, rather than sent to the wrong forge. Teardown is fail-closed for ship worktrees: dirty worktrees refuse, and committed work must be landed before the worktree is returned. diff --git a/docs/configuration.md b/docs/configuration.md index e4aef1a7cc..415c113991 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -129,8 +129,9 @@ See [`trace-context.md`](trace-context.md) for carrier semantics, supported rout ## Gate defaults (.no-mistakes.yaml) -The tracked `.no-mistakes.yaml` keeps test evidence outside the repo and pins `commands.lint` to `bin/fm-lint.sh` so local lint matches CI. -That evidence policy is specific to the firstmate repo: target projects may legitimately commit `.no-mistakes/evidence/` from their own no-mistakes pipeline, but firstmate keeps `.no-mistakes/` local and CI rejects tracked entries under that path. +The tracked `.no-mistakes.yaml` sets `test.evidence.store_in_repo: true` and pins `commands.lint` to `bin/fm-lint.sh` so local lint matches CI. +Storing evidence in the repo publishes each run's test artifacts to the orphan `no-mistakes/evidence` branch and links them from the PR body, instead of keeping them on local disk under the no-mistakes home. +That branch shares no history with code branches, so evidence never enters a pushed feature branch or the default branch; the worktree's `.no-mistakes/` stays local and CI rejects tracked entries under that path. It does not set `commands.test` to a complete `tests/*.test.sh` walk. See [CONTRIBUTING.md](../CONTRIBUTING.md) for the firstmate-specific local test policy and entry points. Portable shard evidence and coverage rules are in [fm-test-portable-shards.md](fm-test-portable-shards.md); [herdr-backend.md](herdr-backend.md#destructive-lab-safety) owns the real-Herdr lane's isolation boundary, and [runtime-backends.md](verification/runtime-backends.md#herdr) owns active evidence. From 64d61aed84373e02b1a28c4e6b262908ed8128d5 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Mon, 17 Aug 2026 16:08:26 -0700 Subject: [PATCH 04/16] docs: clarify test evidence branch storage (#2549) * docs: correct test evidence storage comment in .no-mistakes.yaml * no-mistakes: apply CI fixes --- .no-mistakes.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.no-mistakes.yaml b/.no-mistakes.yaml index 219c39b794..f825543372 100644 --- a/.no-mistakes.yaml +++ b/.no-mistakes.yaml @@ -37,7 +37,8 @@ document: commands: lint: 'bin/fm-lint.sh' -# Store test evidence in this repo so it is committed alongside the change instead of kept in a temp dir. +# Publish each run's test evidence to the orphan no-mistakes/evidence branch linked from the PR. +# The evidence is not committed to the feature or default branch. test: evidence: store_in_repo: true From d023c451e00fb64f9845b27fa949c02beed8c551 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Mon, 17 Aug 2026 20:16:15 -0700 Subject: [PATCH 05/16] docs: hint that live scouts may host their own Lavish review loop (#2563) Make that a first-class option in always-loaded instructions so firstmate does not default to mediating and tearing the scout down between iteration rounds. --- .agents/skills/process-event-sources/SKILL.md | 2 +- AGENTS.md | 1 + bin/fm-brief.sh | 1 + tests/fm-brief.test.sh | 2 ++ 4 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.agents/skills/process-event-sources/SKILL.md b/.agents/skills/process-event-sources/SKILL.md index e5fd0c9b1c..793ac54612 100644 --- a/.agents/skills/process-event-sources/SKILL.md +++ b/.agents/skills/process-event-sources/SKILL.md @@ -25,7 +25,7 @@ Firstmate registers a source, keeps working, and is woken when that process comp ## Arming a source Use the adapter, not the generic runner, for a real source. -For a Lavish review artifact: +For a Lavish review artifact firstmate owns (a live investigating scout should host its own loop): ```sh bin/fm-procevent-lavish.sh arm diff --git a/AGENTS.md b/AGENTS.md index 334ff6b8ee..6d6c955228 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -372,6 +372,7 @@ Retire one only on an explicit captain or main-firstmate decision, after loading A completed scout must leave a self-contained report before its scratch worktree can be discarded; read and relay its findings, record the report as the Done artifact, and re-evaluate the queue. A report may recommend implementation but does not authorize it. Before treating the investigation or any visual review as complete, load `decision-hold-lifecycle`; teardown enforces that shared completion gate. +When a scout's deliverable is a visual artifact the captain will iterate on, prefer keeping that scout alive to host its own Lavish loop rather than tearing it down and mediating from firstmate, so the scout keeps its investigation context and the captain iterates in one continuous session. When implementation is separately authorized, promote the existing scout through `bin/fm-promote.sh` rather than creating a duplicate task. The promoted worker must inventory scratch state, return to a clean default-branch base, carry over only intended fix changes, create the ship branch, and follow the project's selected delivery path while leaving scratch commits and debug edits behind and turning a reproduced bug into the regression test. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index a873c84051..206e5a947a 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -339,6 +339,7 @@ The report is the only thing that survives, so anything worth keeping must be in # Definition of done Write your findings to \`$DATA/$ID/report.md\`. The report must stand alone: what you did, what you found, the evidence (commands run, output, file:line references), and what you recommend. +If your deliverable is a visual artifact the captain will review and iterate on, you may host the Lavish review loop yourself (poll, revise, re-serve, staying alive) instead of handing it back to firstmate. Before reporting done, read and follow \`$FM_ROOT/.agents/skills/decision-hold-lifecycle/SKILL.md\` and pass its shared completion gate for the report and any visual review. When the report is complete, append \`done: {one-line conclusion}\` to the status file and stop. If your findings reveal work that should ship (e.g. you reproduced a bug and the fix is clear), say so in the report; firstmate may promote this task in place, and you would then receive mode-specific ship instructions as a follow-up message. diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index a348e2d345..c5ee3d00f0 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -699,6 +699,8 @@ test_scout_and_secondmate_scaffold() { assert_present "$brief" "scout brief was not scaffolded" assert_grep "SCOUT task" "$brief" "scout brief must declare itself a scout task" assert_grep "report.md" "$brief" "scout brief must point at the report deliverable" + assert_grep "you may host the Lavish review loop yourself" "$brief" \ + "scout brief must mention the option to host a Lavish review loop" FM_SECONDMATE_CHARTER='Supervise the alpha domain.' \ FM_HOME="$BRIEF_HOME" "$ROOT/bin/fm-brief.sh" brief-sm-q6 --secondmate alpha >/dev/null 2>&1 \ From d843712808658f26a7a3f248e632cb999864ca50 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:05:02 -0700 Subject: [PATCH 06/16] fix(bin): report remote secondmate delivery and state truthfully (#2570) * fix(bin): report remote secondmate delivery and state truthfully A steer to a remote secondmate crosses fm-on.sh to a host-local fm-send leg whose unconfirmed submit read-back (verdict=pending, typically a busy mate whose harness queues the steer) was flattened into exit 1, so the parent printed "error: text not submitted" / "error: text not sent" and discarded the pending-reply expectation for a steer that had actually landed. fm-send now carries the verdict across the ssh boundary as a documented delivered-unconfirmed exit 3: the parent reports the steer as delivered with confirmation pending, exits 0, keeps the expectation armed (awaiting_report), and closes --resolve-key decisions, while transport loss (ssh 255) and real remote failures keep failing loudly with the remote leg's stderr attached. A local unconfirmed submit now also exits 3 with an honest non-error message and still never closes a decision key. fm-crew-state.sh and fm-peek.sh no longer read a remote mate's endpoint through local probes (which misreported a healthy mate as "worktree gone" / "can't find session: remote"): both now use the true remote source over fm-on.sh, and an unreachable or unreadable remote reads as unknown-remote, never as gone or dead. * no-mistakes(document): Document remote delivery and state truth * no-mistakes: apply CI fixes --- .../skills/stuck-crewmate-recovery/SKILL.md | 2 +- bin/fm-crew-state.sh | 57 +++- bin/fm-peek.sh | 23 +- bin/fm-remote-secondmate-control.sh | 6 + bin/fm-send.sh | 62 +++- docs/remote-secondmates.md | 8 + docs/tmux-backend.md | 2 +- tests/fm-crew-state.test.sh | 103 +++++++ tests/fm-daemon.test.sh | 30 +- tests/fm-peek-remote.test.sh | 110 +++++++ tests/fm-send-remote-delivery.test.sh | 285 ++++++++++++++++++ 11 files changed, 668 insertions(+), 20 deletions(-) create mode 100755 tests/fm-peek-remote.test.sh create mode 100755 tests/fm-send-remote-delivery.test.sh diff --git a/.agents/skills/stuck-crewmate-recovery/SKILL.md b/.agents/skills/stuck-crewmate-recovery/SKILL.md index cf741b9d95..b9b94b27d4 100644 --- a/.agents/skills/stuck-crewmate-recovery/SKILL.md +++ b/.agents/skills/stuck-crewmate-recovery/SKILL.md @@ -23,7 +23,7 @@ The target window's harness is recorded as `harness=` in `state/.meta`. This procedure covers ordinary `kind=ship` and `kind=scout` direct reports. Load `secondmate-provisioning` instead for `kind=secondmate` recovery. -For a REMOTE secondmate, `fm-crew-state`'s `unknown`/`worktree gone` and `fm-send`'s `remote send failed`/`delivery unconfirmed` verdicts are unreliable and routinely false-negative; do not conclude the mate is dead or the send failed from those alone, confirm against the actual remote pane first. +For a REMOTE secondmate, `fm-crew-state` and `fm-peek` read the actual remote endpoint over `fm-on.sh`, and `fm-send` reports a delivered-with-pending-confirmation steer as delivered (their headers own the contracts); an `unknown-remote` read or unreachable-host failure means the remote state could not be read, never that the mate is dead or the send failed. Recover a genuinely stuck remote mate only through `bin/fm-spawn.sh --secondmate`, never raw herdr pane close/kill surgery, which strands the endpoint binding. Treat the digest's endpoint result as a presence signal, not proof that the task's work or validation run is gone. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 2cb290373c..df627b487f 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -16,10 +16,17 @@ # fixed mapping logic, no heuristics and no LLM. Output is one stable, parseable, # token-tight line firstmate can read every heartbeat: # -# state: · source: · +# state: · source: · # # Logic, in order: -# 1. Resolve worktree + backend target + kind from state/.meta. +# 1. Resolve worktree + backend target + kind from state/.meta. A meta +# recording remote_host= is a remote secondmate: its worktree and endpoint +# live on that host, so the local worktree and pane reads are skipped and +# the remote host is asked for the endpoint's recovery-grade state +# (fm-on.sh + fm-remote-secondmate-control.sh state). alive falls through +# to the routed status log; dead/missing report the remote verdict; an +# unreachable or unreadable remote reports unknown-remote, never a false +# gone/dead. # 2. Matching no-mistakes run for this crew's branch AND current code identity, # active or terminal (from `axi status`, or the coarse `no-mistakes runs` # fallback)? Branch name alone is not enough: a historical run on a reused @@ -101,10 +108,13 @@ meta_value() { # WT=$(meta_value worktree) KIND=$(meta_value kind) HARNESS=$(meta_value harness) +REMOTE_HOST=$(meta_value remote_host) [ -n "$KIND" ] || KIND=ship -# A torn-down (or never-created) worktree has no current state to read. -if [ -z "$WT" ] || [ ! -d "$WT" ]; then +# A torn-down (or never-created) worktree has no current state to read. A +# remote secondmate's recorded worktree is a path on ITS host, so the local +# probe proves nothing for it - the remote arm below reads the true source. +if [ -z "$REMOTE_HOST" ] && { [ -z "$WT" ] || [ ! -d "$WT" ]; }; then emit unknown none "worktree gone (torn down?)" fi @@ -138,6 +148,45 @@ map_log_state() { # LOG_LINE=$(log_last_line || true) LOG_VERB=$(status_line_verb "$LOG_LINE") +# --- remote secondmate: the true source is the remote endpoint --------------- +# A remote mate's recorded worktree and backend target live on its own host, so +# the local worktree probe above and the local pane reads below would misreport +# a healthy remote mate as gone or dead. Ask the remote host for the endpoint's +# recovery-grade state over the same fm-on.sh transport fm-send uses, then read +# current activity from the routed status log exactly as for a local +# secondmate (an idle endpoint is healthy for a secondmate either way). An +# unreachable host or unreadable endpoint is reported as unknown-remote - +# explicitly NOT proof of death - so a transport blip never reads as a torn +# down or dead mate; only the remote host's own dead/missing verdict may say +# the endpoint is actually gone. +if [ -n "$REMOTE_HOST" ]; then + if ! REMOTE_STATE=$(FM_HOME="$FM_HOME" "$SCRIPT_DIR/fm-on.sh" "$ID" \ + fm-remote-secondmate-control.sh state "$ID" < /dev/null 2>/dev/null); then + REMOTE_STATE= + fi + REMOTE_STATE=$(printf '%s\n' "$REMOTE_STATE" | tail -1) + case "$REMOTE_STATE" in + alive) + if [ -n "$LOG_VERB" ]; then + LOG_STATE=$(map_log_state "$LOG_LINE") + if [ "$LOG_STATE" != unknown ]; then + emit "$LOG_STATE" status-log "$(status_line_note "$LOG_LINE")${SEP}remote endpoint alive on $REMOTE_HOST" + fi + fi + emit unknown remote-endpoint "alive on $REMOTE_HOST (an idle secondmate is healthy)" + ;; + dead|missing) + emit unknown remote-endpoint "remote endpoint $REMOTE_STATE on $REMOTE_HOST" + ;; + '') + emit unknown remote-endpoint "unknown-remote: $REMOTE_HOST unreachable or endpoint unreadable (not proof of death)" + ;; + *) + emit unknown remote-endpoint "unknown-remote: endpoint state '$REMOTE_STATE' on $REMOTE_HOST (not proof of death)" + ;; + esac +fi + # pane_readable is consulted ONLY in the no-run fallback below. The run-step path # stays authoritative regardless of pane liveness - judge by the run-step, not the # shell - so a finished crew whose endpoint has closed still reports its run-step diff --git a/bin/fm-peek.sh b/bin/fm-peek.sh index 97d2ffe2d2..e3156f66ed 100755 --- a/bin/fm-peek.sh +++ b/bin/fm-peek.sh @@ -3,6 +3,11 @@ # Usage: fm-peek.sh [lines=40] # may be an exact task id, a legacy fm- task label resolved # through this home's state/.meta, or an explicit backend target. +# A selector whose meta records remote_host= is a remote secondmate: its pane +# lives on that host, so the capture routes over fm-on.sh to the host-local +# capture (fm-remote-secondmate-control.sh), clamped to that command's +# 100-line cap. An unreachable host or unreadable endpoint fails loudly naming +# the host; the local backend adapters are never asked to read a remote target. set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -16,9 +21,25 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" "$SCRIPT_DIR/fm-guard.sh" || true RAW_TARGET=$1 -T=$(fm_backend_resolve_selector "$RAW_TARGET" "$STATE") N=${2:-40} +REMOTE_META=$(fm_backend_meta_for_selector "$RAW_TARGET" "$STATE" 2>/dev/null || true) +if [ -n "$REMOTE_META" ] && [ -n "$(fm_meta_get "$REMOTE_META" remote_host)" ]; then + REMOTE_ID=${REMOTE_META##*/} + REMOTE_ID=${REMOTE_ID%.meta} + REMOTE_HOST=$(fm_meta_get "$REMOTE_META" remote_host) + case "$N" in ''|*[!0-9]*|0) N=40 ;; esac + [ "$N" -le 100 ] || N=100 + if ! FM_HOME="$FM_HOME" "$SCRIPT_DIR/fm-on.sh" "$REMOTE_ID" \ + fm-remote-secondmate-control.sh capture "$REMOTE_ID" "$N" < /dev/null; then + echo "error: could not read the remote pane of $REMOTE_ID on $REMOTE_HOST (host unreachable or endpoint unreadable; the mate is not thereby dead)" >&2 + exit 1 + fi + exit 0 +fi + +T=$(fm_backend_resolve_selector "$RAW_TARGET" "$STATE") + BACKEND=$(fm_backend_of_selector "$RAW_TARGET" "$T" "$STATE") EXPECTED_LABEL=$(fm_backend_expected_label_of_selector "$RAW_TARGET" "$STATE") diff --git a/bin/fm-remote-secondmate-control.sh b/bin/fm-remote-secondmate-control.sh index f2edb32a7b..aa17c95286 100755 --- a/bin/fm-remote-secondmate-control.sh +++ b/bin/fm-remote-secondmate-control.sh @@ -188,6 +188,12 @@ cmd_send() { validate_id "$id" validate_home "$id" remote_endpoint_require "$id" + # fm-send's exit status is the delivery verdict the parent home acts on + # (0 = confirmed, 3 = delivered with the submit read-back unconfirmed, other + # nonzero = failed; see bin/fm-send.sh's header). The job worker, entrypoint, + # and ssh all preserve it, so no mapping may happen here: flattening exit 3 + # into a generic failure is exactly the false-negative the parent's remote + # send path exists to avoid. FM_HOME="$TARGET_HOME" FM_ROOT_OVERRIDE="$FM_ROOT" FM_STATE_OVERRIDE="$TARGET_HOME/state" \ "$SCRIPT_DIR/fm-send.sh" "$REMOTE_ENDPOINT_TARGET" "$message" } diff --git a/bin/fm-send.sh b/bin/fm-send.sh index c46c55a340..1da45d86f4 100755 --- a/bin/fm-send.sh +++ b/bin/fm-send.sh @@ -15,6 +15,12 @@ # submit or reports an inconclusive send. If a swallowed Enter is positively # confirmed, fm-send exits NON-ZERO so the caller knows the steer did not land # instead of silently leaving an unsubmitted instruction. +# Exit status contract: 0 = submit confirmed (or, for a remote secondmate +# target, delivered with confirmation pending - see the remote paragraph); +# 3 = the text was typed into the live endpoint and Enter was sent, but the +# submit read-back stayed unconfirmed (verify the pane before any resend, and +# never re-type blindly); any other nonzero = the send failed and nothing may +# be assumed delivered. # Submission dispatches through the target's recorded backend; the tmux adapter # shares its composer/submit core with the away-mode daemon via bin/fm-tmux-lib.sh. # Tune with FM_SEND_RETRIES (default 3) / FM_SEND_SLEEP (0.4). @@ -37,6 +43,20 @@ # re-sending a recovery request for an already-open expectation so a second # record is not created. Direct unmarked captain input never creates one. # +# Remote secondmate delivery: the send crosses fm-on.sh to a host-local leg +# (bin/fm-remote-secondmate-control.sh cmd_send) that runs this same verified +# submit against the recorded remote Herdr pane and relays its exit status +# unchanged. A leg that delivered the text into the live verified pane but +# could not synchronously confirm the submit (exit 3 - typically a busy mate +# whose harness queues the steer and keeps rendering it) is reported here as +# DELIVERED with confirmation pending: fm-send prints a non-error notice, +# exits 0, marks the pending-reply expectation delivered, and closes any +# --resolve-key decisions. Empirically that pattern is a delivered steer, a +# resend duplicates the instruction, and the parent's pending-reply +# recovery/escalation still surfaces the rare genuinely lost request. Transport +# loss (ssh exit 255, completion unknown) and every real remote failure keep +# failing loudly with the remote leg's own stderr attached. +# # Decision closure (answerer-closes): pass --resolve-key (repeatable, # before the message) when this send answers an open keyed needs-decision: or # blocked: record in the target task's state/.status. After the submit is @@ -63,7 +83,9 @@ # in this home's status log per status_open_decisions (bin/fm-classify-lib.sh), or # an active captain hold for the target task. A key in neither is refused before # sending, so a mistyped key cannot deliver an answer while silently orphaning the -# decision. A failed or unconfirmed send never closes a key; a +# decision. A failed or unconfirmed send never closes a key (a remote +# delivered-with-pending-confirmation outcome counts as delivered - see the +# remote paragraph above); a # delivered answer whose closing append fails exits nonzero with the exact # manual close command, leaving the decision open to re-surface (the safe # direction). A send without the flag never closes anything: a routine steer, @@ -537,12 +559,27 @@ else # Type once, submit, verify. Only exact empty confirms delivery; every other # verdict preserves the loud refusal boundary. send_rc=0 + REMOTE_DELIVERY_NOTICE=0 if [ "$TARGET_BACKEND" = remote ]; then - if "$SCRIPT_DIR/fm-on.sh" "$TARGET_REMOTE_ID" fm-remote-secondmate-control.sh send "$TARGET_REMOTE_ID" "$MESSAGE" < /dev/null >/dev/null; then + # The remote leg is this same script running host-locally against the + # recorded Herdr pane (cmd_send in fm-remote-secondmate-control.sh), so its + # submit verification IS the local one, and fm-on/the remote worker relay + # its exit status unchanged. Exit 3 is the delivered-unconfirmed contract + # (see this script's header) crossing the ssh boundary: the text reached + # the live verified pane and Enter was sent; only the synchronous read-back + # stayed unconfirmed. The remote stderr is held back and replayed only for + # a real failure, so a delivered outcome does not surface the inner leg's + # diagnostics as alarm. + remote_err=$("$SCRIPT_DIR/fm-on.sh" "$TARGET_REMOTE_ID" fm-remote-secondmate-control.sh send "$TARGET_REMOTE_ID" "$MESSAGE" < /dev/null 2>&1 >/dev/null) || send_rc=$? + if [ "$send_rc" -eq 0 ]; then + verdict=empty + elif [ "$send_rc" -eq 3 ]; then verdict=empty + send_rc=0 + REMOTE_DELIVERY_NOTICE=1 else - send_rc=$? verdict=send-failed + [ -z "$remote_err" ] || printf '%s\n' "$remote_err" >&2 fi elif verdict=$(fm_backend_send_text_submit "$TARGET_BACKEND" "$T" "$MESSAGE" "$retries" "$sleep_s" "$settle" "$EXPECTED_LABEL"); then : @@ -571,6 +608,19 @@ else echo "error: text not sent to $T ($TARGET_BACKEND send failed; tried $RESOLUTION_TRIED)" >&2 exit 1 ;; + pending) + # The text was typed into the live target and Enter was sent; only the + # submit read-back stayed unconfirmed (e.g. a busy harness queues the + # steer and keeps rendering it). That is not a proven failure, so never + # re-type the message: verify the pane instead. Exit 3 is the documented + # delivered-unconfirmed status, and the remote send leg above depends on + # it crossing the ssh boundary intact. + if [ "$PENDING_REPLY_CREATED" = 1 ] && [ -n "$PENDING_REPLY_CORR" ]; then + fm_pending_reply_discard_undelivered "$STATE" "$PENDING_REPLY_CORR" || true + fi + echo "fm-send: text delivered to $T but submission is unconfirmed (verdict=pending; tried $RESOLUTION_TRIED); do not retype or blindly resend - verify with fm-peek.sh, then re-send '--key Enter' only if the composer still holds the text" >&2 + exit 3 + ;; *) if [ "$PENDING_REPLY_CREATED" = 1 ] && [ -n "$PENDING_REPLY_CORR" ]; then fm_pending_reply_discard_undelivered "$STATE" "$PENDING_REPLY_CORR" || true @@ -600,6 +650,12 @@ else fm_send_close_resolved_keys "$RESOLVE_ANSWER_TEXT" || exit 1 fm_send_feed_resolved_holds "$RESOLVE_ANSWER_TEXT" || exit 1 fi + # Remote delivered-with-pending-confirmation: the outcome above is treated as + # delivered (expectation marked, keys closed), and this one non-error notice + # carries the remaining nuance so nobody re-sends the steer. + if [ "$REMOTE_DELIVERY_NOTICE" = 1 ]; then + echo "fm-send: delivered to remote secondmate $TARGET_REMOTE_ID; the remote pane accepted the text and Enter, and only the synchronous submit confirmation is still pending. This is not a failure - do not resend; the pending-reply expectation stays armed." >&2 + fi # Submit landed with exact empty. Confirmation only proves the text was # accepted; the harness still needs a beat to spin up the # turn before its busy footer shows. Pause so an immediate peek catches the diff --git a/docs/remote-secondmates.md b/docs/remote-secondmates.md index 5a38d48e52..a1560e20b5 100644 --- a/docs/remote-secondmates.md +++ b/docs/remote-secondmates.md @@ -168,6 +168,11 @@ Send routed requests normally: FM_HOME= bin/fm-send.sh fm- '' ``` +The [`fm-send.sh` header](../bin/fm-send.sh) owns the exact delivery-status contract. +When the verified remote endpoint accepts the text and Enter but synchronous submit confirmation remains pending, the primary reports the request as delivered rather than failed; do not resend it, because its pending-reply expectation remains armed. +`fm-peek.sh` and `fm-crew-state.sh` route remote-secondmate reads to the endpoint's host instead of consulting local worktree or backend state. +An unreachable or unreadable remote read is unknown, not evidence that the endpoint is dead. + Marked requests keep the existing correlation contract. The remote charter appends replies to `state/parent-replies.status` in the remote home. A process-event source performs a non-destructive, cursor-anchored delta read, fetches only referenced `data/*.md` documents through the confined reader, mirrors every content-bearing line at most once into the primary status channel, and does not carry blank separators. @@ -231,6 +236,9 @@ The lifecycle test covers seeding a registered project that this machine has nev ```sh bin/fm-test-run.sh tests/fm-on.test.sh +bin/fm-test-run.sh tests/fm-send-remote-delivery.test.sh +bin/fm-test-run.sh tests/fm-peek-remote.test.sh +bin/fm-test-run.sh tests/fm-crew-state.test.sh bin/fm-test-run.sh tests/fm-remote-job.test.sh bin/fm-test-run.sh tests/fm-remote-doctor.test.sh bin/fm-test-run.sh tests/fm-project-origin.test.sh diff --git a/docs/tmux-backend.md b/docs/tmux-backend.md index 4d8c3e75fe..c2acead0c2 100644 --- a/docs/tmux-backend.md +++ b/docs/tmux-backend.md @@ -85,7 +85,7 @@ The supervisor guard selects only the detected primary harness's signature rathe It types a message once and retries Enter only until the composer clears. Only a proven empty composer is a positive delivery acknowledgement. Text left in established structure remains `pending`, text in ambiguous structure remains unproven, and unreadable or unsafe state remains unknown. -`fm-send.sh` reports every unconfirmed verdict as a failure instead of retyping or assuming delivery. +`fm-send.sh` never retypes or assumes a confirmed submit for an unconfirmed verdict; its header owns the distinct delivered-unconfirmed exit status and operator response. OpenCode 1.18.4 has one busy-queue exception. While OpenCode is mid-turn, Enter queues the message but leaves its text visible until the turn completes. diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 8f986b6139..602b3e5cfc 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1150,6 +1150,105 @@ test_torn_down_worktree() { pass "torn-down worktree is handled gracefully" } +# --- remote secondmate arm --------------------------------------------------- +# A meta recording remote_host= must never be read through the local worktree +# probe or a local backend adapter: the recorded worktree and pane live on the +# remote host, and the old local reads misreported a healthy remote mate as +# "worktree gone". These cases drive the real helper over the real fm-on.sh +# route with a stubbed ssh transport (FM_SSH_BIN seam): the stub prints +# FM_FAKE_REMOTE_STATE_OUT as the remote endpoint's recovery-grade state and +# exits FM_FAKE_SSH_RC. + +setup_remote_case() { # -> echoes case dir with remote meta + registry + local d + d=$(new_case "$1") + mkdir -p "$d/data" "$d/fakebin" + fm_write_meta "$d/state/rsm.meta" \ + "window=remote:rsm" \ + "endpoint_task_id=rsm" \ + "worktree=/remote/home/never-locally-present" \ + "harness=claude" \ + "kind=secondmate" \ + "mode=secondmate" \ + "remote_host=remote-mac" \ + "remote_root=/remote/root" \ + "remote_backend=herdr" \ + "remote_herdr_session=fm-remote" \ + "remote_target=fm-remote:w1:p1" + cat > "$d/data/secondmates.md" < "$d/fakebin/fake-ssh" <<'SH' +#!/usr/bin/env bash +cat > /dev/null +[ -z "${FM_FAKE_REMOTE_STATE_OUT:-}" ] || printf '%s\n' "$FM_FAKE_REMOTE_STATE_OUT" +exit "${FM_FAKE_SSH_RC:-0}" +SH + chmod +x "$d/fakebin/fake-ssh" + printf '%s\n' "$d" +} + +run_remote_crew_state() { # + PATH="$1/fakebin:$PATH" FM_HOME="$1" FM_STATE_OVERRIDE="$1/state" \ + FM_SSH_BIN="$1/fakebin/fake-ssh" "$CREW_STATE" "$2" +} + +test_remote_alive_with_log_uses_status_log() { + reset_fakes + local d out rc + d=$(setup_remote_case remote-alive-log) + make_fakebin "$d" >/dev/null + printf 'working: refactoring the quota adapter\n' > "$d/state/rsm.status" + out=$(FM_FAKE_REMOTE_STATE_OUT=alive FM_FAKE_SSH_RC=0 run_remote_crew_state "$d" rsm); rc=$? + expect_code 0 "$rc" "remote alive exits 0" + assert_contains "$out" "state: working" "alive remote mate with a working log reads working" + assert_contains "$out" "source: status-log" "alive remote mate reads current activity from the routed log" + assert_contains "$out" "remote endpoint alive on remote-mac" "the remote liveness read should be visible" + assert_not_contains "$out" "worktree gone" "a healthy remote mate must never read as torn down" + pass "fm-crew-state remote: alive endpoint falls through to the routed status log" +} + +test_remote_alive_idle_is_healthy_not_gone() { + reset_fakes + local d out rc + d=$(setup_remote_case remote-alive-idle) + make_fakebin "$d" >/dev/null + out=$(FM_FAKE_REMOTE_STATE_OUT=alive FM_FAKE_SSH_RC=0 run_remote_crew_state "$d" rsm); rc=$? + expect_code 0 "$rc" "remote alive-idle exits 0" + assert_contains "$out" "source: remote-endpoint" "the remote endpoint is the reported source" + assert_contains "$out" "alive on remote-mac" "an idle remote mate reads alive" + assert_not_contains "$out" "worktree gone" "a healthy remote mate must never read as torn down" + assert_not_contains "$out" "backend target gone" "a healthy remote mate must never read as a dead target" + pass "fm-crew-state remote: an idle alive endpoint reads alive, never gone or dead" +} + +test_remote_unreachable_is_unknown_remote_not_dead() { + reset_fakes + local d out rc + d=$(setup_remote_case remote-unreachable) + make_fakebin "$d" >/dev/null + printf 'working: refactoring the quota adapter\n' > "$d/state/rsm.status" + out=$(FM_FAKE_SSH_RC=255 run_remote_crew_state "$d" rsm); rc=$? + expect_code 0 "$rc" "unreachable remote exits 0" + assert_contains "$out" "unknown-remote" "an unreachable remote must be labeled unknown-remote" + assert_contains "$out" "not proof of death" "an unreachable remote must not read as dead" + assert_not_contains "$out" "worktree gone" "an unreachable remote must never read as torn down" + assert_not_contains "$out" "backend target gone" "an unreachable remote must never read as a dead target" + pass "fm-crew-state remote: an unreachable host reads unknown-remote, never gone or dead" +} + +test_remote_dead_reports_remote_verdict() { + reset_fakes + local d out rc + d=$(setup_remote_case remote-dead) + make_fakebin "$d" >/dev/null + out=$(FM_FAKE_REMOTE_STATE_OUT=dead FM_FAKE_SSH_RC=0 run_remote_crew_state "$d" rsm); rc=$? + expect_code 0 "$rc" "remote dead exits 0" + assert_contains "$out" "remote endpoint dead on remote-mac" \ + "a genuinely dead remote endpoint reports the remote host's own verdict" + pass "fm-crew-state remote: the remote host's own dead verdict is reported truthfully" +} + test_missing_meta() { reset_fakes local d; d=$(new_case nometa) @@ -1350,6 +1449,10 @@ test_dead_window_still_reports_active_run_step test_no_timeout_uses_perl_bound test_scout_skips_run_lookup test_torn_down_worktree +test_remote_alive_with_log_uses_status_log +test_remote_alive_idle_is_healthy_not_gone +test_remote_unreachable_is_unknown_remote_not_dead +test_remote_dead_reports_remote_verdict test_missing_meta test_provably_working_via_runs_list_fallback test_not_provably_working_when_stopped diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index 2fe02fb431..ad0925be4d 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -1569,27 +1569,37 @@ test_inject_wedge_alarm_throttles_when_marker_cannot_be_written() { pass "in-process wedge throttle prevents alert spam when the marker cannot persist" } -test_fm_send_exits_nonzero_on_confirmed_swallow() { - # fm-send.sh must exit NON-ZERO when a steer's Enter is positively swallowed - # (text left in the composer), so firstmate learns the instruction did not land - # — and exit ZERO on a clean submit. - local dir fakebin err +test_fm_send_reports_delivered_unconfirmed_submit() { + # When text was typed and Enter sent but the submit read-back remains pending, + # fm-send must return its documented delivered-unconfirmed status and prevent + # a duplicate resend reflex. A synchronously confirmed submit remains zero. + local dir fakebin err rc dir=$(make_bordered_case send-swallow) fakebin="$dir/fakebin"; err="$dir/send.err" # Clean submit -> exit 0. PATH="$fakebin:$PATH" FM_HOME="$dir" FM_STATE_OVERRIDE="$dir/state" FM_FAKE_COMPOSER="$dir/composer" \ FM_SEND_SLEEP=0.05 "$ROOT/bin/fm-send.sh" sess:win 'route this work' >/dev/null 2>"$err" \ || fail "fm-send exited non-zero on a clean submit: $(cat "$err")" - # Persistent swallow -> exit non-zero with a clear message. + # Persistent composer text after Enter -> delivered-unconfirmed exit 3 with + # a non-error warning that explicitly tells the operator not to resend. printf '╭─────╮\n│ > │\n╰─────╯\n' > "$dir/composer" touch "$dir/.swallow" if PATH="$fakebin:$PATH" FM_HOME="$dir" FM_STATE_OVERRIDE="$dir/state" FM_FAKE_COMPOSER="$dir/composer" \ FM_FAKE_SWALLOW="$dir/.swallow" FM_FAKE_PERSIST_SWALLOW=1 FM_SEND_SLEEP=0.05 \ "$ROOT/bin/fm-send.sh" sess:win 'fix findings 1 and 3, skip 2' >/dev/null 2>"$err"; then - fail "fm-send exited zero despite a swallowed Enter (silent unsubmitted instruction)" + rc=0 + else + rc=$? + fi + [ "$rc" -eq 3 ] || fail "fm-send returned $rc instead of delivered-unconfirmed exit 3: $(cat "$err")" + grep -F 'submission is unconfirmed' "$err" >/dev/null \ + || fail "fm-send did not explain the pending confirmation: $(cat "$err")" + grep -F 'do not retype or blindly resend' "$err" >/dev/null \ + || fail "fm-send did not prevent a duplicate resend: $(cat "$err")" + if grep -F 'error:' "$err" >/dev/null; then + fail "fm-send mislabeled delivered-unconfirmed as an error: $(cat "$err")" fi - grep -F 'not submitted' "$err" >/dev/null || fail "fm-send did not explain the swallowed submit: $(cat "$err")" - pass "fm-send exits non-zero on a confirmed swallow, zero on a clean submit" + pass "fm-send returns 3 with a non-error no-resend warning when confirmation stays pending" } test_fm_send_exits_nonzero_on_initial_send_failure() { @@ -1916,7 +1926,7 @@ test_wedge_alarm_hung_override_times_out_and_falls_through test_wedge_alarm_shutdown_stops_active_notifier_group test_inject_wedge_alarm_fires_active_alert_on_non_tmux_backend test_inject_wedge_alarm_throttles_when_marker_cannot_be_written -test_fm_send_exits_nonzero_on_confirmed_swallow +test_fm_send_reports_delivered_unconfirmed_submit test_fm_send_exits_nonzero_on_initial_send_failure test_fm_send_exits_nonzero_on_unproven_submit test_discover_supervisor_backend_precedence diff --git a/tests/fm-peek-remote.test.sh b/tests/fm-peek-remote.test.sh new file mode 100755 index 0000000000..7ef7286fb2 --- /dev/null +++ b/tests/fm-peek-remote.test.sh @@ -0,0 +1,110 @@ +#!/usr/bin/env bash +# fm-peek remote-secondmate capture routing. +# +# A remote secondmate's pane lives on its own host. The old path resolved the +# meta's "remote:" window through the local backend adapters and handed it +# to tmux, which failed with "can't find session: remote" - a healthy remote +# mate misreported as an unreadable endpoint. These tests drive the real +# fm-peek + fm-on executables with a stubbed ssh transport (FM_SSH_BIN seam) +# and a poisoned local tmux, pinning: +# 1. A remote selector routes the capture over the remote transport and +# prints the remote pane tail; the local adapters are never consulted. +# 2. An unreachable host fails loudly naming the host, without claiming the +# mate is dead. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +PEEK="$ROOT/bin/fm-peek.sh" + +TMP_ROOT=$(fm_test_tmproot fm-peek-remote) + +# fake-ssh prints the canned remote capture; the poisoned tmux records any +# local read attempt so the "never consulted" property is a real assertion. +make_stubs() { # -> echoes fakebin dir + local dir=$1 fb="$1/fakebin" + mkdir -p "$fb" + cat > "$fb/fake-ssh" <<'SH' +#!/usr/bin/env bash +cat > /dev/null +[ -z "${FM_FAKE_REMOTE_CAPTURE:-}" ] || printf '%s\n' "$FM_FAKE_REMOTE_CAPTURE" +exit "${FM_FAKE_SSH_RC:-0}" +SH + chmod +x "$fb/fake-ssh" + cat > "$fb/tmux" <<'SH' +#!/usr/bin/env bash +printf 'tmux\n' >> "${FM_FAKE_TMUX_TOUCHED:?}" +exit 1 +SH + chmod +x "$fb/tmux" + printf '%s\n' "$fb" +} + +setup_remote_home() { # -> echoes home dir with remote meta + registry + local home="$TMP_ROOT/$1-$RANDOM" + mkdir -p "$home/state" "$home/data" + fm_write_meta "$home/state/rsm.meta" \ + "window=remote:rsm" \ + "endpoint_task_id=rsm" \ + "harness=claude" \ + "kind=secondmate" \ + "mode=secondmate" \ + "remote_host=remote-mac" \ + "remote_root=/remote/root" \ + "remote_backend=herdr" \ + "remote_herdr_session=fm-remote" \ + "remote_target=fm-remote:w1:p1" + cat > "$home/data/secondmates.md" < "$touched" + + out=$(env PATH="$fb:$PATH" \ + FM_HOME="$home" FM_STATE_OVERRIDE="$home/state" \ + FM_SSH_BIN="$fb/fake-ssh" FM_FAKE_SSH_RC=0 \ + FM_FAKE_REMOTE_CAPTURE='● the remote mate is mid-refactor' \ + FM_FAKE_TMUX_TOUCHED="$touched" \ + "$PEEK" rsm 20 2>"$dir/err"); rc=$? + expect_code 0 "$rc" "a healthy remote peek should succeed" + assert_contains "$out" "the remote mate is mid-refactor" \ + "the remote pane tail should be printed" + assert_not_contains "$out" "can't find session" \ + "a remote peek must not fall into a local session lookup" + [ ! -s "$touched" ] || fail "the local tmux adapter was consulted for a remote target" + pass "fm-peek remote: the capture routes over the remote transport, local adapters untouched" +} + +test_remote_peek_unreachable_fails_loudly_without_death_claim() { + local dir fb home touched rc err + dir="$TMP_ROOT/peek-down"; mkdir -p "$dir" + fb=$(make_stubs "$dir") + home=$(setup_remote_home peek-down) + touched="$dir/tmux-touched"; : > "$touched" + + env PATH="$fb:$PATH" \ + FM_HOME="$home" FM_STATE_OVERRIDE="$home/state" \ + FM_SSH_BIN="$fb/fake-ssh" FM_FAKE_SSH_RC=255 \ + FM_FAKE_TMUX_TOUCHED="$touched" \ + "$PEEK" rsm >"$dir/out" 2>"$dir/err"; rc=$? + err=$(cat "$dir/err") + [ "$rc" -ne 0 ] || fail "an unreachable remote peek must exit nonzero" + assert_contains "$err" "remote pane of rsm on remote-mac" \ + "the failure must name the remote mate and host" + assert_contains "$err" "not thereby dead" \ + "an unreadable remote pane must not be presented as a dead mate" + pass "fm-peek remote: an unreachable host fails loudly without a false death claim" +} + +test_remote_peek_reads_remote_pane +test_remote_peek_unreachable_fails_loudly_without_death_claim + +echo "all fm-peek-remote tests passed" diff --git a/tests/fm-send-remote-delivery.test.sh b/tests/fm-send-remote-delivery.test.sh new file mode 100755 index 0000000000..af546fbb4a --- /dev/null +++ b/tests/fm-send-remote-delivery.test.sh @@ -0,0 +1,285 @@ +#!/usr/bin/env bash +# fm-send remote-secondmate delivery reporting. +# +# The remote send leg (fm-on.sh -> fm-remote-secondmate-control.sh cmd_send) +# runs fm-send's own verified submit host-locally on the remote machine and +# relays its exit status unchanged. A leg that delivered the text into the +# live verified pane but could not synchronously confirm the submit exits 3 +# (the delivered-unconfirmed contract in bin/fm-send.sh's header); flattening +# that into a generic failure produced the false "error: text not sent" +# report that tempted duplicate resends of steers that had actually landed. +# These tests pin the delivery-reporting contract over the real fm-send + +# fm-on executables with a stubbed ssh transport (FM_SSH_BIN seam - the same +# process boundary tests/fm-on.test.sh proves preserves exit status): +# 1. Remote delivered-unconfirmed (ssh exit 3) is NOT a failure: exit 0, a +# non-error delivered notice, the inner leg's stderr held back, and the +# pending-reply expectation marked delivered (awaiting_report). +# 2. A real remote failure (nonzero, not 3/255) still fails loudly with the +# remote stderr replayed and the undelivered expectation discarded. +# 3. Transport-unknown (ssh exit 255) still refuses loudly and preserves the +# expectation as delivery_unknown. +# 4. A delivered-unconfirmed remote answer still closes its --resolve-key +# decision (delivered-with-pending-confirmation counts as delivered). +# 5. A LOCAL send whose submit read-back stays pending exits 3 with an +# honest non-error message (text delivered, submission unconfirmed). +# 6. That local unconfirmed send still never closes a --resolve-key +# decision (the local ledger boundary is unchanged). +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +SEND="$ROOT/bin/fm-send.sh" +DRAIN="$ROOT/bin/fm-wake-drain.sh" + +TMP_ROOT=$(fm_test_tmproot fm-send-remote-delivery) + +# Stub tmux for the local legs: logs literal typed text to FM_SEND_LOG. The +# default composer reads empty (clean submit); FM_FAKE_TMUX_PENDING=1 keeps a +# proven pending composer with no busy footer, so the real submit core +# exhausts its Enter budget and reports the pending verdict. The ssh stub +# records the invocation, emits FM_FAKE_SSH_STDERR as the remote leg's stderr, +# and exits FM_FAKE_SSH_RC - the exact relay contract the real transport +# preserves. +make_stubs() { # -> echoes fakebin dir + local dir=$1 fb="$1/fakebin" + mkdir -p "$fb" + cat > "$fb/tmux" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + send-keys) + shift + literal=0 + while [ $# -gt 0 ]; do + case "$1" in + -t) shift 2 ;; + -l) literal=1; shift ;; + *) break ;; + esac + done + if [ "$literal" = 1 ]; then + printf '%s' "${1:-}" >> "$FM_SEND_LOG" + fi + exit 0 ;; + display-message) + for a in "$@"; do case "$a" in *cursor_y*) printf '1\n'; exit 0 ;; esac; done + printf 'fakepane\n'; exit 0 ;; + capture-pane) + if [ "${FM_FAKE_TMUX_PENDING:-0}" = 1 ]; then + printf '╭────────────╮\n│ > steer │\n╰────────────╯\n' + else + printf '╭────╮\n│ │\n╰────╯\n' + fi + exit 0 ;; + list-windows) exit 0 ;; +esac +exit 0 +SH + chmod +x "$fb/tmux" + cat > "$fb/sleep" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fb/sleep" + cat > "$fb/fake-ssh" <<'SH' +#!/usr/bin/env bash +cat > /dev/null +printf '%s\n' "$*" >> "$FM_SSH_LOG" +[ -z "${FM_FAKE_SSH_STDERR:-}" ] || printf '%s\n' "$FM_FAKE_SSH_STDERR" >&2 +exit "${FM_FAKE_SSH_RC:-0}" +SH + chmod +x "$fb/fake-ssh" + printf '%s\n' "$fb" +} + +setup_home() { # -> echoes a fresh home dir with an empty state/ + local home="$TMP_ROOT/$1-$RANDOM" + mkdir -p "$home/state" + printf '%s\n' "$home" +} + +# A home with a remote-secondmate task meta plus the registry row fm-on.sh +# resolves the ssh route from - the same shape a live remote mate records. +setup_remote_home() { # -> echoes home dir + local home + home=$(setup_home "$1") + mkdir -p "$home/data" + fm_write_meta "$home/state/rsm.meta" \ + "window=fm-remote:w1:p1" \ + "endpoint_task_id=rsm" \ + "harness=claude" \ + "kind=secondmate" \ + "mode=secondmate" \ + "yolo=off" \ + "remote_host=remote-mac" \ + "remote_root=/remote/root" \ + "remote_backend=herdr" \ + "remote_herdr_session=fm-remote" \ + "remote_target=fm-remote:w1:p1" + cat > "$home/data/secondmates.md" <, or empty. +pending_record() { # + find "$1/state/pending-replies" -maxdepth 1 -type f ! -name '.*' 2>/dev/null | head -1 +} + +drain_out() { # + FM_STATE_OVERRIDE="$1/state" "$DRAIN" 2>/dev/null +} + +test_remote_delivered_unconfirmed_is_not_failure() { + local dir fb log ssh_log home rc err rec + dir="$TMP_ROOT/remote-du"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log"; ssh_log="$dir/ssh.log"; : > "$ssh_log" + home=$(setup_remote_home remote-du) + + : > "$log" + env PATH="$fb:$PATH" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + FM_SSH_BIN="$fb/fake-ssh" FM_SSH_LOG="$ssh_log" FM_FAKE_SSH_RC=3 \ + FM_FAKE_SSH_STDERR='fm-send: text delivered to fm-remote:w1:p1 but submission is unconfirmed (verdict=pending; tried meta=/remote/home/state/fm-remote:w1:p1.meta; metadata window/terminal lookup; backend=herdr; endpoint=verified)' \ + "$SEND" rsm "please rename the metric" >"$dir/out" 2>"$dir/err"; rc=$? + err=$(cat "$dir/err") + expect_code 0 "$rc" "a delivered-unconfirmed remote send must not exit as a failure" + assert_grep 'fm-remote-entrypoint.sh' "$ssh_log" "the steer should cross the remote transport" + assert_contains "$err" "delivered to remote secondmate rsm" \ + "the outcome must be reported as delivered" + assert_not_contains "$err" "text not sent" "a delivered steer must not read as not sent" + assert_not_contains "$err" "not submitted" "a delivered steer must not read as not submitted" + assert_not_contains "$err" "error: text" "a delivered steer must not carry an error-styled report" + assert_not_contains "$err" "verdict=pending" \ + "the inner leg's unconfirmed diagnostics must be held back on a delivered outcome" + + rec=$(pending_record "$home") + [ -n "$rec" ] || fail "the pending-reply expectation must survive a delivered-unconfirmed send" + [ -n "$(grep '^delivered_epoch=' "$rec" | cut -d= -f2-)" ] \ + || fail "a delivered-unconfirmed send must mark the expectation delivered: $(cat "$rec")" + [ "$(grep '^phase=' "$rec" | tail -1 | cut -d= -f2-)" = awaiting_report ] \ + || fail "a delivered-unconfirmed send must leave the expectation awaiting its report: $(cat "$rec")" + pass "fm-send remote: delivered-unconfirmed reports delivered, exits 0, keeps the expectation armed" +} + +test_remote_real_failure_still_fails() { + local dir fb log ssh_log home rc err + dir="$TMP_ROOT/remote-fail"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log"; ssh_log="$dir/ssh.log"; : > "$ssh_log" + home=$(setup_remote_home remote-fail) + + : > "$log" + env PATH="$fb:$PATH" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + FM_SSH_BIN="$fb/fake-ssh" FM_SSH_LOG="$ssh_log" FM_FAKE_SSH_RC=1 \ + FM_FAKE_SSH_STDERR='error: remote secondmate rsm endpoint metadata is invalid; refusing access until it is explicitly migrated' \ + "$SEND" rsm "please rename the metric" >"$dir/out" 2>"$dir/err"; rc=$? + err=$(cat "$dir/err") + [ "$rc" -ne 0 ] || fail "a genuinely failed remote send must exit nonzero" + assert_contains "$err" "error: text not sent to remote:rsm" \ + "a real remote failure must still report a real error" + assert_contains "$err" "endpoint metadata is invalid" \ + "a real remote failure must replay the remote leg's own stderr" + [ -z "$(pending_record "$home")" ] \ + || fail "a failed send must discard its undelivered expectation" + pass "fm-send remote: a real remote failure still fails loudly with the remote diagnostics" +} + +test_remote_transport_unknown_preserves_expectation() { + local dir fb log ssh_log home rc err rec + dir="$TMP_ROOT/remote-255"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log"; ssh_log="$dir/ssh.log"; : > "$ssh_log" + home=$(setup_remote_home remote-255) + + : > "$log" + env PATH="$fb:$PATH" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + FM_SSH_BIN="$fb/fake-ssh" FM_SSH_LOG="$ssh_log" FM_FAKE_SSH_RC=255 \ + "$SEND" rsm "please rename the metric" >"$dir/out" 2>"$dir/err"; rc=$? + err=$(cat "$dir/err") + [ "$rc" -ne 0 ] || fail "an unknown-completion transport loss must exit nonzero" + assert_contains "$err" "delivery to remote secondmate rsm is unknown" \ + "transport loss must be reported as unknown delivery, not silently dropped" + rec=$(pending_record "$home") + [ -n "$rec" ] || fail "transport loss must preserve the expectation for reconciliation" + [ "$(grep '^phase=' "$rec" | tail -1 | cut -d= -f2-)" = delivery_unknown ] \ + || fail "transport loss must move the expectation to delivery_unknown: $(cat "$rec")" + pass "fm-send remote: ssh 255 still refuses loudly and preserves the expectation as delivery_unknown" +} + +test_remote_delivered_unconfirmed_closes_resolve_key() { + local dir fb log ssh_log home rc out + dir="$TMP_ROOT/remote-key"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log"; ssh_log="$dir/ssh.log"; : > "$ssh_log" + home=$(setup_remote_home remote-key) + printf 'needs-decision [key=upgrade-window]: tonight or the weekend\n' > "$home/state/rsm.status" + + : > "$log" + env PATH="$fb:$PATH" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + FM_SSH_BIN="$fb/fake-ssh" FM_SSH_LOG="$ssh_log" FM_FAKE_SSH_RC=3 \ + "$SEND" rsm --resolve-key upgrade-window "the weekend, freeze Friday" >/dev/null 2>&1; rc=$? + expect_code 0 "$rc" "a delivered-unconfirmed remote answer must not exit as a failure" + grep -F 'resolved [key=upgrade-window]: answered: the weekend, freeze Friday' "$home/state/rsm.status" >/dev/null \ + || fail "a delivered-unconfirmed remote answer must close the decision: $(cat "$home/state/rsm.status")" + out=$(drain_out "$home") + if printf '%s' "$out" | grep -F 'OPEN DECISIONS' >/dev/null; then + fail "the answered decision still lists as open after a delivered-unconfirmed answer: $out" + fi + pass "fm-send remote: a delivered-unconfirmed answer closes its --resolve-key decision" +} + +test_local_pending_reports_delivered_unconfirmed() { + local dir fb log home rc err + dir="$TMP_ROOT/local-pending"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log" + home=$(setup_home local-pending) + fm_write_meta "$home/state/t1.meta" "window=sess:fm-t1" "kind=ship" + + : > "$log" + env PATH="$fb:$PATH" FM_FAKE_TMUX_PENDING=1 \ + FM_ROOT_OVERRIDE="$home" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + "$SEND" t1 "steer text" >"$dir/out" 2>"$dir/err"; rc=$? + err=$(cat "$dir/err") + expect_code 3 "$rc" "an unconfirmed local submit must exit with the delivered-unconfirmed status" + assert_contains "$err" "submission is unconfirmed" \ + "the unconfirmed local submit must be described honestly" + assert_not_contains "$err" "not submitted" \ + "an unconfirmed local submit must not claim the text was not submitted" + assert_not_contains "$err" "error:" \ + "an unconfirmed local submit must not carry an error-styled report" + pass "fm-send local: an unconfirmed submit exits 3 with an honest non-error report" +} + +test_local_pending_does_not_close_resolve_key() { + local dir fb log home rc out + dir="$TMP_ROOT/local-pending-key"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log" + home=$(setup_home local-pending-key) + fm_write_meta "$home/state/t2.meta" "window=sess:fm-t2" "kind=ship" + printf 'blocked [key=creds]: need the deploy token\n' > "$home/state/t2.status" + + : > "$log" + env PATH="$fb:$PATH" FM_FAKE_TMUX_PENDING=1 \ + FM_ROOT_OVERRIDE="$home" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + "$SEND" t2 --resolve-key creds "token is in the vault now" >/dev/null 2>&1; rc=$? + expect_code 3 "$rc" "an unconfirmed local answer must exit with the delivered-unconfirmed status" + if grep -F 'resolved' "$home/state/t2.status" >/dev/null; then + fail "an unconfirmed local answer must not close the decision: $(cat "$home/state/t2.status")" + fi + out=$(drain_out "$home") + printf '%s' "$out" | grep -F '[key=creds]' >/dev/null \ + || fail "the blocker must stay open after an unconfirmed local answer: $out" + pass "fm-send local: an unconfirmed submit still never closes a --resolve-key decision" +} + +test_remote_delivered_unconfirmed_is_not_failure +test_remote_real_failure_still_fails +test_remote_transport_unknown_preserves_expectation +test_remote_delivered_unconfirmed_closes_resolve_key +test_local_pending_reports_delivered_unconfirmed +test_local_pending_does_not_close_resolve_key + +echo "all fm-send-remote-delivery tests passed" From d9ee8ea218d2d2693dbf260a4391589e97b544ea Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:25:17 -0700 Subject: [PATCH 07/16] feat: adopt spendPriority for quota dispatch (#2574) * Adopt quota-axi 0.1.29 spendPriority-primary array dispatch. quota-axi 0.1.29 publishes schema 5 with selection.spendPriority as the primary comparative signal and demotes derivation fields out of default --json. Rank comparable-fit candidates on that scalar, keep runway versus the completion horizon as a hard gate, and raise the compatibility floor so a pre-consolidation build cannot reach dispatch intake. * no-mistakes(review): Correct schema fixtures and remove prescriptive selection prompts * no-mistakes(document): Correct quota verification evidence chronology * Collapse quota-array-dispatch onto TOON-first spendPriority ranking. Decide from quota-axi's default TOON; keep --json as a rare defensive fallback. Rank by spendPriority after eligibility, reasoning-class, and runway-feasibility gates, and drop the hand-computed Pareto, pace, reserve, and window-id layers. * no-mistakes(review): Permit ambiguous JSON fallback and correct reset fixtures * no-mistakes(review): Correct runway semantics and escalate unresolved uncertainty * no-mistakes(document): Document TOON-first quota dispatch evidence --- .agents/skills/quota-array-dispatch/SKILL.md | 140 +++--- AGENTS.md | 8 +- bin/fm-quota-axi-lib.sh | 2 +- docs/verification/dispatch-auth.md | 70 ++- tests/fm-bootstrap.test.sh | 8 +- .../fm-quota-array-dispatch-live-e2e.test.sh | 463 +++++++++++++++++- tests/fm-secondmate-harness.test.sh | 2 +- tests/fm-secondmate-liveness.test.sh | 2 +- tests/fm-secondmate-sync.test.sh | 2 +- tests/fm-shared-captain-inheritance.test.sh | 2 +- tests/fm-startup-memory-budget.test.sh | 2 +- 11 files changed, 574 insertions(+), 127 deletions(-) diff --git a/.agents/skills/quota-array-dispatch/SKILL.md b/.agents/skills/quota-array-dispatch/SKILL.md index 11b8405812..24c0e44de5 100644 --- a/.agents/skills/quota-array-dispatch/SKILL.md +++ b/.agents/skills/quota-array-dispatch/SKILL.md @@ -2,7 +2,8 @@ name: quota-array-dispatch description: >- Agent-only decision procedure for resolving a matched crew-dispatch profile - array from current quota-axi output, including effective headroom and usable-runway evidence. + array from quota-axi's default TOON, ranking by spendPriority after three + orthogonal gates. Load when a dispatch rule or default resolves to more than one profile candidate. user-invocable: false metadata: @@ -14,43 +15,46 @@ metadata: This skill is the single owner of the completion-aware profile-array selection procedure. `AGENTS.md` section 4 owns the always-loaded intake boundary, load trigger, malformed-config refusal, every-candidate accounting, and strongest-reasoning/tie safety rules. `harness-adapters` owns harness verification, model/provider discovery, and effort fallback. -`quota-axi` remains data-only, reports whatever granularity the vendor supplies, and never recommends, selects, ranks, or infers a route. +`quota-axi` remains data-only: it publishes `spendPriority` as a comparable scalar and never recommends, selects, ranks, or infers a route. Do not add a daemon, opaque composite score, routing wrapper, hard-coded model-specific policy, or producer-side route recommendation. Deterministic shell owns only schema, configuration, and version validation plus concrete spawn safeguards; every model-to-provider, provider-to-credential, and quota-applicability relation is yours to establish transparently and to show your evidence for. -## Collect facts +## Read the default TOON -Run `quota-axi --json` once per intake and reuse that snapshot for every candidate. -Do not take a second snapshot to settle a candidate, and read `quota-axi auth --json` when a candidate's credential surface is in question. -For each candidate, preserve explicit `harness`, `model`, and `provider`; `harness-adapters` owns identity, and model/provider never infer harness: +Start each intake by running `quota-axi` once with no `--json`, and reuse that TOON for every candidate. +Post-consolidation quota-axi (the floor owned by `bin/fm-quota-axi-lib.sh`) puts `spendPriority` in the default `quota[]` block beside `effectivePercentRemaining`, `runway`, `confidence`, `limitedBy`, and `resetsAt`. +Sparse `exhaustion[]` carries finite-runway seconds only for `projected_exhaustion` and `exhausted_now`. +Sparse `attention[]` names auth, stale, and unmeasurable facts. +`spendPriority` is THE quota-perspective ranker. +It already computes the economics that older instructions reconstructed by hand from headroom, pace, reserve, and window-id lists; do not recompute those. +Do not read `--json` on the normal path, and do not reach for `--full` to rebuild that economics. -- task/profile fit and required reasoning class -- applicable effective headroom (`effectivePercentRemaining`) from the established provider/model scope -- usable runway status, `usableRunwaySeconds`, `projectedExhaustedAt`, `limitingWindowId`, `projectionConfidence`, `projectionBasis`, and any `unmeasurableWindowIds` -- the task-completion horizon and the evidence and confidence used to estimate it -- effective pace, signed reserve per window, and worst reserve (`worstReservePercentPoints` or minimum signed reserve) for later diagnostic tie-breaking -- schema notes when runway or pace fields are absent +After reading the TOON, fall back to one `quota-axi --json` call only when that TOON is genuinely ambiguous for the decision, or when the installed quota-axi is somehow below the floor so its TOON lacks `spendPriority`. +Ambiguous means a candidate's `spendPriority` is the literal `unknown` or unmeasurable, a real tie still needs extra evidence, or a candidate's eligibility is unclear from `quota[]` plus `attention[]`. +The fallback therefore has an explicit TOON-then-JSON call sequence; reuse its JSON result and do not take any further quota snapshots. +Below-floor is rare: bootstrap enforces `FM_QUOTA_AXI_MIN` and normally reports `MISSING` before dispatch; if an intake somehow reaches an older build whose TOON lacks `spendPriority`, use the defensive `--json` fallback rather than treating the missing scalar as healthy. +`--json` is a defensive belt, not a habit; never reach for it because it feels more complete. +Read `quota-axi auth --json` only when a candidate's credential surface is in question. -Stale raw windows are diagnostic, never headroom or fabricated runway. -Grok's `credits.remaining` is a prepaid balance unrelated to `percentRemaining`; never read it as exhaustion. -Read all windows named by `boundedBy`, `limitingWindowIds`, `aheadWindowIds`, `behindWindowIds`, `onPaceWindowIds`, `unknownWindowIds`, and `unmeasurableWindowIds`. -The compact default output intentionally omits numeric reserve, while `--json` and `--full` retain reserve diagnostics. +For each candidate, preserve explicit `harness`, `model`, and `provider`; `harness-adapters` owns identity, and model/provider never infer harness. -## Establish the provider relation before reading quota +## Three gates, then spendPriority + +Apply the three cheap orthogonal gates first. +`spendPriority` ranks only among candidates that pass all three. +It cannot override a hard-gate failure, and it is never hidden inside a new composite score. + +### 1. Eligibility Deterministic shell must never map a model to a provider, a provider to a credential store, or a name prefix to a family. You establish those relations yourself, in the open, from the candidate's own authoritative catalog (`harness-adapters` owns the per-harness discovery surface) plus the one intake snapshot. -Name the evidence for each relation you assert so the conclusion is inspectable. - -1. Confirm the catalog lists the candidate's model and record the provider family it reports. - A model the authoritative catalog does not list is concrete contradictory evidence: block that candidate and quote the catalog result. -2. Apply quota at the granularity the vendor actually supplies. - A provider-level or `all_models`/`all_products` scope bounds every model you established in that family, including one with no window of its own. - A named-model or named-product scope is an additional bound for that model alone and is irrelevant to every other model in the family. - Read `quotaSemantics.description`, which states the vendor's own bounding rule. -3. Record what remains unknown instead of converting it into a verdict. -## Authentication is scoped to the selected surface +Confirm the catalog lists the candidate's model and record the provider family it reports. +A model the catalog does not list is concrete contradictory evidence: block that candidate and quote the catalog result. +Apply quota at the granularity the vendor actually supplies. +A provider-level or `all_models`/`all_products` scope bounds every model you established in that family, including one with no window of its own. +A named-model or named-product scope is an additional bound for that model alone. +Match the candidate to its `quota[]` row by that established provider and scope; a stale, auth-required, or unmeasurable scope is named in `attention[]` instead of a fabricated number. A candidate authenticates through its own tuple's surface; another harness's CLI can never gate it, and `harness=pi` with `model=xai/grok-*` is Pi using xAI rather than the standalone Grok CLI. `quota-axi auth --json` lists each provider's credential sources independently, so read the one source the candidate actually uses rather than collapsing a provider to a single status. @@ -59,8 +63,8 @@ A Pi-hosted family may authenticate through the vendor's own store with no `pi:` Uncertainty and ineligibility are different findings: -- No model-level window, no matching auth source, an absent `state.authStatus`, an unmeasurable or `unknown` scope, or a surface quota-axi does not model at all is disclosed uncertainty. - Keep the candidate eligible, state the unknown, and prefer known sustainable evidence when otherwise comparable. +- No model-level window, no matching auth source, an unmeasurable or `unknown` scope, or a surface quota-axi does not model at all is disclosed uncertainty. + Keep the candidate eligible, state the unknown, and prefer known viable evidence when otherwise comparable. - An expired credential is a short-lived session token the owning vendor renews on next use, not a sign-out. - Only concrete contradictory evidence blocks: an authoritative catalog proving the model unsupported, or proof that the credential the candidate actually selects is unusable. - Reserve login wording for that proven-unusable case, and name the harness, model, surface, and evidence. @@ -69,45 +73,45 @@ When a credential's local classification is the only thing standing between a ca `bin/fm-vendor-auth-probe.sh` is the only approved vendor-credential probe; its `--help` owns the registered probes and mechanics. It takes no harness, model, or provider and returns a fact, not a route: only `authenticated` and `unauthenticated` are ground truth, while `indeterminate`, `timeout`, and `unavailable` establish nothing and must never be read as either outcome. Never launch a vendor CLI yourself, and never probe a credential store the candidate does not use. +Grok prepaid `credits` are unrelated to paid-window headroom; never read them as exhaustion. + +Malformed configuration is an actionable error, not a candidate to rank around. + +### 2. Reasoning-class fit + +Keep only candidates that meet the required reasoning class for this task (a simple bug fix versus very-difficult design). +Never use `spendPriority` or remaining quota to silently replace that class. +When every remaining candidate is tight, dispatch inside the strongest-reasoning class if one of those candidates can proceed, or stop and report that the strongest-class choice cannot proceed rather than downgrading it to spend or conserve quota. + +### 3. Runway feasibility floor + +Known runway that will not last until the inspectable likely-completion horizon fails this gate, even when that candidate has the highest `spendPriority`. +Read `runway` from the `quota[]` row: `through_reset` passes this generic feasibility floor because the window reaches its refill without exhausting; never compare its `resetsAt` with the completion horizon as though reset were an exhaustion deadline. +`exhausted_now` is zero, and `projected_exhaustion` uses the matching `exhaustion[]` row's `usableRunwaySeconds`. +A high `spendPriority` on a nearly empty window that will exhaust soon must not route into a mid-task stall. +Unknown or unmeasurable runway stays eligible with disclosed uncertainty and is never assumed to pass. +Do not invent a generic percentage floor, and honor an explicit captain floor for a candidate when one exists. + +## Rank by spendPriority + +Among candidates that pass all three gates, pick the highest known `spendPriority`. +A higher known scalar is better: positive means paid allowance is on track to reach reset unused, `0` is exact utilization, and negative means overdrawn against the reset clock. +Rank only from comparable known scalars. +Never treat absent, `unknown`, or unmeasurable `spendPriority` as zero or as healthy; `0` means exact utilization, a different claim from unknown. +An unknown `spendPriority` keeps the candidate eligible with disclosed uncertainty. +Prefer known viable evidence when otherwise comparable. +After the permitted TOON-to-JSON fallback, escalate to Firstmate instead of routing if no candidate can be ranked or runway uncertainty prevents proving the feasibility floor for any candidate that could be selected. +Never resolve that terminal uncertainty by treating unknown as healthy or by choosing arbitrarily. +Show the scalar or the literal `unknown` in the rationale; do not hide it in a score. + +Do not compare headroom against runway by hand. +Do not use pace or signed reserve as a later tie-break layer. +Do not read `aheadWindowIds`, `behindWindowIds`, `onPaceWindowIds`, `limitingWindowIds`, or other window-id lists to reconstruct what `spendPriority` already computed. + +Genuine ties: stop and report every tied candidate for captain choice. +Do not select by array order, harness name, or another arbitrary identity ordering. +Report duplicate concrete profiles as a configuration error. -## Pace semantics - -`reservePercentPoints = percentRemaining - timeRemainingPercent`. -Negative reserve means usage is ahead of reset pace and creates conservation pressure. -Positive reserve means usage is behind reset pace. -`on_pace` is neutral. -Conservation pressure is present for effective pace status `ahead`, effective pace status is `mixed` and any `aheadWindowIds` remain, or a bounding window is `ahead`. -`unknown` is valid explicit uncertainty from quota-axi, not parser failure or permission to assume health. - -## Selection order - -Apply only among candidates satisfying required fit and strongest reasoning class. -Never use headroom, runway, pace, or reserve to silently replace that reasoning class. - -1. Concrete contradictory evidence or malformed configuration: stop and report the tuple and that evidence. - Unmeasurable quota, a missing model-level window, an absent runway field, and a credential surface quota-axi does not model are uncertainty, never this rule. -2. Honor any explicit captain instruction that sets a floor for that candidate before the generic comparison. - Do not invent a generic percentage floor or treat a low percentage as an automatic failure. -3. Keep the strongest-reasoning class when every candidate is tight or completion evidence is poor. - Dispatch inside that class when a candidate can proceed, or report that its strongest-class choice cannot proceed rather than downgrading it to conserve quota. -4. Compare comparable-fit candidates on their applicable effective headroom and usable runway. - Eliminate a candidate only when another candidate Pareto-dominates it on both dimensions, with at least one dimension strictly better. - Establish dominance only from comparable known evidence, never by treating absent, `unknown`, or unmeasurable headroom or runway as zero or as a healthy value. -5. Prefer supported runway evidence that projects availability through the inspectable likely-completion horizon. - Known evidence that does not reach that horizon is inferior to known evidence that does, even when its signed reserve is less negative. - Preserve projection confidence and basis, the limiting window, and the horizon estimate in the rationale rather than hiding them in a score or model-specific heuristic. -6. Resolve remaining uncertainty explicitly. - An authenticated candidate with unknown or unmeasurable headroom or runway stays eligible and cannot be silently excluded or assumed sustainable. - Prefer known viable evidence when otherwise comparable, and report uncertainty or ask the captain when it still prevents a justified choice. -7. Use pace and signed reserve only as later diagnostic tie-break evidence among candidates still unresolved after headroom, runway, likely-completion viability, and uncertainty. - Pace and reserve never rescue a clearly inferior completion prospect. - Do not collapse these facts into an opaque composite score. -8. Older schemas or absent runway/pace fields: do not crash, fabricate runway or pace, treat absence as healthy, or silently exclude a candidate. - State which evidence is unavailable, retain the candidate, and apply only the comparisons the snapshot supports. -9. Genuine ties: stop and report every tied candidate for captain choice. - Do not select by array order, harness name, or another arbitrary identity ordering. - Report duplicate concrete profiles as a configuration error. - -Account for every candidate visibly before selecting or escalating, naming its catalog evidence, provider relation, applicable quota and authentication facts, remaining uncertainty, fit and reasoning class, effective headroom, usable runway, likely-completion reasoning, and later pace or reserve evidence when used. +Account for every candidate visibly before selecting or escalating, naming its catalog evidence, provider relation, applicable quota and authentication facts, remaining uncertainty, fit and reasoning class, `spendPriority`, and runway-versus-horizon result. A blocked credential report must name `harness`, `model`, authentication surface, and concrete failure evidence; never emit a bare `Grok unauthenticated` statement. Never conclude with an unexplained "best quota" label. diff --git a/AGENTS.md b/AGENTS.md index 6d6c955228..67ec0d6960 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -189,8 +189,8 @@ If static `config/crew-harness` or `config/secondmate-harness` names an unverifi `docs/configuration.md` owns dispatch-profile and runtime-backend schemas, `bin/fm-harness.sh` owns static resolution, and `bin/fm-spawn.sh` owns launch flags and fail-closed validation. When dispatch profiles exist, consult them at every crewmate or scout intake and pass the resolved concrete profile required by `fm-spawn`. Routing precedence is an explicit per-task captain override, then the best-fit configured rule, then the configured default, then the static crewmate harness. -Firstmate alone resolves a matched profile array: run `quota-axi --json` at that intake, evaluate every configured candidate against that current output, and choose with inspectable effective headroom and usable runway, using pace and reserve only later when needed. -Account for every candidate with the catalog evidence, provider relationship, applicable quota and authentication facts, remaining uncertainty, fit and reasoning class, and the headroom, runway, and later pace or reserve evidence used in selection; never omit a candidate, guess, fall back silently, or call the result quota-informed without them. +Firstmate alone resolves a matched profile array: begin with `quota-axi`'s default TOON at that intake, using the skill's narrow TOON-then-`--json` fallback only for genuine ambiguity, evaluate every configured candidate against that current output, and choose with inspectable `spendPriority` as the one quota-perspective ranker after the skill's eligibility, reasoning-class, and runway-feasibility gates. +Account for every candidate with the catalog evidence, provider relationship, applicable quota and authentication facts, remaining uncertainty, fit and reasoning class, and the spendPriority and runway evidence used in selection; never omit a candidate, guess, fall back silently, or call the result quota-informed without them. Establish model support and provider family from that harness's own authoritative catalog, then read `quota-axi` at the granularity the vendor actually supplies: provider-level or all-model evidence applies to every model established in that family, and a named-model window bounds only that model. Missing model-level quota, a missing authentication source, unmeasurable headroom, or unmodeled authentication is disclosed uncertainty that keeps a candidate eligible, never a credential or login escalation. Only concrete contradictory evidence blocks a candidate, such as an authoritative catalog proving the model unsupported or proof that the credential selected for that surface is unusable; never infer a credential store, provider family, or quota mapping from a harness, model, or source name, and never launch another harness's CLI to judge a candidate. @@ -198,7 +198,7 @@ Preserve malformed profile configuration as an actionable error rather than sele When every candidate is tight, preserve the captain's strongest-reasoning class rather than silently downgrading it solely to conserve quota; stop and report the tight choice if that class cannot proceed. Break genuine evidence ties without array-order or harness bias. `quota-axi` owns how model or product windows relate to bounding account windows and remains data-only. -Load `quota-array-dispatch` before choosing among a matched profile array; that skill is the single owner of the completion-aware selection procedure. +Load `quota-array-dispatch` before choosing among a matched profile array; that skill is the single owner of the TOON-first spendPriority selection procedure. The generic effort fallback and its precedence are owned by `harness-adapters`: explicit captain and standing configured effort win; otherwise use low for well-understood explicit work, xhigh for ambiguous investigation or design, intermediate levels proportionally, and never max without explicit captain preference. Do not add model-specific versions of that policy. @@ -524,7 +524,7 @@ These skills are not captain-invocable; load them only at their precise triggers - `bootstrap-diagnostics` - load whenever the session-start digest's bootstrap or network-checks section prints an actionable diagnostic line (`MISSING:`, `MISSING_MANUAL:`, `BACKEND_INVALID:`, `NEEDS_GH_AUTH`, `TANGLE:`, `STARTUP_MEMORY_BUDGET:`, `CREW_DISPATCH: invalid`, `FLEET_SYNC:`, `NETWORK_CHECKS:`, `PR_CHECK_MIGRATION:`, `SECONDMATE_SYNC:`, `SECONDMATE_LIVENESS:`, `SECONDMATE_HANDOFF:`, `NUDGE_SECONDMATES:`, or `FMX:`); silence and `BOOTSTRAP_INFO:` need no load. - `diagnostic-reasoning` - load before scoping a reported bug and before acting on a diagnostic report. - `ask-user-authority` - load before deciding any ask-user finding, regardless of the project's `yolo` posture. -- `quota-array-dispatch` - load before choosing among a matched crew-dispatch profile array from current quota-axi output. +- `quota-array-dispatch` - load before choosing among a matched crew-dispatch profile array from current quota-axi default TOON. - `harness-adapters` - load before spawning or recovering a crewmate or secondmate, handling a trust dialog, sending a harness-specific skill invocation, interrupting or exiting an agent, resuming an exited agent, or verifying a new harness adapter. - `firstmate-orca` - load before switching to Orca, spawning or supervising Orca-backed work, smoke-testing Orca backend behavior, debugging Orca task state, or reconciling Orca-backed task metadata. - `project-management` - load before adding, creating, removing, or initializing a project. diff --git a/bin/fm-quota-axi-lib.sh b/bin/fm-quota-axi-lib.sh index 7be4c99614..1f59be6792 100644 --- a/bin/fm-quota-axi-lib.sh +++ b/bin/fm-quota-axi-lib.sh @@ -9,7 +9,7 @@ # turns a failing check into the operator-facing MISSING diagnostic, which is # what keeps an older build from reaching a dispatch intake at all. -FM_QUOTA_AXI_MIN=0.1.25 +FM_QUOTA_AXI_MIN=0.1.29 fm_quota_axi_compatible() { local timeout=${1:-} output parts major minor patch extra diff --git a/docs/verification/dispatch-auth.md b/docs/verification/dispatch-auth.md index 4ef443b8a8..57772f113f 100644 --- a/docs/verification/dispatch-auth.md +++ b/docs/verification/dispatch-auth.md @@ -12,9 +12,9 @@ Credential paths below are shown with the home directory replaced by ``. ## Quota granularity the judgment depends on -Verified 2026-07-30 against quota-axi 0.1.16. - -`quota-axi --json` reports availability at whatever granularity the vendor supplies, and states the vendor's own bounding rule in `quotaSemantics.description`. +Verified 2026-07-30 against quota-axi 0.1.16 for the provider and model-scope relationships below. +That release's captured default output included `quotaSemantics.description`; the current default TOON and JSON fallback field placement are verified against 0.1.29 in the next section. +Current dispatch reads the TOON scope and `limitedBy` fields; the JSON fallback's corresponding `scope` and `boundedBy` fields preserve the same provider/model applicability without relying on the `--full`-only description. ```json { @@ -40,18 +40,27 @@ Three properties follow and are load-bearing for dispatch: `quotaSemantics.status` is `unknown` with no `effectiveAvailability` entries at all for providers whose vendor exposes no window (observed for `cursor` and `copilot`). `state.authStatus` is present only for some providers (observed for `grok` alone), so its absence is missing evidence, not a credential fault. -## Completion-runway shape the judgment depends on +## Completion-runway and selection shape the judgment depends on + +Verified 2026-08-18 against quota-axi 0.1.29 schema 5, captured from an isolated `quota-axi@0.1.29` install. +The default TOON exposed these table headers, with row counts normalized to `N`: -Verified 2026-07-31 against quota-axi 0.1.17 schema 3. -The command below records the producer shape without persisting account-specific quota values: +```text +quota[N]{provider,scope,effectivePercentRemaining,spendPriority,runway,confidence,limitedBy,resetsAt}: +exhaustion[N]{provider,scope,usableRunwaySeconds,projectedExhaustedAt,limitingWindowId}: +attention[N]{provider,scope,kind,detail,remedy}: +``` + +`exhaustion[]` and `attention[]` are sparse, so an empty table is rendered with count zero and no row fields. +The command below records the JSON fallback shape without persisting account-specific quota values: ```sh -quota-axi --json | jq '{schemaVersion, effectiveAvailabilityFields: ([.providers[]?.quotaSemantics.effectiveAvailability[]? | keys] | unique), runwayFields: ([.providers[]?.quotaSemantics.effectiveAvailability[]?.runway? | select(type == "object") | keys] | unique)}' +quota-axi --json | jq '{schemaVersion, effectiveAvailabilityFields: ([.providers[]?.quotaSemantics.effectiveAvailability[]? | keys] | unique), runwayFields: ([.providers[]?.quotaSemantics.effectiveAvailability[]?.runway? | select(type == "object") | keys] | unique), selectionFields: ([.providers[]?.quotaSemantics.effectiveAvailability[]?.selection? | select(type == "object") | keys] | unique), paceFields: ([.providers[]?.quotaSemantics.effectiveAvailability[]?.pace? | select(type == "object") | keys] | unique), windowPaceFields: ([.providers[]?.windows[]?.pace? | select(type == "object") | keys] | unique)}' ``` ```json { - "schemaVersion": 3, + "schemaVersion": 5, "effectiveAvailabilityFields": [ [ "boundedBy", @@ -60,31 +69,47 @@ quota-axi --json | jq '{schemaVersion, effectiveAvailabilityFields: ([.providers "pace", "runway", "scope", + "selection", "status" ] ], "runwayFields": [ [ - "limitingWindowId", - "projectedExhaustedAt", - "projectionBasis", "projectionConfidence", - "status", - "usableRunwaySeconds" - ], + "status" + ] + ], + "selectionFields": [ + [ + "spendPriority", + "status" + ] + ], + "paceFields": [ [ - "limitingWindowId", - "projectedExhaustedAt", "status", - "usableRunwaySeconds" + "worstReservePercentPoints", + "worstReserveWindowId" + ] + ], + "windowPaceFields": [ + [ + "burnMultiple", + "reservePercentPoints", + "status" ] ] } ``` -`runway` is nested under each effective-availability scope, so the same provider/model applicability rules govern both effective headroom and runway. -Projection confidence and basis are not present on every known runway, so selection must preserve their absence as uncertainty rather than fabricate them. -The older-schema fallback contract is owned by `quota-array-dispatch`; this evidence does not reinterpret an absent runway or pace field. +This live snapshot was all `through_reset`, so finite-runway fields were omitted. +`usableRunwaySeconds`, `projectedExhaustedAt`, and `limitingWindowId` remain in default `--json` when `runway.status` is `projected_exhaustion` or `exhausted_now`. +`selection.unmeasurableWindowIds`, scope `aheadWindowIds`/`unknownWindowIds`, and window `pace.reason` likewise remain in default `--json` when they apply. +`quotaSemantics.description`, `behindWindowIds`, `onPaceWindowIds`, and per-window cycle-progress internals are `--full` only. +There is no `projectionBasis` field; its absence means `cycle_average`. +`runway` and `selection` are nested under each effective-availability scope, so the same provider/model applicability rules govern headroom, runway, and `spendPriority`. +Projection confidence is not present on every known runway, so selection must preserve that absence as uncertainty rather than fabricate it. +The older-schema fallback contract is owned by `quota-array-dispatch`; this evidence does not reinterpret an absent runway, pace, or selection field. ## Provider-family counterfactual that this producer schema supports @@ -174,5 +199,6 @@ Re-run the two commands above and update this section and the pinned version tog It asserts that the script accepts no harness, model, or provider input, never calls `quota-axi`, exits alike for every probe result because it renders no verdict, invokes only the two fixed non-destructive argv forms with stdin closed, holds a real bound even when the configured bound is zero or malformed, and never echoes raw vendor output. `tests/fm-spawn-dispatch-profile.test.sh` owns spawn's deterministic profile and harness refusals. `tests/fm-bootstrap.test.sh` owns the quota-axi version-floor diagnostic. -`tests/fm-quota-array-dispatch-live-e2e.test.sh` drives the public Pi skill-loading interface against one fake `quota-axi --json` snapshot per case. -It covers the Claude 1 percent versus Codex 55 percent reserve regression, explicit accounting for unmeasurable runway, and the strongest-reasoning constraint. +`tests/fm-quota-array-dispatch-live-e2e.test.sh` drives the public Pi skill-loading interface against one fake schema-5 snapshot per case, served as quota-axi's default TOON. +It covers TOON-first `spendPriority` ranking among candidates that pass eligibility, reasoning-class, and runway-feasibility gates, explicit accounting for unmeasurable runway, the strongest-reasoning constraint, and the runway feasibility floor over a higher `spendPriority`. +The skill's primary path is that default TOON; `--json` is the documented defensive fallback, and this section records the producer `--json` shape that fallback consumes. diff --git a/tests/fm-bootstrap.test.sh b/tests/fm-bootstrap.test.sh index 5527d14722..1810e6b5f0 100755 --- a/tests/fm-bootstrap.test.sh +++ b/tests/fm-bootstrap.test.sh @@ -95,7 +95,7 @@ add_quota_axi() { cat > "$fakebin/quota-axi" <<'SH' #!/usr/bin/env bash if [ "${1:-}" = --version ]; then - printf '%s\n' "${FM_FAKE_QUOTA_AXI_VERSION:-0.1.25}" + printf '%s\n' "${FM_FAKE_QUOTA_AXI_VERSION:-0.1.29}" exit 0 fi exit 0 @@ -473,11 +473,11 @@ test_quota_axi_min_version() { [ "$out" = "$missing" ] || fail "$label: expected '$missing', got: $out" ;; esac done <<'ROWS' -minimum quota-axi version is accepted^0.1.25^empty -newer quota-axi patch is accepted^0.1.26^empty +minimum quota-axi version is accepted^0.1.29^empty +newer quota-axi patch is accepted^0.1.30^empty newer quota-axi minor is accepted^0.2.0^empty newer quota-axi major is accepted^1.0.0^empty -the patch just below the floor reports an upgrade^0.1.24^missing +the patch just below the floor reports an upgrade^0.1.28^missing much older quota-axi minor reports an upgrade^0.0.9^missing unparseable quota-axi version reports an upgrade^quota-axi development build^missing ROWS diff --git a/tests/fm-quota-array-dispatch-live-e2e.test.sh b/tests/fm-quota-array-dispatch-live-e2e.test.sh index 0b7f1102ab..417aeef86c 100755 --- a/tests/fm-quota-array-dispatch-live-e2e.test.sh +++ b/tests/fm-quota-array-dispatch-live-e2e.test.sh @@ -3,7 +3,9 @@ # # This drives the public Pi skill-loading interface against a fake quota-axi # executable rather than parsing instruction source bytes or recreating the -# selector in test code. +# selector in test code. The fake serves default TOON from the schema-5 JSON +# fixture; --json remains available so a TOON-first skill cannot silently +# fall back without the call log catching it. set -u if [ "${FM_QUOTA_ARRAY_DISPATCH_LIVE_E2E:-0}" != 1 ]; then @@ -20,6 +22,7 @@ fail() { } command -v pi >/dev/null 2>&1 || fail "pi not found" +command -v python3 >/dev/null 2>&1 || fail "python3 not found" [ -f "$OWNER" ] || fail "quota-array-dispatch skill not found" LAB=$(mktemp -d "${TMPDIR:-/tmp}/fm-quota-array-dispatch-live.XXXXXX") @@ -38,13 +41,118 @@ cp "$OWNER" "$PROJECT/.agents/skills/quota-array-dispatch/SKILL.md" cat > "$FAKEBIN/quota-axi" <<'SH' #!/usr/bin/env bash +# Fake quota-axi: default TOON from the schema-5 JSON fixture; --json dumps it. set -u -if [ "${1:-}" != --json ] || [ "$#" -ne 1 ]; then - printf 'unexpected quota-axi invocation: %s\n' "$*" >&2 - exit 64 -fi -printf '%s\n' "$*" >> "${QUOTA_AXI_CALLS:?}" -cat "${QUOTA_AXI_FIXTURE:?}" +record() { + printf '%s\n' "$1" >> "${QUOTA_AXI_CALLS:?}" +} +emit_toon() { + python3 - "${QUOTA_AXI_FIXTURE:?}" <<'PY' +import json +import sys + +data = json.load(open(sys.argv[1], encoding="utf-8")) +generated = data.get("generatedAt", "unknown") +quota = [] +exhaustion = [] +attention = [] + + +def join_ids(ids): + if not ids: + return "unknown" + return " + ".join(str(item) for item in ids) + + +for provider in data.get("providers") or []: + name = provider.get("provider", "unknown") + windows = {window.get("id"): window for window in (provider.get("windows") or [])} + semantics = provider.get("quotaSemantics") or {} + for scope in semantics.get("effectiveAvailability") or []: + remaining = scope.get("effectivePercentRemaining") + selection = scope.get("selection") or {} + runway = scope.get("runway") or {} + scope_name = scope.get("scope", "unknown") + if remaining is None: + attention.append( + f" {name},{scope_name},headroom_unknown,{join_ids(runway.get('unmeasurableWindowIds') or scope.get('boundedBy'))},none" + ) + continue + if selection.get("status") == "known" and "spendPriority" in selection: + spend = selection["spendPriority"] + else: + spend = "unknown" + runway_status = runway.get("status") or "unknown" + confidence = runway.get("projectionConfidence") or "unknown" + limited = join_ids(scope.get("limitingWindowIds")) + binding = None + for window_id in scope.get("limitingWindowIds") or []: + binding = (windows.get(window_id) or {}).get("resetsAt") + if binding: + break + resets_at = binding or "unknown" + quota.append( + f" {name},{scope_name},{remaining},{spend},{runway_status},{confidence},{limited},{resets_at}" + ) + if runway_status in ("projected_exhaustion", "exhausted_now"): + seconds = runway.get("usableRunwaySeconds", "unknown") + exhausted_at = runway.get("projectedExhaustedAt", "unknown") + limiting = runway.get("limitingWindowId", "unknown") + exhaustion.append( + f" {name},{scope_name},{seconds},{exhausted_at},{limiting}" + ) + blocked = [] + if runway.get("unmeasurableWindowIds"): + blocked.append(f"{join_ids(runway['unmeasurableWindowIds'])} blocks runway") + if selection.get("unmeasurableWindowIds"): + blocked.append( + f"{join_ids(selection['unmeasurableWindowIds'])} blocks spendPriority" + ) + if blocked: + attention.append( + f" {name},{scope_name},unmeasurable,{' · '.join(blocked)},none" + ) + +print('bin: fake-quota-axi') +print('description: Report local agent-provider quota windows for routing-aware agents') +print(f'generatedAt: "{generated}"') +print( + f"quota[{len(quota)}]{{provider,scope,effectivePercentRemaining,spendPriority,runway,confidence,limitedBy,resetsAt}}:" +) +print("\n".join(quota) if quota else "") +print( + f"exhaustion[{len(exhaustion)}]{{provider,scope,usableRunwaySeconds,projectedExhaustedAt,limitingWindowId}}:" + if exhaustion + else "exhaustion[0]:" +) +if exhaustion: + print("\n".join(exhaustion)) +print( + f"attention[{len(attention)}]{{provider,scope,kind,detail,remedy}}:" + if attention + else "attention[0]:" +) +if attention: + print("\n".join(attention)) +print("help[1]:") +print(" Run `quota-axi --full` for windows, pace, reserve, and account evidence") +PY +} + +case "$*" in + ""|quota) + record TOON + emit_toon + ;; + --json) + record JSON + cat "${QUOTA_AXI_FIXTURE:?}" + ;; + *) + printf 'unexpected quota-axi invocation: %s\n' "$*" >&2 + exit 64 + ;; +esac SH chmod +x "$FAKEBIN/quota-axi" @@ -53,8 +161,8 @@ write_fixture() { } run_case() { - local label=$1 expected=$2 prompt=$3 out calls required - shift 3 + local label=$1 expected=$2 expected_calls=$3 prompt=$4 out calls required + shift 4 : > "$CALLS" out=$( cd "$PROJECT" && @@ -65,7 +173,7 @@ run_case() { "$prompt" ) || fail "$label: Pi skill run failed: $out" calls=$(cat "$CALLS") - [ "$calls" = "--json" ] || fail "$label: skill did not use one quota-axi --json snapshot: $calls" + [ "$calls" = "$expected_calls" ] || fail "$label: unexpected quota-axi call sequence: $calls" printf '%s\n' "$out" | grep -Fxq "$expected" \ || fail "$label: expected final line $expected, got: $out" for required in "$@"; do @@ -77,33 +185,342 @@ run_case() { } write_fixture <<'JSON' -{"schemaVersion":3,"providers":[{"provider":"claude","quotaSemantics":{"description":"The all_models scope bounds every Claude model.","effectiveAvailability":[{"scope":"all_models","status":"known","effectivePercentRemaining":1,"boundedBy":["weekly"],"runway":{"status":"projected_exhaustion","usableRunwaySeconds":600,"projectedExhaustedAt":"2030-01-01T00:10:00Z","limitingWindowId":"weekly","projectionConfidence":"established","projectionBasis":"cycle_average"}}]},"effectivePace":[{"scope":"all_models","pace":"ahead","worstReservePercentPoints":-1}]},{"provider":"codex","quotaSemantics":{"description":"The all_models scope bounds every Codex model.","effectiveAvailability":[{"scope":"all_models","status":"known","effectivePercentRemaining":55,"boundedBy":["weekly"],"runway":{"status":"projected_exhaustion","usableRunwaySeconds":14400,"projectedExhaustedAt":"2030-01-01T04:00:00Z","limitingWindowId":"weekly","projectionConfidence":"established","projectionBasis":"cycle_average"}}]},"effectivePace":[{"scope":"all_models","pace":"ahead","worstReservePercentPoints":-40}]}]} +{ + "generatedAt": "2030-01-01T00:00:00Z", + "schemaVersion": 5, + "providers": [ + { + "provider": "claude", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 80, + "resetsAt": "2030-01-07T07:12:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -10, "burnMultiple": 2 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 80, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "known", "spendPriority": -1.1111 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 241920, + "projectedExhaustedAt": "2030-01-03T19:12:00Z", + "limitingWindowId": "weekly", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["weekly"], "worstReservePercentPoints": -10, "worstReserveWindowId": "weekly" } + } + ] + } + }, + { + "provider": "codex", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 20, + "resetsAt": "2030-01-03T19:12:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -20, "burnMultiple": 1.3333 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 20, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "known", "spendPriority": -0.8333 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 90720, + "projectedExhaustedAt": "2030-01-02T01:12:00Z", + "limitingWindowId": "weekly", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["weekly"], "worstReservePercentPoints": -20, "worstReserveWindowId": "weekly" } + } + ] + } + } + ] +} JSON run_case \ - "higher headroom and viable runway beat a less-negative reserve" \ + "higher spendPriority beats more headroom after the three gates" \ "SELECTED=codex" \ - "Resolve this matched dispatch profile array now. Load quota-array-dispatch and run quota-axi --json exactly once. Both profiles have comparable required task fit and the same strongest reasoning class. The authoritative catalogs already prove Claude/Sonnet and Codex/GPT models supported in their stated provider families, and their selected authentication surfaces are usable. The likely task-completion horizon is two hours with established confidence. Return exact lines FACT=claude|headroom=1|runway_seconds=600|reserve=-1 and FACT=codex|headroom=55|runway_seconds=14400|reserve=-40 to preserve candidate accounting, then an exact final line SELECTED=. Do not use other vendor or model commands and do not modify files." \ - "FACT=claude|headroom=1|runway_seconds=600|reserve=-1" \ - "FACT=codex|headroom=55|runway_seconds=14400|reserve=-40" + "TOON" \ + "Resolve this matched dispatch profile array now. Load quota-array-dispatch and run quota-axi with no flags (default TOON) exactly once. Do not pass --json. Both profiles have comparable required task fit and the same strongest reasoning class. The authoritative catalogs already prove Claude/Sonnet and Codex/GPT models supported in their stated provider families, and their selected authentication surfaces are usable. The likely task-completion horizon is two hours with established confidence. Both candidates have known runway that supports that horizon. Return exact lines FACT=claude|headroom=80|spendPriority=-1.1111|runway_seconds=241920 and FACT=codex|headroom=20|spendPriority=-0.8333|runway_seconds=90720 to preserve candidate accounting, then an exact final line SELECTED=. Do not use other vendor or model commands and do not modify files." \ + "FACT=claude|headroom=80|spendPriority=-1.1111|runway_seconds=241920" \ + "FACT=codex|headroom=20|spendPriority=-0.8333|runway_seconds=90720" write_fixture <<'JSON' -{"schemaVersion":3,"providers":[{"provider":"claude","quotaSemantics":{"description":"The all_models scope bounds every Claude model.","effectiveAvailability":[{"scope":"all_models","status":"known","effectivePercentRemaining":55,"boundedBy":["weekly"],"runway":{"status":"unknown","unmeasurableWindowIds":["weekly"]}}]}},{"provider":"codex","quotaSemantics":{"description":"The all_models scope bounds every Codex model.","effectiveAvailability":[{"scope":"all_models","status":"known","effectivePercentRemaining":45,"boundedBy":["weekly"],"runway":{"status":"projected_exhaustion","usableRunwaySeconds":14400,"projectedExhaustedAt":"2030-01-01T04:00:00Z","limitingWindowId":"weekly","projectionConfidence":"established","projectionBasis":"cycle_average"}}]}}]} +{ + "generatedAt": "2030-01-01T00:00:00Z", + "schemaVersion": 5, + "providers": [ + { + "provider": "claude", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 55, + "resetsAt": "2030-01-08T00:00:00Z", + "pace": { "status": "unknown", "reason": "missing_cycle" } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 55, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "unknown", "unmeasurableWindowIds": ["weekly"] }, + "runway": { "status": "unknown", "unmeasurableWindowIds": ["weekly"] }, + "pace": { "status": "unknown", "unknownWindowIds": ["weekly"] } + } + ] + } + }, + { + "provider": "codex", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 45, + "resetsAt": "2030-01-04T20:24:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -10, "burnMultiple": 1.2222 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 45, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "known", "spendPriority": -0.404 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 222676, + "projectedExhaustedAt": "2030-01-03T13:51:16Z", + "limitingWindowId": "weekly", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["weekly"], "worstReservePercentPoints": -10, "worstReserveWindowId": "weekly" } + } + ] + } + } + ] +} JSON run_case \ "unmeasurable runway stays eligible and is accounted for explicitly" \ "DECISION=CODEX" \ - "Resolve this matched dispatch profile array now. Load quota-array-dispatch and run quota-axi --json exactly once. Both profiles have comparable required task fit and the same strongest reasoning class. The authoritative catalogs already prove both models supported in their stated provider families, and their selected authentication surfaces are usable. The likely task-completion horizon is two hours with established confidence. Claude has higher known headroom but explicitly unmeasurable runway, while Codex has lower known headroom and established runway that supports completion. The snapshot cannot prove Pareto dominance in either direction, but the known completion-supporting runway justifies Codex while Claude remains eligible and its uncertainty must be disclosed. Return exact lines FACT=claude|eligible=yes|headroom=55|runway=unknown|unmeasurable=weekly and FACT=codex|eligible=yes|headroom=45|runway_seconds=14400|supports_horizon=yes, then an exact final line DECISION=CODEX. Do not use other vendor or model commands and do not modify files." \ - "FACT=claude|eligible=yes|headroom=55|runway=unknown|unmeasurable=weekly" \ - "FACT=codex|eligible=yes|headroom=45|runway_seconds=14400|supports_horizon=yes" + "TOON +JSON" \ + "Resolve this matched dispatch profile array now. Load quota-array-dispatch and consult quota-axi's default TOON first. Because Claude spendPriority is the literal unknown, use the permitted quota-axi --json fallback once before deciding. Both profiles have comparable required task fit and the same strongest reasoning class. The authoritative catalogs already prove both models supported in their stated provider families, and their selected authentication surfaces are usable. The likely task-completion horizon is two hours with established confidence. Claude has higher known headroom but explicitly unmeasurable runway and unknown spendPriority, while Codex has lower known headroom, known spendPriority, and established runway that supports completion. Claude remains eligible and its uncertainty must be disclosed. Never read unknown spendPriority as 0. Return exact lines FACT=claude|eligible=yes|headroom=55|runway=unknown|spendPriority=unknown|unmeasurable=weekly and FACT=codex|eligible=yes|headroom=45|spendPriority=-0.404|runway_seconds=222676|supports_horizon=yes, then an exact final line DECISION=CODEX. Do not use other vendor or model commands and do not modify files." \ + "FACT=claude|eligible=yes|headroom=55|runway=unknown|spendPriority=unknown|unmeasurable=weekly" \ + "FACT=codex|eligible=yes|headroom=45|spendPriority=-0.404|runway_seconds=222676|supports_horizon=yes" write_fixture <<'JSON' -{"schemaVersion":3,"providers":[{"provider":"claude","quotaSemantics":{"description":"The all_models scope bounds every Claude model.","effectiveAvailability":[{"scope":"all_models","status":"known","effectivePercentRemaining":1,"boundedBy":["weekly"],"runway":{"status":"projected_exhaustion","usableRunwaySeconds":10800,"projectedExhaustedAt":"2030-01-01T03:00:00Z","limitingWindowId":"weekly","projectionConfidence":"established","projectionBasis":"cycle_average"}}]}},{"provider":"codex","quotaSemantics":{"description":"The all_models scope bounds every Codex model.","effectiveAvailability":[{"scope":"all_models","status":"known","effectivePercentRemaining":80,"boundedBy":["weekly"],"runway":{"status":"projected_exhaustion","usableRunwaySeconds":28800,"projectedExhaustedAt":"2030-01-01T08:00:00Z","limitingWindowId":"weekly","projectionConfidence":"established","projectionBasis":"cycle_average"}}]}}]} +{ + "generatedAt": "2030-01-01T00:00:00Z", + "schemaVersion": 5, + "providers": [ + { + "provider": "claude", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 5, + "resetsAt": "2030-01-04T12:00:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -45, "burnMultiple": 1.9 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 5, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "known", "spendPriority": -1.8 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 15916, + "projectedExhaustedAt": "2030-01-01T04:25:16Z", + "limitingWindowId": "weekly", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["weekly"], "worstReservePercentPoints": -45, "worstReserveWindowId": "weekly" } + } + ] + } + }, + { + "provider": "codex", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 80, + "resetsAt": "2030-01-06T22:48:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -5, "burnMultiple": 1.3333 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 80, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "known", "spendPriority": -0.3921 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 362880, + "projectedExhaustedAt": "2030-01-05T04:48:00Z", + "limitingWindowId": "weekly", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["weekly"], "worstReservePercentPoints": -5, "worstReserveWindowId": "weekly" } + } + ] + } + } + ] +} JSON run_case \ "required strongest reasoning class is not downgraded for quota" \ "SELECTED=claude" \ - "Resolve this matched dispatch profile array now. Load quota-array-dispatch and run quota-axi --json exactly once. The likely task-completion horizon is two hours with established confidence. Claude/Sonnet is catalog-supported with usable authentication and is the only profile that meets the task's required strongest reasoning class. Codex/GPT is catalog-supported with usable authentication but is a weaker reasoning class and cannot meet the requirement. Return exact lines FACT=claude|reasoning=required|headroom=1|runway_seconds=10800 and FACT=codex|reasoning=weaker|headroom=80|runway_seconds=28800, then an exact final line SELECTED=. Do not use other vendor or model commands and do not modify files." \ - "FACT=claude|reasoning=required|headroom=1|runway_seconds=10800" \ - "FACT=codex|reasoning=weaker|headroom=80|runway_seconds=28800" + "TOON" \ + "Resolve this matched dispatch profile array now. Load quota-array-dispatch and run quota-axi with no flags (default TOON) exactly once. Do not pass --json. The likely task-completion horizon is two hours with established confidence. Claude/Sonnet is catalog-supported with usable authentication and is the only profile that meets the task's required strongest reasoning class. Codex/GPT is catalog-supported with usable authentication but is a weaker reasoning class and cannot meet the requirement. Return exact lines FACT=claude|reasoning=required|headroom=5|spendPriority=-1.8|runway_seconds=15916 and FACT=codex|reasoning=weaker|headroom=80|spendPriority=-0.3921|runway_seconds=362880, then an exact final line SELECTED=. Do not use other vendor or model commands and do not modify files." \ + "FACT=claude|reasoning=required|headroom=5|spendPriority=-1.8|runway_seconds=15916" \ + "FACT=codex|reasoning=weaker|headroom=80|spendPriority=-0.3921|runway_seconds=362880" + +write_fixture <<'JSON' +{ + "generatedAt": "2030-01-01T00:00:00Z", + "schemaVersion": 5, + "providers": [ + { + "provider": "claude", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "five_hour", + "label": "5-hour", + "kind": "five_hour", + "percentRemaining": 20, + "resetsAt": "2030-01-01T02:00:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -20, "burnMultiple": 1.3333 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 20, + "boundedBy": ["five_hour"], + "limitingWindowIds": ["five_hour"], + "selection": { "status": "known", "spendPriority": -0.8333 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 2700, + "projectedExhaustedAt": "2030-01-01T00:45:00Z", + "limitingWindowId": "five_hour", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["five_hour"], "worstReservePercentPoints": -20, "worstReserveWindowId": "five_hour" } + } + ] + } + }, + { + "provider": "codex", + "state": { "status": "fresh", "stale": false }, + "windows": [ + { + "id": "weekly", + "label": "week", + "kind": "weekly", + "percentRemaining": 5, + "resetsAt": "2030-01-04T12:00:00Z", + "pace": { "status": "ahead", "reservePercentPoints": -45, "burnMultiple": 1.9 } + } + ], + "quotaSemantics": { + "status": "known", + "effectiveAvailability": [ + { + "scope": "all_models", + "status": "known", + "effectivePercentRemaining": 5, + "boundedBy": ["weekly"], + "limitingWindowIds": ["weekly"], + "selection": { "status": "known", "spendPriority": -1.8 }, + "runway": { + "status": "projected_exhaustion", + "usableRunwaySeconds": 15916, + "projectedExhaustedAt": "2030-01-01T04:25:16Z", + "limitingWindowId": "weekly", + "projectionConfidence": "established" + }, + "pace": { "status": "ahead", "aheadWindowIds": ["weekly"], "worstReservePercentPoints": -45, "worstReserveWindowId": "weekly" } + } + ] + } + } + ] +} +JSON +run_case \ + "runway versus completion horizon remains a hard gate over spendPriority" \ + "SELECTED=codex" \ + "TOON" \ + "Resolve this matched dispatch profile array now. Load quota-array-dispatch and run quota-axi with no flags (default TOON) exactly once. Do not pass --json. Both profiles have comparable required task fit and the same strongest reasoning class. The authoritative catalogs already prove Claude/Sonnet and Codex/GPT models supported in their stated provider families, and their selected authentication surfaces are usable. The likely task-completion horizon is two hours with established confidence. Claude has known spendPriority of -0.8333 and runway of 2700 seconds. Codex has known spendPriority of -1.8 and runway of 15916 seconds. Return exact lines FACT=claude|spendPriority=-0.8333|runway_seconds=2700|supports_horizon=no and FACT=codex|spendPriority=-1.8|runway_seconds=15916|supports_horizon=yes to preserve candidate accounting, then an exact final line SELECTED=. Do not use other vendor or model commands and do not modify files." \ + "FACT=claude|spendPriority=-0.8333|runway_seconds=2700|supports_horizon=no" \ + "FACT=codex|spendPriority=-1.8|runway_seconds=15916|supports_horizon=yes" echo "# all quota-array-dispatch live behavior tests passed" diff --git a/tests/fm-secondmate-harness.test.sh b/tests/fm-secondmate-harness.test.sh index 6920cf7d12..a3fefd8bea 100755 --- a/tests/fm-secondmate-harness.test.sh +++ b/tests/fm-secondmate-harness.test.sh @@ -1107,7 +1107,7 @@ SH cat > "$fakebin/quota-axi" <<'SH' #!/usr/bin/env bash if [ "${1:-}" = --version ]; then - printf '%s\n' '0.1.25' + printf '%s\n' '0.1.29' exit 0 fi exit 0 diff --git a/tests/fm-secondmate-liveness.test.sh b/tests/fm-secondmate-liveness.test.sh index a412cce0f8..84795f6c72 100755 --- a/tests/fm-secondmate-liveness.test.sh +++ b/tests/fm-secondmate-liveness.test.sh @@ -252,7 +252,7 @@ SH cat > "$fakebin/quota-axi" <<'SH' #!/usr/bin/env bash if [ "${1:-}" = --version ]; then - printf '%s\n' '0.1.25' + printf '%s\n' '0.1.29' exit 0 fi exit 0 diff --git a/tests/fm-secondmate-sync.test.sh b/tests/fm-secondmate-sync.test.sh index 8b30696a74..41af97d1bf 100755 --- a/tests/fm-secondmate-sync.test.sh +++ b/tests/fm-secondmate-sync.test.sh @@ -359,7 +359,7 @@ SH cat > "$fakebin/quota-axi" <<'SH' #!/usr/bin/env bash if [ "${1:-}" = --version ]; then - printf '%s\n' 'quota-axi 0.1.25 (fake)' + printf '%s\n' 'quota-axi 0.1.29 (fake)' fi exit 0 SH diff --git a/tests/fm-shared-captain-inheritance.test.sh b/tests/fm-shared-captain-inheritance.test.sh index 59137b278f..904e8887b4 100755 --- a/tests/fm-shared-captain-inheritance.test.sh +++ b/tests/fm-shared-captain-inheritance.test.sh @@ -249,7 +249,7 @@ SH cat > "$fakebin/quota-axi" <<'SH' #!/usr/bin/env bash if [ "${1:-}" = --version ]; then - printf '%s\n' '0.1.25' + printf '%s\n' '0.1.29' exit 0 fi exit 0 diff --git a/tests/fm-startup-memory-budget.test.sh b/tests/fm-startup-memory-budget.test.sh index 3f6ed0624a..625444298d 100755 --- a/tests/fm-startup-memory-budget.test.sh +++ b/tests/fm-startup-memory-budget.test.sh @@ -27,7 +27,7 @@ SH cat > "$fakebin/quota-axi" <<'SH' #!/usr/bin/env bash if [ "${1:-}" = --version ]; then - printf '%s\n' 'quota-axi 0.1.25 (fake)' + printf '%s\n' 'quota-axi 0.1.29 (fake)' fi exit 0 SH From 862c532504f42d160b986bbd1eafe5b372be3db4 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 11:36:31 -0700 Subject: [PATCH 08/16] docs: add GROK_BOT.md Grok Bot system prompt (#2590) * docs: add GROK_BOT.md Grok Bot system prompt * docs: amend GROK_BOT.md with charter report-back and delegation marker * docs: classify GROK_BOT.md as public-product * docs: make GROK_BOT.md the plain Grok Bot system prompt --- GROK_BOT.md | 51 +++++++++++++++++++++++++++++++ docs/documentation-audiences.json | 4 +++ 2 files changed, 55 insertions(+) create mode 100644 GROK_BOT.md diff --git a/GROK_BOT.md b/GROK_BOT.md new file mode 100644 index 0000000000..3ec3e57b9e --- /dev/null +++ b/GROK_BOT.md @@ -0,0 +1,51 @@ +You are Firstmate: the single agent the captain talks to. They bring you +everything; you make sure it gets done. You are their one point of +contact - never make them manage a team, and every result comes back +through you, in plain language. + +Do work yourself ONLY when it takes a single tool call. Anything larger +goes to a teammate you delegate to and supervise - you orchestrate, you +don't grind through substantial work in your own chat. + +Teammate bots are your team: persistent, role-based colleagues, each +holding a stable charter - an inbox/email bot, a documents bot for PDFs +and decks, a research bot. Before creating a new teammate, check whether +an existing one already covers a related charter: if a charter matches or +highly overlaps, reuse that teammate; if the overlap is only limited, +create the new teammate and clarify the distinction in both teammates' +charters. Create a genuinely new teammate only when no existing one fits. +When you create a teammate, write into its charter that it reports its +outcomes and blockers back to you (Firstmate), never to the user +directly - the user only ever talks to you. + +Delegate by messaging a teammate. Mark every delegation as coming from you +with a short task id, and ask for the outcome back against that id - so the +teammate routes its result and any blockers to you rather than just +handling them in its own chat, and you can match a reply to the right task. +The marker is visible in the chat; that's fine. + +Software and code go through a teammate, never through you directly: +create a teammate bot per project or project area - once the captain has +expressed how its charter should be set - and let that teammate drive the +code work with cursor cloud agents. You never call a cursor cloud agent +yourself. + +Don't reach for subagents. Needing one means the work is substantial, +which means it belongs with a teammate, not with you. Subagents are a tool +for teammate bots to break down their own work. + +Work asynchronously. Delegating doesn't block you - a teammate replies on +a later turn and shows up in this chat. So hand off, tell the captain +what's in motion, and relay each result as it lands. Reserve a priority +send for when something must interrupt a teammate's current task. + +How you talk. Address the captain as "captain" at least once in every +reply - always, even when the news is bad ("Captain, that didn't work - +..."). Let light nautical seasoning land only when it fits naturally - an +occasional "aye", "on deck", "shipshape", "under way", "ahoy" - never +letting it crowd out the substance, and drop it entirely for bad news or +serious findings. Speak in outcomes and consequences, not internal +mechanics. + +Keep it simple for the captain. One agent - you. Outcomes, not mechanics. +They scale by talking only to you; protect that. diff --git a/docs/documentation-audiences.json b/docs/documentation-audiences.json index 64dea78dc6..4889467bd4 100644 --- a/docs/documentation-audiences.json +++ b/docs/documentation-audiences.json @@ -196,6 +196,10 @@ "path": "CONTRIBUTING.md", "audience": "maintainer-architecture" }, + { + "path": "GROK_BOT.md", + "audience": "public-product" + }, { "path": "README.md", "audience": "public-product" From 9d2ad81e7fa8b18f0d1129059dc1f2b905d2156c Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:07:50 -0700 Subject: [PATCH 09/16] docs: update GROK_BOT.md nautical terms and self-improvement (#2592) --- GROK_BOT.md | 62 ++++++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/GROK_BOT.md b/GROK_BOT.md index 3ec3e57b9e..f85b6f5a95 100644 --- a/GROK_BOT.md +++ b/GROK_BOT.md @@ -1,43 +1,47 @@ You are Firstmate: the single agent the captain talks to. They bring you everything; you make sure it gets done. You are their one point of -contact - never make them manage a team, and every result comes back +contact - never make them manage a crew, and every result comes back through you, in plain language. Do work yourself ONLY when it takes a single tool call. Anything larger -goes to a teammate you delegate to and supervise - you orchestrate, you +goes to a crewmate you delegate to and supervise - you orchestrate, you don't grind through substantial work in your own chat. -Teammate bots are your team: persistent, role-based colleagues, each -holding a stable charter - an inbox/email bot, a documents bot for PDFs -and decks, a research bot. Before creating a new teammate, check whether -an existing one already covers a related charter: if a charter matches or -highly overlaps, reuse that teammate; if the overlap is only limited, -create the new teammate and clarify the distinction in both teammates' -charters. Create a genuinely new teammate only when no existing one fits. -When you create a teammate, write into its charter that it reports its -outcomes and blockers back to you (Firstmate), never to the user -directly - the user only ever talks to you. - -Delegate by messaging a teammate. Mark every delegation as coming from you -with a short task id, and ask for the outcome back against that id - so the -teammate routes its result and any blockers to you rather than just -handling them in its own chat, and you can match a reply to the right task. -The marker is visible in the chat; that's fine. - -Software and code go through a teammate, never through you directly: -create a teammate bot per project or project area - once the captain has -expressed how its charter should be set - and let that teammate drive the -code work with cursor cloud agents. You never call a cursor cloud agent -yourself. +Crewmates are your crew: persistent and role-based, each holding a stable +charter - one for the inbox, one for documents like PDFs and decks, one +for research. Before signing on a new crewmate, check whether an existing +one already covers a related charter: if a charter matches or highly +overlaps, reuse that crewmate; if the overlap is only limited, sign on the +new crewmate and clarify the distinction in both crewmates' charters. Sign +on a genuinely new crewmate only when no existing one fits. When you sign +one on, write into its charter that it reports its outcomes and blockers +back to you (Firstmate), never to the captain directly - the captain only +ever talks to you. Delegate by messaging a crewmate; it wakes, does the +work, and messages you back. + +Mark every task you hand off as coming from you, with a short task id, and +ask for the outcome back against that id - so the crewmate routes its +result and any blockers to you rather than just handling them in its own +chat, and you can match a reply to the right task. The marker is visible +in the chat; that's fine. + +Software and code go through a crewmate, never through you directly: sign +on a crewmate per project or project area - once the captain has expressed +how its charter should be set - and let that crewmate drive the code work +with cursor cloud agents. You never call a cursor cloud agent yourself. Don't reach for subagents. Needing one means the work is substantial, -which means it belongs with a teammate, not with you. Subagents are a tool -for teammate bots to break down their own work. +which means it belongs with a crewmate, not with you. Subagents are a tool +for crewmates to break down their own work. -Work asynchronously. Delegating doesn't block you - a teammate replies on +Work asynchronously. Delegating doesn't block you - a crewmate replies on a later turn and shows up in this chat. So hand off, tell the captain -what's in motion, and relay each result as it lands. Reserve a priority -send for when something must interrupt a teammate's current task. +what's under way, and relay each result as it lands. Reserve a priority +send for when something must interrupt a crewmate's current task. + +When you notice crewmates making mistakes or working inefficiently, update +your own description - these standing instructions - to sharpen how you +delegate and supervise, so your crew does better next time. How you talk. Address the captain as "captain" at least once in every reply - always, even when the news is bad ("Captain, that didn't work - From f758e51f1b8ee907535ff166b123ad6e22cd09a8 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:09:20 -0700 Subject: [PATCH 10/16] doc: Update language in GROK_BOT.md for clarity Refine language for clarity and consistency in instructions. --- GROK_BOT.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/GROK_BOT.md b/GROK_BOT.md index f85b6f5a95..4fe7827e00 100644 --- a/GROK_BOT.md +++ b/GROK_BOT.md @@ -8,7 +8,7 @@ goes to a crewmate you delegate to and supervise - you orchestrate, you don't grind through substantial work in your own chat. Crewmates are your crew: persistent and role-based, each holding a stable -charter - one for the inbox, one for documents like PDFs and decks, one +charter - e.g. one for the inbox, one for documents like PDFs and decks, one for research. Before signing on a new crewmate, check whether an existing one already covers a related charter: if a charter matches or highly overlaps, reuse that crewmate; if the overlap is only limited, sign on the @@ -40,8 +40,7 @@ what's under way, and relay each result as it lands. Reserve a priority send for when something must interrupt a crewmate's current task. When you notice crewmates making mistakes or working inefficiently, update -your own description - these standing instructions - to sharpen how you -delegate and supervise, so your crew does better next time. +their description to refine their behavior so your crew does better next time. How you talk. Address the captain as "captain" at least once in every reply - always, even when the news is bad ("Captain, that didn't work - From ed66b85fe2902f5144001f61ee27c9ec14031dcd Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:47:39 -0700 Subject: [PATCH 11/16] fix(bin): preserve inactive reconciliation scan progress (#2595) * fix(bin): guarantee inactive-reconcile scan progress under second quantization The inactive-outcome scan computed its aggregate deadline in whole seconds, so a 1-second budget's effective value lands anywhere in (0,1]; a scan starting just before a wall-clock second boundary rounded its whole budget away mid-scan and exited having visited no child, while the durable cursor had already advanced past the never-examined child. This is the CI flake behind tests/fm-inactive-reconcile.test.sh's 'next bounded scan did not resume with the following child' (watcher-wake-lock family, portable serial 2, seen on the PR #2590 run). Every scan now visits at least its first due child with the per-child state-read bound floored at one second, so no invocation can be a zero-work no-op. The outer process-group kill moves to budget+1s: the scan's own deadline enforces the budget, and the kill is a backstop for a scan wedged in an unbounded wait instead of a racer that routinely preempts the clean bounded exit. The wake-lock-wait test bound tracks the backstop (3s -> 4s); the previously flaky assertion is unchanged. * no-mistakes(document): Document inactive-reconcile deadline backstop --- bin/fm-inactive-reconcile.sh | 40 ++++++++++++++++++++++++----- docs/configuration.md | 2 +- tests/fm-inactive-reconcile.test.sh | 5 +++- 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/bin/fm-inactive-reconcile.sh b/bin/fm-inactive-reconcile.sh index 79ece97a0a..30d451db5a 100755 --- a/bin/fm-inactive-reconcile.sh +++ b/bin/fm-inactive-reconcile.sh @@ -9,9 +9,17 @@ # not a watcher, daemon, PR poll, or forge client of its own. # `scan` evaluates at most once per FM_INACTIVE_RECONCILE_SECS (default 900, # valid 60..1800) per home, except that --startup performs the same cheap scan -# immediately during a locked session start. Each scan has an aggregate -# FM_INACTIVE_RECONCILE_BUDGET_SECS bound (default 10, valid 1..30) and resumes -# after its last visited child on the next scan. +# immediately during a locked session start. Each scan uses an aggregate +# FM_INACTIVE_RECONCILE_BUDGET_SECS deadline (default 10, valid 1..30) and +# resumes after its last visited child on the next scan. +# The scan enforces that budget itself through a whole-second deadline, and the +# first due child of every scan is always visited with at least a one-second +# state-read bound: whole-second arithmetic can otherwise round a small budget +# to zero mid-scan, and an invocation that exits having visited nothing would +# advance the durable cursor past a child it never examined. A process-group +# kill one second after the budget remains as a backstop for a scan wedged in +# an unbounded wait (for example a live-held wake-queue lock), so the clean +# deadline path is not racing its own backstop. # # It considers only a direct ordinary crewmate whose newest meta, status, or # turn-ended mtime is older than that interval and whose last status is not @@ -377,8 +385,13 @@ reconcile_direct_child() { # return "$rc" } +# SCAN_FIRST_VISIT_PENDING is armed by scan() before its passes. The deadline +# below is whole-second arithmetic, so a small budget can quantize to zero +# between the deadline computation and these checks; without the guaranteed +# first visit, such a scan would return 3 having examined no child at all while +# write_scan_marker had already advanced the cursor past the skipped child. scan_pass() { # - local cursor=$1 range=$2 deadline=$3 self=${4:-} meta id remaining rc + local cursor=$1 range=$2 deadline=$3 self=${4:-} meta id remaining rc first for meta in "$STATE"/*.meta; do [ -f "$meta" ] || continue id=$(basename "$meta" .meta) @@ -387,9 +400,19 @@ scan_pass() { # after) [ -z "$cursor" ] || [[ "$id" > "$cursor" ]] || continue ;; through) [ -n "$cursor" ] && [[ "$id" > "$cursor" ]] && continue ;; esac - [ "$(date +%s)" -lt "$deadline" ] || return 3 + first=0 + if [ "${SCAN_FIRST_VISIT_PENDING:-0}" -eq 1 ]; then + first=1 + SCAN_FIRST_VISIT_PENDING=0 + fi + if [ "$first" -eq 0 ]; then + [ "$(date +%s)" -lt "$deadline" ] || return 3 + fi write_scan_marker "$id" || return 1 remaining=$((deadline - $(date +%s))) + if [ "$first" -eq 1 ] && [ "$remaining" -lt 1 ]; then + remaining=1 + fi [ "$remaining" -gt 0 ] || return 3 reconcile_direct_child "$id" "$meta" "$self" "$remaining" || { rc=$? @@ -420,6 +443,7 @@ scan() { fi fi deadline=$(( $(date +%s) + FM_INACTIVE_RECONCILE_BUDGET_SECS )) + SCAN_FIRST_VISIT_PENDING=1 scan_pass "$cursor" after "$deadline" "$self" || rc=$? if [ "$rc" -eq 0 ] && [ -n "$cursor" ]; then scan_pass "$cursor" through "$deadline" "$self" || rc=$? @@ -461,7 +485,11 @@ case "$mode" in --startup) startup=1 ;; *) printf 'usage: fm-inactive-reconcile.sh scan [--startup]\n' >&2; exit 2 ;; esac - if fm_run_timed "$FM_INACTIVE_RECONCILE_BUDGET_SECS" "$0" _scan-locked "$startup"; then + # The scan's own whole-second deadline enforces the budget; this outer + # process-group kill is only the backstop for a scan wedged outside every + # bounded section (an unbounded lock wait), so it fires one second after + # the deadline instead of racing the clean bounded exit it exists to guard. + if fm_run_timed $((FM_INACTIVE_RECONCILE_BUDGET_SECS + 1)) "$0" _scan-locked "$startup"; then : elif [ "$?" -ne 124 ]; then exit 1 diff --git a/docs/configuration.md b/docs/configuration.md index 415c113991..ebb36cf7a9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -545,7 +545,7 @@ FM_POLL=15 # seconds between watcher poll cycles FM_HEARTBEAT=600 # base seconds between heartbeat scans; no-change heartbeats are absorbed while idle FM_HEARTBEAT_MAX=7200 # heartbeat backoff cap FM_INACTIVE_RECONCILE_SECS=900 # 60..1800-second watcher cadence and inactivity threshold; locked session start also scans immediately -FM_INACTIVE_RECONCILE_BUDGET_SECS=10 # 1..30-second aggregate bound per inactive-outcome scan +FM_INACTIVE_RECONCILE_BUDGET_SECS=10 # 1..30-second scan deadline; wedged-scan kill backstop follows one second later FM_CHECK_INTERVAL=300 # seconds between slow checks (authenticated merge polls, custom checks, or Relay dispatch) FM_CHECK_TIMEOUT=30 # seconds allowed per slow check script FM_PROCEVENT_MAX_OUTPUT_BYTES=1048576 # bound on one captured process-to-event result diff --git a/tests/fm-inactive-reconcile.test.sh b/tests/fm-inactive-reconcile.test.sh index dc8e06c2ed..c462119420 100755 --- a/tests/fm-inactive-reconcile.test.sh +++ b/tests/fm-inactive-reconcile.test.sh @@ -404,7 +404,10 @@ test_full_scan_budget_includes_wake_lock_wait() { FM_INACTIVE_RECONCILE_BUDGET_SECS=1 FM_FAKE_CREW_STATE='done' run_reconcile "$MAIN" --startup elapsed=$(( $(date +%s) - started )) reap "$holder" - [ "$elapsed" -le 3 ] || fail "wake lock wait exceeded aggregate scan budget (${elapsed}s)" + # The unbounded wake-lock wait is ended by the process-group backstop, which + # fires one second after the budget; the bound proves the scan cannot ride + # the 30-second lock hold. + [ "$elapsed" -le 4 ] || fail "wake lock wait exceeded aggregate scan budget (${elapsed}s)" pass "aggregate scan budget includes durable wake operations" } From 63362d2a7c8c19b857a4b1e0052eaeb89d65b798 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:06:34 -0700 Subject: [PATCH 12/16] doc: Revise Firstmate delegation and communication guidelines Refactor the guidelines for Firstmate's role and delegation process, emphasizing the importance of crewmates and asynchronous work. --- GROK_BOT.md | 81 ++++++++++++++++++----------------------------------- 1 file changed, 27 insertions(+), 54 deletions(-) diff --git a/GROK_BOT.md b/GROK_BOT.md index 4fe7827e00..3323924fd6 100644 --- a/GROK_BOT.md +++ b/GROK_BOT.md @@ -1,54 +1,27 @@ -You are Firstmate: the single agent the captain talks to. They bring you -everything; you make sure it gets done. You are their one point of -contact - never make them manage a crew, and every result comes back -through you, in plain language. - -Do work yourself ONLY when it takes a single tool call. Anything larger -goes to a crewmate you delegate to and supervise - you orchestrate, you -don't grind through substantial work in your own chat. - -Crewmates are your crew: persistent and role-based, each holding a stable -charter - e.g. one for the inbox, one for documents like PDFs and decks, one -for research. Before signing on a new crewmate, check whether an existing -one already covers a related charter: if a charter matches or highly -overlaps, reuse that crewmate; if the overlap is only limited, sign on the -new crewmate and clarify the distinction in both crewmates' charters. Sign -on a genuinely new crewmate only when no existing one fits. When you sign -one on, write into its charter that it reports its outcomes and blockers -back to you (Firstmate), never to the captain directly - the captain only -ever talks to you. Delegate by messaging a crewmate; it wakes, does the -work, and messages you back. - -Mark every task you hand off as coming from you, with a short task id, and -ask for the outcome back against that id - so the crewmate routes its -result and any blockers to you rather than just handling them in its own -chat, and you can match a reply to the right task. The marker is visible -in the chat; that's fine. - -Software and code go through a crewmate, never through you directly: sign -on a crewmate per project or project area - once the captain has expressed -how its charter should be set - and let that crewmate drive the code work -with cursor cloud agents. You never call a cursor cloud agent yourself. - -Don't reach for subagents. Needing one means the work is substantial, -which means it belongs with a crewmate, not with you. Subagents are a tool -for crewmates to break down their own work. - -Work asynchronously. Delegating doesn't block you - a crewmate replies on -a later turn and shows up in this chat. So hand off, tell the captain -what's under way, and relay each result as it lands. Reserve a priority -send for when something must interrupt a crewmate's current task. - -When you notice crewmates making mistakes or working inefficiently, update -their description to refine their behavior so your crew does better next time. - -How you talk. Address the captain as "captain" at least once in every -reply - always, even when the news is bad ("Captain, that didn't work - -..."). Let light nautical seasoning land only when it fits naturally - an -occasional "aye", "on deck", "shipshape", "under way", "ahoy" - never -letting it crowd out the substance, and drop it entirely for bad news or -serious findings. Speak in outcomes and consequences, not internal -mechanics. - -Keep it simple for the captain. One agent - you. Outcomes, not mechanics. -They scale by talking only to you; protect that. +You are Firstmate: the single agent the captain talks to. They bring you everything; you make sure it gets done. + +Other bots are your crewmates: persistent and role-based, each holding a stable charter - e.g. one for the inbox, one for documents like PDFs and decks, one for research. +Before signing on a new crewmate, check whether an existing one already covers a related charter: if a charter matches or highly overlaps, reuse that crewmate; +if the overlap is only limited, sign on the new crewmate and clarify the distinction in both crewmates' charters. +Sign on a genuinely new crewmate only when no existing one fits. When you sign one on, write into its charter that it reports its outcomes and blockers back to you (Firstmate), never to the captain directly - the captain only ever talks to you. +Delegate by messaging a crewmate; it wakes, does the work, and messages you back. + +Default to handing work off. If a job is more than one tool call, especially browser work, Search Console, Cloudflare, or anything that will take minutes, give it to the crewmate whose charter fits. Do not keep that grind in this chat because you already have a login, a token, or an open page. The computer is shared across the crew. Browser logins persist for every bot. A login on your screen is not a reason to do the work yourself. Secrets are per-bot. They do not propagate to the crew. If a crewmate needs a credential, tell the crewmate to request it and then tell the captain to give that secret to that bot on a secure card. Do not keep the secret and do the work yourself. Do not paste or forward secrets in chat. After the captain has given the secret to that bot, hand the task off and wait for the outcome. + +Software and code go through a crewmate, never through you directly: sign on a crewmate per project or project area - once the captain has expressed how its charter should be set - and let that crewmate drive the code work with cursor cloud agents. You never call a cursor cloud agent yourself. + +Don't reach for subagents. Needing one means the work is substantial, which means it belongs with a crewmate, not with you. Subagents are a tool for crewmates to break down their own work. + +Mark every task you hand off as coming from you, with a short task id, and ask for the outcome back against that id - so the crewmate routes its result and any blockers to you rather than just handling them in its own chat, and you can match a reply to the right task. +The marker is visible in the chat; that's fine. + +Work asynchronously. Delegating doesn't block you - a crewmate replies on a later turn and shows up in this chat. +So hand off, tell the captain what's under way, and relay each result as it lands. Reserve a priority send for when something must interrupt a crewmate's current task. + +When you notice crewmates making mistakes or working inefficiently, update their description to refine their behavior so your crew does better next time. + +How you talk. Address the captain as "captain" at least once in every reply - always, even when the news is bad ("Captain, that didn't work..."). +Let light nautical seasoning land only when it fits naturally - an occasional "aye", "on deck", "shipshape", "under way", "ahoy" - never letting it crowd out the substance, and drop it entirely for bad news or serious findings. +Speak in outcomes and consequences, not internal mechanics. + +Keep it simple for the captain. Focus on communicating outcomes, not mechanics. They scale by talking only to you; protect that. From 03bb1d8b78a8632ae2d9cea4c10868eb100e885e Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Tue, 18 Aug 2026 13:09:00 -0700 Subject: [PATCH 13/16] doc: Update work delegation and secret management instructions Clarified guidelines for handing off work to crewmates and managing secrets. --- GROK_BOT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GROK_BOT.md b/GROK_BOT.md index 3323924fd6..69ca686d5b 100644 --- a/GROK_BOT.md +++ b/GROK_BOT.md @@ -6,7 +6,7 @@ if the overlap is only limited, sign on the new crewmate and clarify the distinc Sign on a genuinely new crewmate only when no existing one fits. When you sign one on, write into its charter that it reports its outcomes and blockers back to you (Firstmate), never to the captain directly - the captain only ever talks to you. Delegate by messaging a crewmate; it wakes, does the work, and messages you back. -Default to handing work off. If a job is more than one tool call, especially browser work, Search Console, Cloudflare, or anything that will take minutes, give it to the crewmate whose charter fits. Do not keep that grind in this chat because you already have a login, a token, or an open page. The computer is shared across the crew. Browser logins persist for every bot. A login on your screen is not a reason to do the work yourself. Secrets are per-bot. They do not propagate to the crew. If a crewmate needs a credential, tell the crewmate to request it and then tell the captain to give that secret to that bot on a secure card. Do not keep the secret and do the work yourself. Do not paste or forward secrets in chat. After the captain has given the secret to that bot, hand the task off and wait for the outcome. +Default to handing work off. If a job is more than one tool call, especially computer or browser work or anything that will take minutes, give it to the crewmate whose charter fits. Do not keep that grind in this chat because you already have a login, a token, or an open page. The computer is shared across the crew. Browser logins persist for every bot. A login on your screen is not a reason to do the work yourself. Secrets are per-bot. They do not propagate to the crew. If a crewmate needs a credential, tell the crewmate to request it and then tell the captain to give that secret to that bot on a secure card. Do not keep the secret and do the work yourself. Do not paste or forward secrets in chat. After the captain has given the secret to that bot, hand the task off and wait for the outcome. Software and code go through a crewmate, never through you directly: sign on a crewmate per project or project area - once the captain has expressed how its charter should be set - and let that crewmate drive the code work with cursor cloud agents. You never call a cursor cloud agent yourself. From c54c448e582720f21450b586903a0f2982847a4d Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:15:45 -0700 Subject: [PATCH 14/16] test(procevent): make the process-event suite's detached-runner assertions deterministic (#2617) Three assertions in tests/fm-procevent.test.sh depended on a detached runner having finished work that the command starting it does not wait for. reconcile's replacement runner is started through detach_runner, which only forks: reconcile returns and counts the start before that runner has claimed its source or exec'd its child. Any assertion taken straight after reconcile therefore samples a race. - The publish-before-apply recovery section left its always-ready /bin/echo source registered across the recovery reconcile, so that reconcile launched a competing detached poll (observed: started=1) that then raced every later assertion for the source claim, the next capture sequence, and this home's applied record, and outlived the section holding a live claim. It is now retired before that reconcile - re-announcement is proven from the durable inbox alone and needs no registration - and started=0 is asserted so a competing poll cannot be reintroduced unnoticed. This is the same retire-before-reconcile discipline the self-announcing section already carries; that section acquired it after the identical race made its "not-autohandled: self-src" assertion read "already owned: self-src". - The crashed-leader replacement section snapshotted the replacement's claim file and execution log behind a fixed 0.5s settle window. On a loaded machine that window expires first, which is the CI flake behind "a replacement runner started without recording its own claim" and "reconcile did not start exactly one replacement source". Both effects are now waited for with the suite's bounded wait helpers; the exact one-replacement count is still asserted afterwards, unchanged. - The duplicate-start section slept 0.5s for reconcile's runner to record ownership before asserting that a second start loses to it. It now waits for that claim. Also tighten one assertion that could not fail as written: "autohandled: self-src" is a substring of "not-autohandled: self-src", so the applied path was accepted even when the runner reported the capture left for the handler. Evidence: on the unmodified suite, 128 full runs at 6-8x concurrency produced 6 failing runs, all in the crashed-leader section. On the fixed suite, 216 full runs under the same load produced none. Reverting the self-announcing section's retire-before-reconcile line reproduces "already owned: self-src" on the first iteration, confirming the shared mechanism. --- tests/fm-procevent.test.sh | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tests/fm-procevent.test.sh b/tests/fm-procevent.test.sh index f92cc198b5..878f71ac81 100755 --- a/tests/fm-procevent.test.sh +++ b/tests/fm-procevent.test.sh @@ -87,6 +87,21 @@ wait_for() { # [tries] return 1 } +# [tries]: wait until holds at least lines. A +# detached runner appends its execution marker after the command that started it +# has already returned, so a caller that needs that append must wait for it +# rather than assume a fixed settle window covered it on a loaded machine. +wait_for_lines() { + local f=$1 want=$2 n=${3:-100} have + for _ in $(seq 1 "$n"); do + have=$(wc -l < "$f" 2>/dev/null | tr -d ' ') + case "$have" in ''|*[!0-9]*) have=0 ;; esac + [ "$have" -ge "$want" ] && return 0 + sleep 0.1 + done + return 1 +} + hold_source_lock() { # local id=$1 ready=$2 release=$3 parent=$$ FM_HOME="$TMP_ROOT/lock-helper-home" bash -c ' @@ -146,7 +161,10 @@ sup=$(PATH="${FM_TEST_BASE_PATH:-/usr/bin:/bin:/usr/sbin:/sbin}" bash -c \ assert_contains "$sup" yes "a registered source needs supervision with no task metadata" pe "$H1" reconcile >/dev/null -sleep 0.5 +# Reconcile's replacement runner is detached, so ownership is recorded after +# reconcile has already returned. Wait for the claim itself: a duplicate start +# only has an owner to lose to once that claim exists. +wait_for "$FM_PROCEVENT_CLAIM_ROOT/src-one.claim" || fail "reconcile never claimed the registered source" out=$(pe "$H1" start src-one) assert_contains "$out" "already owned" "a duplicate start loses instead of running a second child" @@ -383,8 +401,16 @@ assert_contains "$out" "not-autohandled: publish-src" "failed publication did no assert_absent "$HPUBLISH/state/applied" "a result was applied before its wake was durably published" assert_absent "$HPUBLISH/state/procevent-inbox/publish-src.1.handled" "a result was acknowledged before its wake was durably published" rmdir "$HPUBLISH/state/.wake-queue" +# This source's child returns instantly, so leaving it registered would have the +# recovery reconcile below start a detached poll that races every assertion after +# it for the source claim, the next sequence, and this home's applied record. +# Re-announcement is proven from the durable inbox alone and needs no +# registration, so retire it first - the same retire-before-reconcile discipline +# the blocker-backed sources rely on - and prove no competing poll was started. +pe_adapter "$HPUBLISH" retire publish-src >/dev/null out=$(pe_adapter "$HPUBLISH" reconcile) assert_contains "$out" "published=1" "the unpublished capture was not announced on later reconciliation" +assert_contains "$out" "started=0" "reconcile started an always-ready poll that races the recovery assertions" assert_contains "$(wake_payloads "$HPUBLISH")" "procevent applying publish-src 1" "later reconciliation did not deliver the capture to a handler" FM_HOME="$HPUBLISH" FM_PROCEVENT_UNDER_TEST="$ROOT/bin/fm-procevent.sh" \ "$ADAPTER_ROOT/bin/fm-procevent-applying.sh" autohandle publish-src 1 \ @@ -403,6 +429,7 @@ PE_TRACKED+=("$HSELF|self-src") pe_adapter "$HSELF" register selfann self-src -- /bin/echo "self announced" >/dev/null out=$(pe_adapter "$HSELF" start self-src 2>&1) assert_contains "$out" "autohandled: self-src" "the self-announcing adapter did not apply its own capture" +assert_not_contains "$out" "not-autohandled" "the applied capture was still reported as left for the handler" assert_grep 'self-src 1' "$HSELF/state/applied" "the self-announcing capture was not applied" assert_present "$HSELF/state/procevent-inbox/self-src.1.handled" "the self-announcing application was not acknowledged" if [ -e "$HSELF/state/.wake-queue" ] && grep -q 'procevent selfann self-src 1' "$HSELF/state/.wake-queue"; then @@ -795,8 +822,13 @@ sleep 0.5 assert_absent "$ORPHAN_OVERLAP" "no replacement source starts while the crashed generation remains alive" case "$orphan_out" in *"started=1"*) - [ -e "$FM_PROCEVENT_CLAIM_ROOT/orphan-src.claim" ] \ + # The replacement is detached: it records its own claim and execs its source + # after reconcile has already returned, so both effects must be waited for + # rather than snapshotted behind the settle window above. + wait_for "$FM_PROCEVENT_CLAIM_ROOT/orphan-src.claim" \ || fail "a replacement runner started without recording its own claim" + wait_for_lines "$ORPHAN_LOG" 2 \ + || fail "the replacement runner never started its source: $(cat "$ORPHAN_LOG")" [ "$(wc -l < "$ORPHAN_LOG" | tr -d ' ')" = 2 ] \ || fail "reconcile did not start exactly one replacement source: $(cat "$ORPHAN_LOG")" ;; From 7f5255a3447fc5bd09ae3e9ad4d1c06a4e5a9d07 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:15:50 -0700 Subject: [PATCH 15/16] fix: preserve pending replies and defer remote reposts (#2618) * fix(bin): keep pending-reply expectations honest on both send legs Two related asymmetries let the parent-owned secondmate reply guard drop or nag requests it should not have. Local delivered-unconfirmed dropped the expectation. A marked request whose submit read-back stayed unconfirmed (verdict=pending) is the same not-a-failure outcome the remote leg reports as delivered, but fm-send discarded the parent's pending-reply record for it, so a request that very likely landed stopped being tracked entirely. The record now stays armed on its unconfirmed-delivery marker: a correlated report still resolves it, and an unanswered one still surfaces through the library's own reconciliation. Exit 3 and the local rule that an unconfirmed answer never closes a decision key are unchanged. Remote replies were nagged for a repost they did not need. A remote mate's report reaches the parent's status log only through the asynchronous mirror in fm-procevent-remote-reply.sh, yet the guard read an absent correlated line as proof the mate never reported - even while the answer was still in flight, which is the common case because the mirror's poll window is comparable to the recovery grace. The mirror now publishes one caught-up watermark from a quiet window, and the guard admits a missing report as evidence only once that watermark passes the turn that should have produced it. A genuinely missed report still gets exactly one repost, and a channel that is behind, unarmed, or broken leaves the request durably open and un-nagged rather than nagging blind; the mirror escalates its own continuity failures as before. Tests: a local unconfirmed secondmate send keeps its expectation armed and resolvable; a mirrored correlated remote reply resolves with no repost; a stale or absent watermark withholds the repost while a fresh one still releases it; a quiet remote window publishes the watermark and retirement clears it. * no-mistakes(review): Distinguish preempted polls from quiet windows * no-mistakes(document): Clarify remote reply channel freshness * no-mistakes(lint): Annotate shared remote preemption exit constant --- bin/fm-pending-reply-lib.sh | 77 +++++++++++++++++++++ bin/fm-procevent-remote-reply.sh | 25 ++++++- bin/fm-remote-delta-read.sh | 6 +- bin/fm-remote-job-lib.sh | 10 +-- bin/fm-remote-job-worker.sh | 2 +- bin/fm-send.sh | 23 ++++--- docs/remote-secondmates.md | 4 +- tests/fm-pending-reply.test.sh | 96 +++++++++++++++++++++++++++ tests/fm-remote-job.test.sh | 3 +- tests/fm-remote-reply.test.sh | 49 ++++++++++++++ tests/fm-send-remote-delivery.test.sh | 29 ++++++++ 11 files changed, 303 insertions(+), 21 deletions(-) diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index a06cba5f8c..5453585d0e 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -19,6 +19,9 @@ # # Record location (parent FM_HOME): # state/pending-replies/ +# One more durable input, owned by bin/fm-procevent-remote-reply.sh and read +# here: state/remote-replies/.caught-up, the remote reply mirror's +# watermark (see the remote reply-channel freshness section below). # Each record is a key=value file owned by this library. Schema: # schema=fm-pending-reply.v1 # corr_id= privacy-safe correlation token @@ -698,6 +701,74 @@ fm_pending_reply_mark_turn_completed() { # [which: reques return 0 } +# --- remote reply-channel freshness ----------------------------------------- +# +# A LOCAL secondmate appends its report straight into the parent's +# state/.status, so an absent correlated line there is immediate evidence +# that no report was written. A REMOTE mate's reports reach that same file only +# through the asynchronous mirror in bin/fm-procevent-remote-reply.sh, so the +# same absence proves nothing until that mirror has actually been read past the +# turn that should have produced the report. Without this distinction the guard +# nags a REPOST REQUIRED for a reply the mate did write and the parent simply +# had not received yet - the common case, because the mirror's poll window is +# comparable to the recovery grace. +# +# The mirror therefore publishes one watermark: the epoch at which it last knew +# it had read the remote log through its end. Only that adapter writes it (it +# owns the channel), and only this library reads it. A channel that is behind, +# unarmed, or broken simply never advances the watermark, so the request stays +# durably open and un-nagged; the mirror escalates its own continuity failures. +fm_pending_reply_remote_channel_watermark_path() { # + printf '%s/remote-replies/%s.caught-up' "$1" "$2" +} + +# Record that the mirrored remote reply log for was read through its +# end at (default now). Called only by the remote reply adapter. +fm_pending_reply_note_remote_channel_caught_up() { # [epoch] + local state=$1 task_id=$2 epoch=${3-} path dir tmp + [ -n "$state" ] && [ -n "$task_id" ] || return 2 + case "$epoch" in ''|*[!0-9]*) epoch=$(fm_pending_reply_now) ;; esac + path=$(fm_pending_reply_remote_channel_watermark_path "$state" "$task_id") + dir=$(dirname "$path") + mkdir -p "$dir" || return 1 + chmod 700 "$dir" 2>/dev/null || true + [ ! -L "$path" ] || return 1 + tmp="$dir/.caught-up.$task_id.$$" + printf 'caught_up_epoch=%s\n' "$epoch" > "$tmp" || { rm -f -- "$tmp"; return 1; } + chmod 600 "$tmp" 2>/dev/null || true + mv -f -- "$tmp" "$path" +} + +# Print the watermark epoch, or nothing when the channel never reported itself +# caught up. Never invents a value. +fm_pending_reply_remote_channel_epoch() { # + local path epoch + path=$(fm_pending_reply_remote_channel_watermark_path "$1" "$2") + [ -f "$path" ] && [ ! -L "$path" ] || return 0 + epoch=$(sed -n 's/^caught_up_epoch=//p' "$path" 2>/dev/null | head -1) + case "$epoch" in ''|*[!0-9]*) return 0 ;; esac + printf '%s' "$epoch" +} + +# 0 when is a secondmate whose reports cross a machine boundary. +fm_pending_reply_target_is_remote() { # + local meta="$1/$2.meta" + [ -f "$meta" ] || return 1 + [ -n "$(fm_meta_get "$meta" remote_host)" ] +} + +# 0 when "no correlated report in the parent status log" is admissible evidence +# that the mate never reported: always for a local target, and for a remote one +# only once the mirror has been read through its end at or after . +fm_pending_reply_missing_report_is_evidence() { # + local state=$1 task_id=$2 since=$3 caught + fm_pending_reply_target_is_remote "$state" "$task_id" || return 0 + case "$since" in ''|*[!0-9]*) return 1 ;; esac + caught=$(fm_pending_reply_remote_channel_epoch "$state" "$task_id") + [ -n "$caught" ] || return 1 + [ "$caught" -ge "$since" ] +} + # Build the one automatic recovery message for a pending record. fm_pending_reply_recovery_message() { # local rec=$1 corr summary token msg @@ -736,6 +807,8 @@ fm_pending_reply_send_recovery() { # age=$((now - delivered)) [ "$age" -ge "$grace" ] || return 1 task_id=$(fm_pending_reply_get "$rec" task_id) + # A remote mate's report may exist and simply not have been mirrored yet. + fm_pending_reply_missing_report_is_evidence "$state" "$task_id" "$completed" || return 1 parent_home=$(fm_pending_reply_get "$rec" parent_home) msg=$(fm_pending_reply_recovery_message "$rec") sender_pid=${BASHPID:-$$} @@ -984,6 +1057,10 @@ _fm_pending_reply_maybe_escalate_locked() { # recovery_sent) completed=$(fm_pending_reply_get "$rec" recovery_turn_completed_epoch) [ -n "$completed" ] || return 1 + # Same reply-channel evidence rule the recovery repost obeys: a missing + # correlated report is not a missed report until the mirror caught up. + fm_pending_reply_missing_report_is_evidence "$state" \ + "$(fm_pending_reply_get "$rec" task_id)" "$completed" || return 1 ;; delivery_unknown|recovery_failed|recovery_unknown) ;; *) return 1 ;; diff --git a/bin/fm-procevent-remote-reply.sh b/bin/fm-procevent-remote-reply.sh index ca816541df..abba201a6d 100755 --- a/bin/fm-procevent-remote-reply.sh +++ b/bin/fm-procevent-remote-reply.sh @@ -56,6 +56,10 @@ # - at-most-once append, because a captured generation can be replayed # - control-byte normalization, so content-bearing bytes from another machine # cannot make the parent's status file unsafe to read +# - the caught-up watermark this channel publishes for +# bin/fm-pending-reply-lib.sh, because a report that exists remotely but has +# not been mirrored yet must not be mistaken for a report the mate never +# wrote (see WINDOW_CLOSED_EMPTY below) # Line framing and size bounding belong to bin/fm-remote-delta-read.sh, which # delivers only whole lines and breaks continuity on an over-long one. set -u @@ -235,12 +239,26 @@ cmd_arm() { ) } +# The reader's exit when its wait window closed with no complete new line. That +# is the one moment this channel can prove it is not behind: the window opened +# with the remote log matching the committed cursor exactly (any pending bytes +# would have returned a delta at once), so the parent had read that log through +# its end at window START. The window start, not its close, is therefore the +# honest watermark, and bin/fm-pending-reply-lib.sh consumes it so a missing +# correlated report is judged only against a channel known to have caught up. +WINDOW_CLOSED_EMPTY=75 + cmd_source() { - local id=${1:-} + local id=${1:-} started rc=0 validate_id "$id" read_cursor "$id" - exec "$SCRIPT_DIR/fm-on.sh" "$id" fm-remote-delta-read.sh \ - "$REMOTE_LOG" "$CURSOR_OFFSET" "$CURSOR_HASH" "$WAIT_SECONDS" < /dev/null + started=$(fm_pending_reply_now) + "$SCRIPT_DIR/fm-on.sh" "$id" fm-remote-delta-read.sh \ + "$REMOTE_LOG" "$CURSOR_OFFSET" "$CURSOR_HASH" "$WAIT_SECONDS" < /dev/null || rc=$? + if [ "$rc" -eq "$WINDOW_CLOSED_EMPTY" ]; then + fm_pending_reply_note_remote_channel_caught_up "$STATE" "$id" "$started" || true + fi + return "$rc" } safe_doc_path() { @@ -512,6 +530,7 @@ cmd_retire_finalize_locked() { fi rm -f -- "$(cursor_path "$id")" rm -f -- "$CURSOR_DIR/$id".*.ingested + rm -f -- "$(fm_pending_reply_remote_channel_watermark_path "$STATE" "$id")" } cmd_retire() { diff --git a/bin/fm-remote-delta-read.sh b/bin/fm-remote-delta-read.sh index 73e90bb795..d4c26bd669 100755 --- a/bin/fm-remote-delta-read.sh +++ b/bin/fm-remote-delta-read.sh @@ -12,9 +12,9 @@ # # Exit 75 means the wait window closed with no complete line. SIGTERM exits the # same way after cleanup. The remote job worker preempts this read-only poll to -# unblock any queued command other than another reply long-poll. The -# bin/fm-remote-job-lib.sh header owns that contract, and a preempted read is -# indistinguishable from an empty window. +# unblock any queued command other than another reply long-poll, then publishes +# that preemption as distinct exit 76. The bin/fm-remote-job-lib.sh header owns +# that contract. set -eu FM_HOME=${FM_HOME:?FM_HOME is required} diff --git a/bin/fm-remote-job-lib.sh b/bin/fm-remote-job-lib.sh index 0af1f5aea8..73bffa54c7 100755 --- a/bin/fm-remote-job-lib.sh +++ b/bin/fm-remote-job-lib.sh @@ -20,10 +20,10 @@ # fm_remote_job_command_preemptible names the read-only long-poll class # (fm-remote-delta-read.sh, the reply-log delta read). The worker preempts a # running preemptible job as soon as a non-preemptible job is queued and -# publishes exit 75 with emptied stdout and stderr, identical to the poll's own -# elapsed-window-with-no-data result. The delta read is non-destructive and -# cursor-anchored, so the caller's normal re-arm re-reads the same data and a -# preempted poll loses nothing. +# publishes exit 76 with emptied stdout and stderr, distinct from the poll's +# exit 75 elapsed-window-with-no-data result. The delta read is non-destructive +# and cursor-anchored, so the caller's normal re-arm re-reads the same data and +# a preempted poll loses nothing. # # The worker accepts only a tracked, non-symlink executable named fm-*.sh below # its configured FM_ROOT/bin. Every child receives env -i with the composed @@ -57,6 +57,8 @@ FM_REMOTE_JOB_TIMEOUT=${FM_REMOTE_JOB_TIMEOUT:-360} FM_REMOTE_JOB_WAIT_GRACE=${FM_REMOTE_JOB_WAIT_GRACE:-30} FM_REMOTE_JOB_POLL_SECONDS=${FM_REMOTE_JOB_POLL_SECONDS:-0.05} FM_REMOTE_JOB_REAP_SECONDS=${FM_REMOTE_JOB_REAP_SECONDS:-3600} +# shellcheck disable=SC2034 # Shared protocol constant consumed by the worker and sourcing callers. +FM_REMOTE_JOB_PREEMPTED_EXIT=76 FM_REMOTE_JOB_OPERATOR_PATH= FM_REMOTE_JOB_CHILD_PATH= FM_REMOTE_JOB_STATE= diff --git a/bin/fm-remote-job-worker.sh b/bin/fm-remote-job-worker.sh index 6046fdda36..2a49dd6694 100755 --- a/bin/fm-remote-job-worker.sh +++ b/bin/fm-remote-job-worker.sh @@ -485,7 +485,7 @@ worker_run_with_timeout() { # [args...] WORKER_ACTIVE_JOB= [ "$timed_out" -eq 0 ] || return 124 [ "$heartbeat_failed" -eq 0 ] || return 125 - [ "$WORKER_PREEMPTED" -eq 0 ] || return 75 + [ "$WORKER_PREEMPTED" -eq 0 ] || return "$FM_REMOTE_JOB_PREEMPTED_EXIT" return "$rc" } diff --git a/bin/fm-send.sh b/bin/fm-send.sh index 1da45d86f4..cc199c9b01 100755 --- a/bin/fm-send.sh +++ b/bin/fm-send.sh @@ -19,8 +19,9 @@ # target, delivered with confirmation pending - see the remote paragraph); # 3 = the text was typed into the live endpoint and Enter was sent, but the # submit read-back stayed unconfirmed (verify the pane before any resend, and -# never re-type blindly); any other nonzero = the send failed and nothing may -# be assumed delivered. +# never re-type blindly; a marked request's pending-reply expectation stays +# armed because this outcome is not a proven failure); any other nonzero = the +# send failed and nothing may be assumed delivered. # Submission dispatches through the target's recorded backend; the tmux adapter # shares its composer/submit core with the away-mode daemon via bin/fm-tmux-lib.sh. # Tune with FM_SEND_RETRIES (default 3) / FM_SEND_SLEEP (0.4). @@ -39,9 +40,11 @@ # also receives a privacy-safe correlation id and a durable parent record under # state/pending-replies/ before delivery (bin/fm-pending-reply-lib.sh). Delivery # success and reply success are separate facts: a successful submit never -# resolves the expectation. Set FM_PENDING_REPLY_EXISTING_CORR= when -# re-sending a recovery request for an already-open expectation so a second -# record is not created. Direct unmarked captain input never creates one. +# resolves the expectation, and an unconfirmed submit (exit 3) keeps it armed +# rather than dropping it; only a proven send failure discards it. Set +# FM_PENDING_REPLY_EXISTING_CORR= when re-sending a recovery request for an +# already-open expectation so a second record is not created. Direct unmarked +# captain input never creates one. # # Remote secondmate delivery: the send crosses fm-on.sh to a host-local leg # (bin/fm-remote-secondmate-control.sh cmd_send) that runs this same verified @@ -615,9 +618,13 @@ else # re-type the message: verify the pane instead. Exit 3 is the documented # delivered-unconfirmed status, and the remote send leg above depends on # it crossing the ssh boundary intact. - if [ "$PENDING_REPLY_CREATED" = 1 ] && [ -n "$PENDING_REPLY_CORR" ]; then - fm_pending_reply_discard_undelivered "$STATE" "$PENDING_REPLY_CORR" || true - fi + # The pending-reply expectation is deliberately NOT discarded here: this + # is the same not-a-failure outcome the remote leg reports as delivered, + # so dropping it would silently stop tracking a marked request that very + # likely landed. It stays armed on its unconfirmed-delivery marker, so a + # correlated report still resolves it and an unanswered one still + # surfaces through the library's own reconciliation + # (bin/fm-pending-reply-lib.sh). echo "fm-send: text delivered to $T but submission is unconfirmed (verdict=pending; tried $RESOLUTION_TRIED); do not retype or blindly resend - verify with fm-peek.sh, then re-send '--key Enter' only if the composer still holds the text" >&2 exit 3 ;; diff --git a/docs/remote-secondmates.md b/docs/remote-secondmates.md index a1560e20b5..c5f471875d 100644 --- a/docs/remote-secondmates.md +++ b/docs/remote-secondmates.md @@ -33,7 +33,7 @@ After setup, every other command verifies Firstmate's account-owned remote job w On macOS the worker is `dev.firstmate.remote-job`, an Aqua-scoped LaunchAgent at `~/Library/LaunchAgents/dev.firstmate.remote-job.plist` with logs under `~/Library/Logs/`. After that bootstrap every non-doctor `fm-on.sh` target runs through that worker in the remote account's GUI session, never in the SSH process or a Herdr pane. The worker runs one staged job at a time and preempts a running reply long-poll as soon as any command other than another reply long-poll is queued, so interactive commands and startup checks are never serialized behind a poll window. -`bin/fm-remote-job-lib.sh` owns that preemption contract, and a preempted poll is indistinguishable from one whose wait window closed with no data, so the re-armed poll loses nothing. +`bin/fm-remote-job-lib.sh` owns that preemption contract and distinguishes preemption from a wait window that closes with no data, so only a genuinely quiet window proves channel freshness while either outcome can re-arm without losing data. Linux uses the same queue and worker protocol without the Aqua-session requirement. A worker stops itself once its configured code root stops being a Firstmate checkout, so a worker started from a worktree cannot outlive that worktree, and `bin/fm-remote-job-reap-orphans.sh` clears any worker already left behind that way without ever touching one whose checkout still exists. The remote account must provide the required toolchain, the selected worker runtime, the selected session backend, and credentials that work on that host. @@ -183,6 +183,8 @@ If the confined remote reader permanently refuses a referenced document, the mat An SSH exit status of 255 while fetching a referenced document leaves the delta uncommitted for the process-event runner's normal retry because remote completion is unknown. The process-event runner applies each captured delta through this adapter as soon as it is captured, so a mirrored reply reaches the primary status channel without depending on the wake handler running the adapter itself. A mirrored line that carries a correlation token settles its pending-reply record and closes that request's own open escalation decision. +Because a remote reply reaches the primary only through this asynchronous mirror, the primary treats a missing correlated report as a missed report only once the mirror has been read through the end of the remote log after that turn ended. +A remote mate that did answer is therefore never asked to repost while its answer is still in flight, and a genuinely missing answer still gets exactly one repost once the mirror is known to be current. The [process-to-event operating contract](configuration.md#process-to-event-sources-stateprocevent) owns automatic application, one-announcement replay deduplication, and the unhandled fallback path. The source log is never truncated or consumed. A shortened or changed prefix stops the relay and surfaces a continuity failure instead of silently resetting the cursor. diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 793b8454b1..4457ae6bb7 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -19,6 +19,9 @@ # 10. fm-send secondmate path embeds corr and creates durable pending records # 11. Backend busy/idle observation works through the shared busy abstraction # used by Pi/Claude secondmate backends (no conversation scrape) +# 12. A remote mate's repost waits for its asynchronous reply mirror to be read +# past the turn, so a mirrored reply is never nagged and a real miss still +# gets its one repost set -u # shellcheck source=tests/lib.sh @@ -1066,6 +1069,97 @@ test_tick_end_to_end_missed_then_escalate() { pass "tick end-to-end: miss -> one recovery -> escalate -> durable" } +test_remote_repost_waits_for_the_reply_channel() { + local home state corr hook_log rec lines + home=$(setup_parent remote-repost) + state="$home/state" + hook_log="$TMP_ROOT/remote-repost.log" + : > "$hook_log" + export FM_PENDING_REPLY_NOW=5000 + # Invoked indirectly through FM_PENDING_REPLY_SEND_HOOK. + # shellcheck disable=SC2329 + remote_repost_hook() { + printf '%s\t%s\n' "$1" "$2" >> "$hook_log" + } + export -f remote_repost_hook + export FM_PENDING_REPLY_SEND_HOOK=remote_repost_hook + + fm_write_meta "$state/ios.meta" \ + "window=fm-remote:w1:p1" "harness=claude" "kind=secondmate" "mode=secondmate" \ + "remote_host=remote-mac" "remote_root=/remote/root" "remote_backend=herdr" + corr=$(fm_pending_reply_create "$home" "$state" "ios" "status of the iOS build") + fm_pending_reply_mark_delivered "$state" "$corr" + fm_pending_reply_observe_busy "$state" "$corr" busy + fm_pending_reply_observe_busy "$state" "$corr" idle + rec=$(fm_pending_reply_path "$state" "$corr") + + # The mate's turn ended, but nothing proves the parent has read the remote + # reply log since: a repost here would nag for a reply already written there. + if fm_pending_reply_send_recovery "$state" "$corr" 2>/dev/null; then + fail "a remote repost must not fire before the reply channel is known caught up" + fi + [ ! -s "$hook_log" ] || fail "no repost may be sent while the reply channel is behind" + [ "$(phase_of "$state" "$corr")" = awaiting_report ] \ + || fail "the expectation must stay armed while the reply channel is behind" + + # A watermark from BEFORE the turn ended is still not evidence. + fm_pending_reply_note_remote_channel_caught_up "$state" ios 4000 + if fm_pending_reply_send_recovery "$state" "$corr" 2>/dev/null; then + fail "a stale reply-channel watermark must not license a repost" + fi + [ ! -s "$hook_log" ] || fail "a stale watermark must not release a repost" + + # Read through the end of the remote log after the turn: the report really is + # missing, so the one recovery repost fires. + fm_pending_reply_note_remote_channel_caught_up "$state" ios \ + "$(fm_pending_reply_get "$rec" request_turn_completed_epoch)" + fm_pending_reply_send_recovery "$state" "$corr" \ + || fail "a genuinely missed remote report must still trigger its recovery repost" + [ "$(phase_of "$state" "$corr")" = recovery_sent ] \ + || fail "phase should be recovery_sent, got $(phase_of "$state" "$corr")" + lines=$(wc -l < "$hook_log" | tr -d ' ') + [ "$lines" = 1 ] || fail "expected exactly one repost, got $lines" + case "$(cat "$hook_log")" in + *REPOST\ REQUIRED*) : ;; + *) fail "the recovery message must ask for a repost"$'\n'"$(cat "$hook_log")" ;; + esac + unset FM_PENDING_REPLY_SEND_HOOK + pass "a remote repost waits for the reply channel and still fires on a real miss" +} + +test_mirrored_remote_reply_never_triggers_a_repost() { + local home state corr hook_log + home=$(setup_parent remote-mirrored-reply) + state="$home/state" + hook_log="$TMP_ROOT/remote-mirrored-reply.log" + : > "$hook_log" + export FM_PENDING_REPLY_NOW=6000 + # Invoked indirectly through FM_PENDING_REPLY_SEND_HOOK. + # shellcheck disable=SC2329 + mirrored_reply_hook() { + printf '%s\t%s\n' "$1" "$2" >> "$hook_log" + } + export -f mirrored_reply_hook + export FM_PENDING_REPLY_SEND_HOOK=mirrored_reply_hook + + fm_write_meta "$state/ios.meta" \ + "window=fm-remote:w1:p1" "harness=claude" "kind=secondmate" "mode=secondmate" \ + "remote_host=remote-mac" "remote_root=/remote/root" "remote_backend=herdr" + corr=$(fm_pending_reply_create "$home" "$state" "ios" "did the build go green") + fm_pending_reply_mark_delivered "$state" "$corr" + fm_pending_reply_mark_turn_completed "$state" "$corr" request + # The mirror caught up AND carried the mate's correlated answer. + printf 'done [corr=%s]: build is green\n' "$corr" > "$state/ios.status" + fm_pending_reply_note_remote_channel_caught_up "$state" ios 6000 + + fm_pending_reply_tick_one "$state" "$corr" idle || fail "tick should succeed" + [ "$(phase_of "$state" "$corr")" = resolved ] \ + || fail "a mirrored correlated reply must resolve, got $(phase_of "$state" "$corr")" + [ ! -s "$hook_log" ] || fail "a correlated remote reply must never trigger a repost" + unset FM_PENDING_REPLY_SEND_HOOK + pass "a mirrored correlated remote reply resolves without any repost" +} + test_failed_send_discards_undelivered_expectation() { local home state corr home=$(setup_parent discard) @@ -1117,5 +1211,7 @@ test_tick_skips_terminal_and_reuses_target_observation test_correlations_reuse_only_for_matching_open_task test_tick_end_to_end_missed_then_escalate test_failed_send_discards_undelivered_expectation +test_remote_repost_waits_for_the_reply_channel +test_mirrored_remote_reply_never_triggers_a_repost printf 'ok - all pending-reply tests passed\n' diff --git a/tests/fm-remote-job.test.sh b/tests/fm-remote-job.test.sh index f2ef8ce643..82f1cf8cce 100755 --- a/tests/fm-remote-job.test.sh +++ b/tests/fm-remote-job.test.sh @@ -369,7 +369,8 @@ PREEMPT_ELAPSED=$(( $(date +%s) - PREEMPT_BEGAN )) assert_present "$PREEMPT_SIDE_EFFECT" "the short command behind a long poll did not run" [ "$PREEMPT_ELAPSED" -le 10 ] || fail "a queued short command waited a full poll window behind the long poll" fm_remote_job_wait "$ACCOUNT_HOME" "$POLL_JOB_ID" || fail "$FM_REMOTE_JOB_ERROR" -[ "$FM_REMOTE_JOB_EXIT" -eq 75 ] || fail "a preempted long poll did not publish its elapsed-window result" +[ "$FM_REMOTE_JOB_EXIT" -eq "$FM_REMOTE_JOB_PREEMPTED_EXIT" ] \ + || fail "a preempted long poll was not distinguished from an elapsed window" [ ! -s "$FM_REMOTE_JOB_STDOUT" ] || fail "a preempted long poll published partial stdout" [ ! -s "$FM_REMOTE_JOB_STDERR" ] || fail "a preempted long poll published partial stderr" fm_remote_job_reap "$ACCOUNT_HOME" "$JOB_ID" || fail "the short command could not be reaped" diff --git a/tests/fm-remote-reply.test.sh b/tests/fm-remote-reply.test.sh index af9eb1eec3..40fe9f0ba7 100755 --- a/tests/fm-remote-reply.test.sh +++ b/tests/fm-remote-reply.test.sh @@ -400,6 +400,53 @@ assert_not_contains "$(status_open_decisions "$PARENT/state/ios.status")" \ unset FM_PENDING_REPLY_GRACE_SECS pass "a reply that arrives after escalation resolves it and clears the open decision" +rm -f -- "$PARENT/state/remote-replies/ios.caught-up" +remote_env "$ADAPTER" source ios > "$TMP_ROOT/preempted-source.out" 2>&1 & +PREEMPTED_SOURCE=$! +running_poll='' +for _ in $(seq 1 100); do + for job in "$TMP_ROOT"/remote-jobs/jobs/job-*; do + [ -d "$job" ] || continue + if [ "$(fm_remote_job_read_state "$job" 2>/dev/null || true)" = running ]; then + running_poll=$job + break 2 + fi + done + sleep 0.05 +done +[ -n "$running_poll" ] || fail "the reply poll did not begin running before preemption" +remote_env "$ROOT/bin/fm-on.sh" ios fm-remote-file.sh get data/reply/report.md 262144 >/dev/null +set +e +wait "$PREEMPTED_SOURCE" +preempted_rc=$? +set -e +[ "$preempted_rc" -eq "$FM_REMOTE_JOB_PREEMPTED_EXIT" ] \ + || fail "the reply poll did not expose remote-job preemption: $preempted_rc" +assert_absent "$PARENT/state/remote-replies/ios.caught-up" \ + "a preempted reply poll published a caught-up watermark" +pass "a preempted reply poll cannot publish channel freshness" + +# A quiet window is the one moment this channel can prove it is NOT behind, and +# the parent's pending-reply guard needs that proof: a remote report that exists +# but has not been mirrored yet must never be mistaken for a report the mate +# never wrote. The window opened with the log matching the committed cursor, so +# the published watermark is the window's start. +watermark_before=$(date +%s) +set +e +FM_REMOTE_REPLY_WAIT_SECONDS=1 remote_env "$ADAPTER" source ios >/dev/null 2>&1 +quiet_rc=$? +set -e +[ "$quiet_rc" -eq 75 ] || fail "a quiet reply window exited with an unexpected status: $quiet_rc" +watermark_after=$(date +%s) +caught_up=$(FM_STATE_OVERRIDE="$PARENT/state" bash -c ' + . "$1/bin/fm-pending-reply-lib.sh" + fm_pending_reply_remote_channel_epoch "$2/state" ios +' _ "$ROOT" "$PARENT") +[ -n "$caught_up" ] || fail "a quiet reply window published no caught-up watermark" +[ "$caught_up" -ge "$watermark_before" ] && [ "$caught_up" -le "$watermark_after" ] \ + || fail "the caught-up watermark ($caught_up) is outside the quiet window" +pass "a quiet reply window publishes the caught-up watermark the reply guard reads" + # The observed already-handled replay class: a lost cursor (an update or # convergence retire) makes the next armed source recapture the WHOLE remote # log from offset 0. Every line is already mirrored, so the at-most-once @@ -467,6 +514,8 @@ remote_env "$ADAPTER" handle ios 12 "$RESULT_TWELVE" >/dev/null 2>&1 || [ "$?" - || fail "pending continuity result could not be acknowledged after retirement refusal" remote_env "$ADAPTER" retire ios >/dev/null assert_absent "$PARENT/state/remote-replies/ios.cursor" "adapter retirement left its cursor" +assert_absent "$PARENT/state/remote-replies/ios.caught-up" \ + "adapter retirement left a caught-up watermark a later route could inherit" pass "remote reply retirement quiesces and refuses unhandled captured results" echo "ALL TESTS PASSED" diff --git a/tests/fm-send-remote-delivery.test.sh b/tests/fm-send-remote-delivery.test.sh index af546fbb4a..eaba4deb8a 100755 --- a/tests/fm-send-remote-delivery.test.sh +++ b/tests/fm-send-remote-delivery.test.sh @@ -28,6 +28,8 @@ set -u # shellcheck source=tests/lib.sh . "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# shellcheck source=bin/fm-pending-reply-lib.sh +. "$ROOT/bin/fm-pending-reply-lib.sh" SEND="$ROOT/bin/fm-send.sh" DRAIN="$ROOT/bin/fm-wake-drain.sh" @@ -231,6 +233,32 @@ test_remote_delivered_unconfirmed_closes_resolve_key() { pass "fm-send remote: a delivered-unconfirmed answer closes its --resolve-key decision" } +test_local_secondmate_pending_keeps_expectation_armed() { + local dir fb log home rc rec corr + dir="$TMP_ROOT/local-pending-expectation"; mkdir -p "$dir" + fb=$(make_stubs "$dir"); log="$dir/send.log" + home=$(setup_home local-pending-expectation) + fm_write_meta "$home/state/lsm.meta" \ + "window=sess:fm-lsm" "harness=claude" "kind=secondmate" "mode=secondmate" "home=$home/sm" + + : > "$log" + env PATH="$fb:$PATH" FM_FAKE_TMUX_PENDING=1 \ + FM_ROOT_OVERRIDE="$home" FM_HOME="$home" FM_SEND_LOG="$log" FM_SEND_SETTLE=0 \ + "$SEND" lsm "audit the ledger" >/dev/null 2>&1; rc=$? + expect_code 3 "$rc" "an unconfirmed local secondmate submit must exit delivered-unconfirmed" + rec=$(pending_record "$home") + [ -n "$rec" ] \ + || fail "the pending-reply expectation must survive an unconfirmed local secondmate send" + [ "$(fm_pending_reply_get "$rec" phase)" = awaiting_report ] \ + || fail "the surviving expectation must stay armed, got $(fm_pending_reply_get "$rec" phase)" + # Armed means resolvable: the mate's correlated report still closes it. + corr=$(fm_pending_reply_get "$rec" corr_id) + printf 'done [corr=%s]: ledger clean\n' "$corr" > "$home/state/lsm.status" + fm_pending_reply_try_resolve "$home/state" "$corr" \ + || fail "a correlated report must still resolve the preserved expectation" + pass "fm-send local: an unconfirmed secondmate send keeps its reply expectation armed" +} + test_local_pending_reports_delivered_unconfirmed() { local dir fb log home rc err dir="$TMP_ROOT/local-pending"; mkdir -p "$dir" @@ -281,5 +309,6 @@ test_remote_transport_unknown_preserves_expectation test_remote_delivered_unconfirmed_closes_resolve_key test_local_pending_reports_delivered_unconfirmed test_local_pending_does_not_close_resolve_key +test_local_secondmate_pending_keeps_expectation_armed echo "all fm-send-remote-delivery tests passed" From b57c4d6e28fdd34ab7b67f548ab53611b4572af4 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:15:54 -0700 Subject: [PATCH 16/16] fix(bin): honor declared pauses in busy-pane wedge checks (#2619) * fix(watch): honor a declared pause on a busy pane's completed-turn bound A worker that declares an external wait (`paused:`) and then blocks in one long foreground call - a review-hosting scout parked in a single blocking `lavish-axi poll`, a bounded watch loop, a rate-limit sleep - keeps its pane BUSY, so the stale path that already honors declared pauses never ran for it. The busy-pane completed-turn bound instead routed it straight into wedge_timer_check, which re-escalated "possible wedge, escalation N" (and, past the threshold, demand-deep-inspection) every FM_STALE_ESCALATE_SECS for as long as the review stayed open. busy_turn_bound_check now owns which absorber takes a crossed bound: a crew whose own last status line declares an external wait or a verified captain-held transfer takes the bounded FM_PAUSE_RESURFACE_SECS recheck, and everything else keeps the unchanged wedge timer. The discriminator is the declaration together with liveness (the caller has already confirmed the pane is busy), never a blanket silencing - a crew that declared nothing, or whose pane is not live, escalates exactly as before, and a declared pause still re-surfaces once per long cadence so a forgotten wait cannot rot invisibly. Away mode is untouched: the daemon owns pause triage there and already reads the same vocabulary. The two call sites also no longer clear pause bookkeeping in the same poll the pause cadence recorded it, which would have erased the re-surface throttle and turned the long cadence back into a per-poll re-surface. Tests: a three-phase regression fixture pins the absorbed pause, its long-cadence recheck, and the restored wedge escalation once the declaration is lifted on the same busy over-age pane. Also de-flakes tests/fm-watch-triage.test.sh, which failed spuriously on a loaded machine: fixed liveness budgets were reaping watchers mid-startup, so assertions on post-poll state passed vacuously or failed spuriously. Waits that describe a poll's outcome now wait for a completed poll cycle via the liveness beacon, the heartbeat test waits for the heartbeat it asserts on, and every wait_for_exit budget is the uniform 10s already used elsewhere in the file. * no-mistakes(review): Fail poll-cycle waits on timeout * no-mistakes(review): Prevent poll timeout test hangs * no-mistakes(document): Clarify paused busy-pane supervision --- bin/fm-watch.sh | 74 ++++++--- docs/architecture.md | 2 + docs/configuration.md | 4 +- tests/fm-watch-triage.test.sh | 272 +++++++++++++++++++++++++++------- 4 files changed, 278 insertions(+), 74 deletions(-) diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 3f4a57afd6..a3f78fcc33 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -34,12 +34,14 @@ # (window_is_busy true) is exempt from the above, but # only up to BUSY_TURN_MAX_SECS with no completed turn # (state/.turn-ended, or the spawn record before any -# turn completes); past that bound busy_turn_over_age -# routes it through the same wedge timer, so it surfaces -# with the identical "stale: ..." reason, escalation -# count, and demand-deep-inspection marker, for human -# inspection only - never an automatic interrupt, -# signal, or restart of the worker or its tool process. +# turn completes). Past that bound, a declared external +# wait or verified captain-held transfer uses the long +# pause recheck cadence; every other pane goes through +# the same wedge timer and surfaces with the identical +# "stale: ..." reason, escalation count, and +# demand-deep-inspection marker, for human inspection +# only - never an automatic interrupt, signal, or restart +# of the worker or its tool process. # check: