From 4055cbd6ec99e54210e01698c7c9b01f66e0a74c Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Thu, 17 Sep 2026 19:24:16 -0700 Subject: [PATCH 001/237] fix: harden mail checks and rebalance full-coverage CI (#4800) * Improve CI reliability and rebalance full-coverage validation * no-mistakes(document): Clarify lint partition documentation --- .github/workflows/ci.yml | 33 +++- CONTRIBUTING.md | 21 +- bin/fm-lint.sh | 104 +++++++--- bin/fm-mail-check.sh | 12 +- bin/fm-test-run.sh | 334 +++++++++++++++++--------------- docs/fm-test-portable-shards.md | 35 +++- tests/fm-ci-workflow.test.sh | 31 +++ tests/fm-lint.test.sh | 63 +++++- tests/fm-mail-check.test.sh | 51 +++++ tests/fm-test-run.test.sh | 8 +- tests/fm-watcher-lock.test.sh | 67 ++++++- tests/wake-helpers.sh | 16 +- 12 files changed, 552 insertions(+), 223 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a68f49cdd02..8436d065119 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ permissions: contents: read # Per-PR supersession: a new push to the same PR replaces that PR's in-flight -# CI instead of letting superseded heads keep 13 jobs of hosted-runner work. +# CI instead of letting superseded heads keep the full hosted-runner fan-out. # The group uses the PR number for pull_request events, so every run of one PR # shares a group, and falls back to the unique run id for push events, so each # main push gets its own group and is never cancelled. Cancellation is likewise @@ -24,11 +24,14 @@ concurrency: jobs: lint: - name: Lint + name: Lint ${{ matrix.partition }} runs-on: ubuntu-latest - # Hang tripwire only: lint executions measured at 14-16 minutes in the - # September 12 starvation report, so this leaves deliberate margin. + # Keep the hang tripwire separate from the measured performance target. timeout-minutes: 25 + strategy: + fail-fast: false + matrix: + partition: [1, 2] steps: - uses: actions/checkout@v6 - name: Install pinned ShellCheck @@ -45,7 +48,19 @@ jobs: # 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 + - name: Lint canonical partition + run: | + set -eu + mkdir -p "$RUNNER_TEMP/fm-lint" + bin/fm-lint.sh --partition "${{ matrix.partition }}of${{ strategy.job-total }}" \ + --telemetry "$RUNNER_TEMP/fm-lint/partition-${{ matrix.partition }}.tsv" + - name: Upload lint telemetry + if: always() + uses: actions/upload-artifact@v4 + with: + name: fm-lint-telemetry-${{ matrix.partition }} + path: ${{ runner.temp }}/fm-lint/partition-${{ matrix.partition }}.tsv + if-no-files-found: warn # Deterministic proof that portable parallel shards + portable serial + Herdr # equal the complete tests/*.test.sh inventory with no missing or duplicates, @@ -159,15 +174,15 @@ jobs: tests-portable-serial: name: Behavior portable serial ${{ matrix.shard }} runs-on: ubuntu-latest - # Current runners can take ~20 min for a balanced shard. This 30-minute cap - # preserves the timeout as a hang tripwire while allowing runner-speed and - # job-setup margin; it is not the expected healthy end of the lane. + # Refreshed weights put the longest modeled shard near 12 minutes across + # nine runners. Preserve the existing hang tripwire until complete Linux + # measurements establish the new healthy envelope; a model is not a timer. timeout-minutes: 30 strategy: # Every shard reports so one failure never hides another shard's result. fail-fast: false matrix: - shard: [1, 2, 3, 4, 5] + shard: [1, 2, 3, 4, 5, 6, 7, 8, 9] steps: - uses: actions/checkout@v6 with: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5250d77e555..e2fd860cafd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -32,6 +32,23 @@ GitHub Actions and Dependabot are exempt so their automation keeps working, but See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/start-here/quick-start/) for the full first-run walkthrough. +## Maintaining required checks + +GitHub required checks are configured in the repository's existing main ruleset, not activated by committing workflow YAML. +When applying this CI layout, preserve its existing pull-request, merge-method, linear-history, deletion, non-fast-forward, and administrator-bypass settings. +Add required status checks with `strict_required_status_checks_policy: false`; a main update alone must not force a branch update and retest. +Bind the checks to the GitHub Actions app already producing them, rather than accepting the same context from any integration. +No new app installation or manual runner setup is needed for that setting. + +Require the actual job contexts: `Lint 1`, `Lint 2`, `Test coverage guard`, `Repo invariants`, `Stock macOS Bash snapshot compatibility`, `Behavior portable parallel 1`, `Behavior portable parallel 2`, `Behavior portable serial 1` through `Behavior portable serial 9`, `Behavior tests (Herdr)`, `Behavior timing aggregate`, and `PR must be raised via no-mistakes`. +The last name is the compliance job context, not its workflow title; its existing automation exceptions remain unchanged. +The timing aggregate is not a substitute for individual jobs because it can succeed while collecting evidence from a failed run. + +Apply the approved rule change only after the corresponding workflow is green and landed, confirming exact names and the Actions integration id from real checks first. +Snapshot the current ruleset, amend that same rule with the authenticated GitHub API or settings UI, and read back both the ruleset and effective branch rules. +Verify missing or red checks prevent ordinary merging without creating a test merge; administrator override intentionally remains available. +Coordinate any workflow rollback with its required-check names so a retired check cannot leave ordinary merges waiting forever. + ## Repo conventions - This repo is a template for running a firstmate orchestrator agent. @@ -48,7 +65,9 @@ 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, pinned shellcheck version, pinned actionlint workflow lint, and the backend-purity check rejecting direct Beads CLI calls in core `bin/` scripts), and both CI and the no-mistakes pre-push gate invoke it with no arguments. + `bin/fm-lint.sh` must pass: it is the single owner of the lint definition (the shellcheck file set, config, pinned shellcheck version, pinned actionlint workflow lint, and the backend-purity check rejecting direct Beads CLI calls in core `bin/` scripts). + CI uses its full canonical partitions; the no-mistakes pre-push gate uses its context-selected default. + `docs/fm-test-portable-shards.md` owns partition verification and performance evidence. Its header and `--help` output own the exact local lint modes, file-set selection, and analysis flags. 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. diff --git a/bin/fm-lint.sh b/bin/fm-lint.sh index 9408508aff9..9886476177f 100755 --- a/bin/fm-lint.sh +++ b/bin/fm-lint.sh @@ -2,9 +2,9 @@ # 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 -# no-mistakes both invoke this script with no arguments, so this owner selects -# the context-appropriate rule set without duplicating lint configuration. +# ambient configuration disabled, and one exact ShellCheck version. CI selects +# canonical partitions; no-mistakes invokes the context-selected default, so +# both use this owner without duplicating lint configuration. # The explicit --fast mode is local-only and disables ShellCheck's extended # dataflow analysis while preserving ordinary shell lint checks and source # following. CI, main, and merge-base-less runs keep --norc --external-sources @@ -41,10 +41,15 @@ # invocations in the core bin/ and bin/backends/ scripts so every configured # backlog backend follows the same tasks-axi lifecycle path. # -# Canonical lint defaults to two bounded workers over two stable logical shards. -# Each shard writes separate diagnostics, and the parent replays those outputs in -# deterministic shard and root order after every worker finishes. FM_LINT_JOBS=1 -# runs the same shards serially with byte-identical diagnostics and exit selection. +# Lint defaults to two bounded workers over two stable logical shards. +# Diagnostics replay in stable shard/root order. FM_LINT_JOBS=1 changes +# concurrency, not diagnostics or exit selection. +# --partition 1of2/2of2 splits the entire canonical inventory across +# two CI runners, each with those same bounded workers. Partitions are complete, +# disjoint, and byte-weight balanced; --list-files exposes their actual roots. +# Partition mode is always full source-aware analysis, never changed-only or +# --fast, and does not accept explicit paths. Each partition also runs workflow +# lint and backend-purity checks, keeping either invocation independently useful. # # Optional quiet telemetry writes one bounded TSV snapshot of content and source # graph identity, wall/CPU/RSS, shard load, and competing ShellCheck processes. @@ -54,6 +59,7 @@ # fm-lint.sh --fast [path]... local lint with extended analysis disabled # fm-lint.sh ... lint explicit roots with the same config # fm-lint.sh --jobs <1|2> [path]... override bounded worker count +# fm-lint.sh --partition <1of2|2of2> lint one full-rigor canonical CI partition # fm-lint.sh --telemetry ... write a quiet metrics snapshot # fm-lint.sh --required-version print the ShellCheck pin # fm-lint.sh --list-files print the file set that would be linted @@ -396,6 +402,8 @@ JOBS=${FM_LINT_JOBS:-2} TELEMETRY=${FM_LINT_TELEMETRY:-} FAST=0 ANALYSIS_MODE=full +PARTITION= +PARTITION_REQUESTED=0 LIST_FILES=0 while [ "$#" -gt 0 ]; do case "$1" in @@ -417,6 +425,17 @@ while [ "$#" -gt 0 ]; do TELEMETRY=${1#*=} shift ;; + --partition) + [ "$#" -ge 2 ] || { printf 'fm-lint.sh: --partition requires 1of2 or 2of2.\n' >&2; exit 2; } + PARTITION=$2 + PARTITION_REQUESTED=1 + shift 2 + ;; + --partition=*) + PARTITION=${1#*=} + PARTITION_REQUESTED=1 + shift + ;; --fast) FAST=1 ANALYSIS_MODE=fast @@ -443,6 +462,22 @@ case "$JOBS" in *) printf 'fm-lint.sh: jobs must be 1 or 2, got %s.\n' "$JOBS" >&2; exit 2 ;; esac +case "$PARTITION" in + '') + if [ "$PARTITION_REQUESTED" -eq 1 ]; then + printf 'fm-lint.sh: --partition requires 1of2 or 2of2.\n' >&2 + exit 2 + fi + ;; + 1of2|2of2) + if [ "$FAST" -eq 1 ] || [ "$#" -gt 0 ]; then + printf 'fm-lint.sh: --partition requires full canonical lint; omit --fast and explicit paths.\n' >&2 + exit 2 + fi + ;; + *) printf 'fm-lint.sh: --partition must be 1of2 or 2of2, got %s.\n' "$PARTITION" >&2; exit 2 ;; +esac + if [ "$FAST" -eq 1 ] && { [ "${GITHUB_ACTIONS:-}" = true ] || [ "${CI:-}" = true ]; }; then printf 'fm-lint.sh: --fast is local-only; CI uses full ShellCheck analysis.\n' >&2 exit 2 @@ -492,7 +527,7 @@ if [ "$#" -gt 0 ]; then ROOTS=("$@") else full_lint=1 - if [ "${GITHUB_ACTIONS:-}" != true ] && [ "${CI:-}" != true ] \ + if [ -z "$PARTITION" ] && [ "${GITHUB_ACTIONS:-}" != true ] && [ "${CI:-}" != true ] \ && command -v git >/dev/null 2>&1 \ && git rev-parse --is-inside-work-tree >/dev/null 2>&1 \ && [ "$(git rev-parse --abbrev-ref HEAD 2>/dev/null)" != main ]; then @@ -519,6 +554,38 @@ if [ "$CHANGED_MODE" -eq 1 ] && [ "$FAST" -eq 0 ]; then EXCLUDE_CODES=$LOCAL_NOX_EXCLUDE ANALYSIS_MODE=local fi +# Stable largest-first packing is shared by cross-runner partition selection +# and the two local workers. Weights are a scheduling proxy, never a skip rule. +TAB=$(printf '\t') +fm_lint_root_weights() { + local index=1 path weight + for path in "${ROOTS[@]}"; do + case "$path" in + *"$TAB"*|*$'\n'*) + printf 'fm-lint.sh: paths containing tabs or newlines are not supported: %s\n' "$path" >&2 + return 2 + ;; + esac + weight=1 + if [ -f "$path" ]; then + weight=$(wc -c < "$path" 2>/dev/null | tr -d '[:space:]') + fi + case "$weight" in ''|*[!0-9]*) weight=1 ;; esac + printf '%s\t%s\t%s\n' "$weight" "$index" "$path" + index=$((index + 1)) + done +} + +if [ -n "$PARTITION" ]; then + PARTITION_ROOTS=() + partition_weights=$(fm_lint_root_weights) || exit $? + while IFS="$TAB" read -r index path; do + PARTITION_ROOTS+=("$path") + done < <(printf '%s\n' "$partition_weights" | LC_ALL=C sort -t "$TAB" -k1,1nr -k2,2n | awk -F '\t' -v want="${PARTITION%%of*}" ' + { shard=(load[2] < load[1]) ? 2 : 1; load[shard]+=$1; if (shard == want) print $2 "\t" $3 } + ' | LC_ALL=C sort -t "$TAB" -k1,1n) + ROOTS=("${PARTITION_ROOTS[@]}") +fi ROOT_COUNT=${#ROOTS[@]} if [ "$LIST_FILES" -eq 1 ]; then @@ -597,7 +664,6 @@ trap 'exit 129' HUP trap 'exit 130' INT trap 'exit 143' TERM -TAB=$(printf '\t') WEIGHTS="$TMP_ROOT/weights" OUTPUT_DIR="$TMP_ROOT/output" mkdir -p "$OUTPUT_DIR" @@ -608,24 +674,7 @@ while [ "$worker" -lt "$SHARD_COUNT" ]; do worker=$((worker + 1)) done -index=1 -: > "$WEIGHTS" -for path in "${ROOTS[@]}"; do - case "$path" in - *"$TAB"*|*$'\n'*) - printf 'fm-lint.sh: paths containing tabs or newlines are not supported: %s\n' "$path" >&2 - exit 2 - ;; - esac - if [ -f "$path" ]; then - weight=$(wc -c < "$path" 2>/dev/null | tr -d '[:space:]') - else - weight=1 - fi - case "$weight" in ''|*[!0-9]*) weight=1 ;; esac - printf '%s\t%s\t%s\n' "$weight" "$index" "$path" >> "$WEIGHTS" - index=$((index + 1)) -done +fm_lint_root_weights > "$WEIGHTS" || exit $? # Largest-first deterministic greedy assignment keeps the two bounded workers # balanced without affecting replay order. Direct bytes are a stable portable @@ -841,6 +890,7 @@ EOF printf 'content_cksum\t%s\n' "$content_cksum" printf 'shellcheck_version\t%s\n' "$resolved" printf 'analysis_mode\t%s\n' "$ANALYSIS_MODE" + printf 'partition\t%s\n' "${PARTITION:-all}" printf 'jobs\t%s\n' "$JOBS" printf 'root_count\t%s\n' "$ROOT_COUNT" printf 'direct_lines\t%s\n' "$direct_lines" diff --git a/bin/fm-mail-check.sh b/bin/fm-mail-check.sh index 6d73102594c..b30c642f3e3 100755 --- a/bin/fm-mail-check.sh +++ b/bin/fm-mail-check.sh @@ -126,9 +126,11 @@ fi # dropped, and an empty failure gets a truth-stating fallback. poll_summary() { local rc=$1 out=$2 line - line=$(printf '%s\n' "$out" | sed -n '/^fm-mail: woke for /d; s/^fm-mail: //p' | head -n 1) + # First-line selectors must still drain the stream: head/quiet grep can + # close a large poll's pipe early and add a Broken pipe diagnostic. + line=$(printf '%s\n' "$out" | sed -n '/^fm-mail: woke for /d; s/^fm-mail: //p' | sed -n '1p') if [ -z "$line" ]; then - line=$(printf '%s\n' "$out" | sed -n '/^fm-mail: woke for /d; /^$/d; p' | head -n 1) + line=$(printf '%s\n' "$out" | sed -n '/^fm-mail: woke for /d; /^$/d; p' | sed -n '1p') fi if [ -z "$line" ]; then line="poll failed (rc=$rc)" @@ -174,8 +176,8 @@ record_write() { poll_has_publication_evidence() { local rc=${1:-0} out=$2 woken_before=$3 [ "$rc" -eq 124 ] && return 0 - if [ -n "$out" ] && printf '%s\n' "$out" | grep -qE \ - '^fm-mail: woke for |the wake stays queued|could not clear retry for recovered' + if [ -n "$out" ] && printf '%s\n' "$out" | grep -E \ + '^fm-mail: woke for |the wake stays queued|could not clear retry for recovered' >/dev/null then return 0 fi @@ -210,7 +212,7 @@ action_check() { line="poll did not finish within the ${BUDGET_SECS}s budget" elif [ "${rc:-0}" -ne 0 ]; then line=$(poll_summary "$rc" "$out") - elif printf '%s\n' "$out" | grep -q '^fm-mail: woke for '; then + elif printf '%s\n' "$out" | grep '^fm-mail: woke for ' >/dev/null; then # A successful poll can still surface new mail: the poll itself already # appended the durable mail wake rows, but the watcher only calls wake() # when THIS check's output is non-empty. Emit one line naming a surfaced diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 4819cc579b6..b939101c943 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -194,7 +194,7 @@ CHANGED_DEFAULT_TIMEOUT_SECS=900 # How many separate-runner shards the portable serial remainder splits into. # One owner: CI lane names carry this count and are refused when they disagree. -PORTABLE_SERIAL_SHARDS=5 +PORTABLE_SERIAL_SHARDS=9 # Balance hint for a portable-serial script with no measured duration, close to # the measured per-script mean so a newly added test neither starves nor @@ -657,163 +657,188 @@ list_portable_serial() { # Measured portable-serial script durations in milliseconds, from the CI timing # artifacts recorded in docs/fm-test-portable-shards.md. Each value is the -# slowest of several green runs, so the balance holds on a slow runner rather +# slowest successful sample in the referenced complete/partial CI runs, rather # than only on the fastest one measured. These are balance hints only: the shard # partition stays complete and disjoint whatever they say, so a stale hint costs # balance rather than coverage. That doc owns the refresh procedure. portable_serial_weight_hints() { cat <<'EOF' -tests/fm-agy-harness.test.sh 11000 -tests/fm-agy-signals-live-e2e.test.sh 23 -tests/fm-afk-contract.test.sh 3000 -tests/fm-afk-inject-e2e.test.sh 35792 -tests/fm-afk-pi-herdr-return-e2e.test.sh 100 -tests/fm-afk-return.test.sh 1837 -tests/fm-ask-user-authority.test.sh 128 +tests/fm-afk-contract.test.sh 15645 +tests/fm-afk-inject-e2e.test.sh 35889 +tests/fm-afk-pi-herdr-return-e2e.test.sh 45 +tests/fm-afk-return.test.sh 20385 +tests/fm-agy-harness.test.sh 47933 +tests/fm-agy-signals-live-e2e.test.sh 49 +tests/fm-ask-user-authority.test.sh 131 tests/fm-backend-cmux-smoke.test.sh 33 -tests/fm-backend-cmux.test.sh 3657 -tests/fm-backend-orca.test.sh 19253 -tests/fm-backend-tmux-smoke.test.sh 393 -tests/fm-backend-zellij-smoke.test.sh 23 -tests/fm-backend-zellij.test.sh 9418 -tests/fm-backend.test.sh 20061 -tests/fm-backlog-atomicity.test.sh 161989 -tests/fm-backlog-handoff.test.sh 52291 -tests/fm-bearings-board-render.test.sh 1528 -tests/fm-bearings-board.test.sh 4195 -tests/fm-bearings-snapshot.test.sh 116374 -tests/fm-bootstrap-network-parallel.test.sh 8214 -tests/fm-bootstrap.test.sh 25208 -tests/fm-branch-supervision.test.sh 5729 -tests/fm-busy-adapter-wiring.test.sh 49731 -tests/fm-busy-state.test.sh 2926 -tests/fm-calm-pi-extension.test.sh 256 -tests/fm-check-unregister.test.sh 481 -tests/fm-classify-corr-token.test.sh 38742 -tests/fm-classify-decision-key.test.sh 1167 -tests/fm-claude-stop-autoarm-live-e2e.test.sh 21 -tests/fm-claude-stop-autoarm.test.sh 60709 -tests/fm-cmux-claude-composer-live-e2e.test.sh 23 -tests/fm-codex-continuity-live-e2e.test.sh 21 -tests/fm-composer-matrix-live-e2e.test.sh 23 -tests/fm-control-relaunch.test.sh 48210 -tests/fm-control.test.sh 54301 -tests/fm-cursor-harness.test.sh 30103 -tests/fm-cursor-primary-live-e2e.test.sh 21 -tests/fm-cursor-primary.test.sh 54947 -tests/fm-dispatch-resolve.test.sh 1800 -tests/fm-daemon.test.sh 26870 -tests/fm-documentation-audiences.test.sh 732 -tests/fm-extension-binding.test.sh 7398 -tests/fm-fleet-snapshot-view.test.sh 8547 -tests/fm-fleet-sync.test.sh 37749 -tests/fm-gate-refuse.test.sh 4977 -tests/fm-gitignore-config.test.sh 62 -tests/fm-gotmp.test.sh 1310 -tests/fm-grok-continuity-live-e2e.test.sh 20 -tests/fm-grok-stop-live-e2e.test.sh 21 -tests/fm-guard-stale-banner.test.sh 32981 -tests/fm-harness-adapter-instructions-live-e2e.test.sh 20 -tests/fm-harness-adapter-references.test.sh 55 -tests/fm-harness-liveness-drift-live-e2e.test.sh 21 -tests/fm-herdr-attached-viewer-live-e2e.test.sh 19000 -tests/fm-herdr-session-cleanup.test.sh 6704 -tests/fm-herdr-submit-confirm-live-e2e.test.sh 23 -tests/fm-herdr-version-floor-live-e2e.test.sh 23 -tests/fm-home-summary-refresh.test.sh 34793 -tests/fm-inactive-reconcile.test.sh 74399 -tests/fm-kimi-harness.test.sh 18015 -tests/fm-lint-workflows.test.sh 855 -tests/fm-live-gate.test.sh 6000 -tests/fm-muse-harness.test.sh 55572 -tests/fm-muse-signals-live-e2e.test.sh 23 -tests/fm-no-mistakes-required.test.sh 370 -tests/fm-omp-harness.test.sh 59969 -tests/fm-on.test.sh 34087 -tests/fm-opencode-primary-live-e2e.test.sh 21 -tests/fm-operational-input.test.sh 231 -tests/fm-peek-remote.test.sh 1018 -tests/fm-pending-reply.test.sh 86711 -tests/fm-pi-branch-extension.test.sh 22239 -tests/fm-pi-branch-live-e2e.test.sh 56 -tests/fm-pi-branch-responsiveness-live-e2e.test.sh 21 -tests/fm-pi-primary-live-e2e.test.sh 20 -tests/fm-pi-watch-extension.test.sh 42970 +tests/fm-backend-cmux.test.sh 3498 +tests/fm-backend-orca.test.sh 23381 +tests/fm-backend-tmux-smoke.test.sh 363 +tests/fm-backend-zellij-smoke.test.sh 21 +tests/fm-backend-zellij.test.sh 9064 +tests/fm-backend.test.sh 21658 +tests/fm-backlog-atomicity.test.sh 196948 +tests/fm-backlog-handoff.test.sh 51990 +tests/fm-backlog-read-bound.test.sh 24288 +tests/fm-bearings-board-lavish-live-e2e.test.sh 48 +tests/fm-bearings-board-render.test.sh 12591 +tests/fm-bearings-board.test.sh 36490 +tests/fm-bearings-snapshot.test.sh 171176 +tests/fm-bootstrap-network-parallel.test.sh 9539 +tests/fm-bootstrap.test.sh 46634 +tests/fm-branch-supervision.test.sh 8915 +tests/fm-busy-adapter-wiring.test.sh 27817 +tests/fm-busy-state.test.sh 2990 +tests/fm-calm-claude-mod-live-e2e.test.sh 46 +tests/fm-calm-claude-mod-plugin.test.sh 172 +tests/fm-calm-claude-mod.test.sh 1252 +tests/fm-calm-pi-extension.test.sh 45128 +tests/fm-check-unregister.test.sh 464 +tests/fm-ci-workflow.test.sh 2073 +tests/fm-classify-corr-token.test.sh 49294 +tests/fm-classify-decision-key.test.sh 3336 +tests/fm-claude-stop-autoarm-live-e2e.test.sh 45 +tests/fm-claude-stop-autoarm.test.sh 60797 +tests/fm-claude-trust.test.sh 10410 +tests/fm-cmux-claude-composer-live-e2e.test.sh 47 +tests/fm-codex-continuity-live-e2e.test.sh 71 +tests/fm-codex-hook-layer-live-e2e.test.sh 47 +tests/fm-composer-codex-idle-live-e2e.test.sh 229 +tests/fm-composer-matrix-live-e2e.test.sh 47 +tests/fm-contributions.test.sh 35676 +tests/fm-control-relaunch.test.sh 137013 +tests/fm-control.test.sh 39524 +tests/fm-cursor-harness.test.sh 30212 +tests/fm-cursor-primary-live-e2e.test.sh 72 +tests/fm-cursor-primary.test.sh 52269 +tests/fm-daemon.test.sh 27262 +tests/fm-dispatch-resolve.test.sh 4397 +tests/fm-documentation-audiences.test.sh 847 +tests/fm-extension-binding.test.sh 9053 +tests/fm-fleet-snapshot-view.test.sh 17465 +tests/fm-fleet-sync.test.sh 35983 +tests/fm-gate-refuse.test.sh 5328 +tests/fm-gemini-harness.test.sh 938 +tests/fm-gitignore-config.test.sh 58 +tests/fm-gotmp.test.sh 1320 +tests/fm-grok-continuity-live-e2e.test.sh 45 +tests/fm-grok-stop-live-e2e.test.sh 46 +tests/fm-guard-stale-banner.test.sh 14968 +tests/fm-harness-adapter-instructions-live-e2e.test.sh 48 +tests/fm-harness-adapter-references.test.sh 83 +tests/fm-harness-liveness-drift-live-e2e.test.sh 881 +tests/fm-harness-precedence.test.sh 3661 +tests/fm-herdr-pi-stale-registration-live-e2e.test.sh 47 +tests/fm-herdr-session-cleanup.test.sh 6828 +tests/fm-herdr-submit-confirm-live-e2e.test.sh 46 +tests/fm-herdr-version-floor-live-e2e.test.sh 72 +tests/fm-home-summary-refresh.test.sh 37264 +tests/fm-inactive-reconcile.test.sh 53178 +tests/fm-kimi-harness.test.sh 19151 +tests/fm-lint-workflows.test.sh 785 +tests/fm-live-gate.test.sh 1755 +tests/fm-mail-check.test.sh 9162 +tests/fm-mail.test.sh 9703 +tests/fm-muse-harness.test.sh 40970 +tests/fm-muse-signals-live-e2e.test.sh 77 +tests/fm-nm-test-contract.test.sh 128 +tests/fm-no-mistakes-required.test.sh 247 +tests/fm-omp-harness.test.sh 47734 +tests/fm-omp-primary-live-e2e.test.sh 46 +tests/fm-on.test.sh 11001 +tests/fm-opencode-primary-live-e2e.test.sh 48 +tests/fm-operational-input.test.sh 221 +tests/fm-peek-remote.test.sh 964 +tests/fm-pending-reply.test.sh 28255 +tests/fm-pi-branch-extension.test.sh 60394 +tests/fm-pi-branch-live-e2e.test.sh 72 +tests/fm-pi-branch-responsiveness-live-e2e.test.sh 13121 +tests/fm-pi-codex-native.test.sh 46 +tests/fm-pi-primary-live-e2e.test.sh 47 +tests/fm-pi-watch-extension.test.sh 50637 tests/fm-pi-windows-shell-invocation.test.sh 5121 -tests/fm-pr-check-security.test.sh 172215 -tests/fm-procevent-quota.test.sh 1949 -tests/fm-procevent-when.test.sh 17392 -tests/fm-procevent.test.sh 69715 -tests/fm-project-origin.test.sh 137 -tests/fm-public-followup.test.sh 196745 -tests/fm-quota-array-dispatch-live-e2e.test.sh 21 -tests/fm-quota-choose.test.sh 1461 -tests/fm-remote-backlog-handoff.test.sh 41432 -tests/fm-remote-doctor.test.sh 5198 -tests/fm-remote-entrypoint.test.sh 132 -tests/fm-remote-herdr-guard.test.sh 1500 -tests/fm-remote-job-orphan-reap.test.sh 2972 -tests/fm-remote-job.test.sh 59603 -tests/fm-remote-reply.test.sh 101690 -tests/fm-remote-secondmate-lifecycle-e2e.test.sh 209631 -tests/fm-remote-secondmate-parent-binding.test.sh 29562 -tests/fm-remote-secondmate-trace-context.test.sh 67096 -tests/fm-remote-transport-lanes.test.sh 63976 -tests/fm-secondmate-harness.test.sh 151589 -tests/fm-secondmate-lifecycle-e2e.test.sh 8793 -tests/fm-secondmate-liveness.test.sh 18146 -tests/fm-secondmate-reconcile.test.sh 62726 -tests/fm-secondmate-restart.test.sh 119085 -tests/fm-secondmate-safety.test.sh 57689 -tests/fm-secondmate-sync.test.sh 17183 -tests/fm-send-inbox-doorbell-live-e2e.test.sh 22 -tests/fm-send-inbox.test.sh 38956 -tests/fm-send-remote-delivery.test.sh 27686 -tests/fm-send-resolve-key.test.sh 19619 -tests/fm-send-secondmate-marker-herdr-e2e.test.sh 51 -tests/fm-send-secondmate-marker.test.sh 6252 -tests/fm-session-lock-ancestry.test.sh 1414 -tests/fm-session-start.test.sh 156952 -tests/fm-sessionstart-hook-live-e2e.test.sh 20 -tests/fm-sessionstart-instruction-refresh-live-e2e.test.sh 22 -tests/fm-sessionstart-nudge.test.sh 66194 -tests/fm-shared-captain-inheritance.test.sh 6108 -tests/fm-spawn-dispatch-profile.test.sh 63996 -tests/fm-spawn-pool-base-freshen.test.sh 34920 -tests/fm-spawn-worktree-settle.test.sh 5687 -tests/fm-startup-memory-budget.test.sh 6964 -tests/fm-startup-network.test.sh 62274 -tests/fm-stow-cascade.test.sh 3101 -tests/fm-subagent-pretool-check.test.sh 1030 -tests/fm-supervision-events.test.sh 719 -tests/fm-tangle-guard.test.sh 9662 -tests/fm-task-delivery.test.sh 5952 -tests/fm-task-inbox.test.sh 25369 -tests/fm-teardown-endpoint-safety.test.sh 4620 -tests/fm-teardown.test.sh 97603 -tests/fm-test-fixture-cleanup.test.sh 915 -tests/fm-test-fixtures.test.sh 151 -tests/fm-test-isolation-proof.test.sh 2567 -tests/fm-tmux-agent-liveness.test.sh 1516 -tests/fm-turnend-foreign-owner-arm-fix.test.sh 2530 -tests/fm-tool-update-check.test.sh 14176 -tests/fm-trace-context-lib.test.sh 209 -tests/fm-trace-context-spawn.test.sh 44702 -tests/fm-turnend-guard.test.sh 42565 -tests/fm-update.test.sh 5212 -tests/fm-vendor-auth-probe.test.sh 43316 -tests/fm-voice-relay.test.sh 28699 -tests/fm-wake-daemon-lifecycle-e2e.test.sh 7381 -tests/fm-wake-drain-open-decisions-cursor.test.sh 20629 -tests/fm-wake-drain-open-decisions.test.sh 6240 -tests/fm-wake-drain-outcome-backstop.test.sh 15182 -tests/fm-wake-drain-unread-status.test.sh 35078 -tests/fm-wake-queue.test.sh 56674 -tests/fm-watch-arm.test.sh 69464 -tests/fm-watch-checkpoint.test.sh 5779 -tests/fm-watch-recovery-loop.test.sh 58731 -tests/fm-watch-triage.test.sh 262626 -tests/fm-watcher-lock.test.sh 88554 +tests/fm-pr-check-security.test.sh 226546 +tests/fm-pr-reviewers.test.sh 273 +tests/fm-pr-state-live-e2e.test.sh 45 +tests/fm-pr-state.test.sh 531 +tests/fm-procevent-quota.test.sh 1900 +tests/fm-procevent-when.test.sh 23805 +tests/fm-procevent.test.sh 221745 +tests/fm-project-origin.test.sh 136 +tests/fm-public-followup.test.sh 153508 +tests/fm-quota-array-dispatch-live-e2e.test.sh 71 +tests/fm-quota-choose.test.sh 1484 +tests/fm-remote-backlog-handoff.test.sh 73123 +tests/fm-remote-doctor.test.sh 13889 +tests/fm-remote-entrypoint.test.sh 108 +tests/fm-remote-herdr-guard.test.sh 3044 +tests/fm-remote-job-orphan-reap.test.sh 2905 +tests/fm-remote-job.test.sh 59354 +tests/fm-remote-reply.test.sh 118669 +tests/fm-remote-secondmate-lifecycle-e2e.test.sh 241208 +tests/fm-remote-secondmate-parent-binding.test.sh 32176 +tests/fm-remote-secondmate-trace-context.test.sh 59689 +tests/fm-remote-transport-lanes.test.sh 62635 +tests/fm-rovo-harness.test.sh 14322 +tests/fm-rovo-signals-live-e2e.test.sh 48 +tests/fm-secondmate-harness.test.sh 163801 +tests/fm-secondmate-lifecycle-e2e.test.sh 9633 +tests/fm-secondmate-liveness.test.sh 10402 +tests/fm-secondmate-reconcile.test.sh 97544 +tests/fm-secondmate-restart.test.sh 44488 +tests/fm-secondmate-safety.test.sh 127260 +tests/fm-secondmate-sync.test.sh 54502 +tests/fm-send-agy-confirm.test.sh 3983 +tests/fm-send-inbox-doorbell-live-e2e.test.sh 46 +tests/fm-send-inbox.test.sh 38632 +tests/fm-send-remote-delivery.test.sh 27717 +tests/fm-send-resolve-key.test.sh 28685 +tests/fm-send-secondmate-marker-herdr-e2e.test.sh 52 +tests/fm-send-secondmate-marker.test.sh 5309 +tests/fm-session-lock-ancestry.test.sh 2857 +tests/fm-session-start.test.sh 179350 +tests/fm-sessionstart-hook-live-e2e.test.sh 97 +tests/fm-sessionstart-instruction-refresh-live-e2e.test.sh 46 +tests/fm-sessionstart-nudge.test.sh 66247 +tests/fm-shared-captain-inheritance.test.sh 5687 +tests/fm-spawn-dispatch-profile.test.sh 138433 +tests/fm-spawn-pool-base-freshen.test.sh 62249 +tests/fm-spawn-worktree-settle.test.sh 8482 +tests/fm-startup-memory-budget.test.sh 7392 +tests/fm-startup-network.test.sh 61336 +tests/fm-stat-shadowing.test.sh 48 +tests/fm-stow-cascade.test.sh 3022 +tests/fm-subagent-pretool-check.test.sh 949 +tests/fm-supervision-events.test.sh 659 +tests/fm-tangle-guard.test.sh 7470 +tests/fm-task-delivery.test.sh 19784 +tests/fm-task-inbox.test.sh 30004 +tests/fm-tasks-axi.test.sh 1953 +tests/fm-teardown-endpoint-safety.test.sh 33210 +tests/fm-teardown.test.sh 145174 +tests/fm-test-fixture-cleanup.test.sh 937 +tests/fm-test-fixtures.test.sh 1562 +tests/fm-test-isolation-proof.test.sh 2692 +tests/fm-tmux-agent-liveness.test.sh 1953 +tests/fm-tool-update-check.test.sh 13832 +tests/fm-trace-context-lib.test.sh 227 +tests/fm-trace-context-spawn.test.sh 49071 +tests/fm-turnend-foreign-owner-arm-fix.test.sh 2397 +tests/fm-turnend-guard.test.sh 33450 +tests/fm-update.test.sh 11572 +tests/fm-vendor-auth-probe.test.sh 45255 +tests/fm-voice-relay.test.sh 32486 +tests/fm-wake-daemon-lifecycle-e2e.test.sh 7477 +tests/fm-wake-drain-open-decisions-cursor.test.sh 38506 +tests/fm-wake-drain-open-decisions.test.sh 6890 +tests/fm-wake-drain-outcome-backstop.test.sh 44076 +tests/fm-wake-drain-unread-status.test.sh 16169 +tests/fm-wake-queue.test.sh 85252 +tests/fm-watch-arm.test.sh 68479 +tests/fm-watch-checkpoint.test.sh 6076 +tests/fm-watch-recovery-loop.test.sh 58946 +tests/fm-watch-triage.test.sh 697969 +tests/fm-watcher-lock.test.sh 108940 EOF } @@ -2183,6 +2208,13 @@ fi # An explicit --jobs names a concurrency for exactly the selection given, so an # unproven script in it is a refusal rather than something to schedule around. if [ "$JOBS" -gt 1 ] && [ "$AUTO_CONCURRENCY" -eq 0 ]; then + # A single heavy suite can occupy a whole serial shard. Its family may have + # a separate concurrency proof, but that never changes this lane's contract. + if [ "$MODE" = lane ]; then + case "$LANE" in + portable-serial|portable-serial-*) die "--jobs $JOBS refused: portable serial lanes stay serial; use --jobs 1" ;; + esac + fi for s in "${SCRIPTS[@]}"; do if ! script_allows_concurrency "$s"; then die "--jobs $JOBS refused: $s is not in the proven-isolated set (see bin/fm-test-isolation-proof.sh --list) and its family has no recorded concurrent proof. Unproven stateful scripts stay serial." diff --git a/docs/fm-test-portable-shards.md b/docs/fm-test-portable-shards.md index a327902b99c..c7d6be38fde 100644 --- a/docs/fm-test-portable-shards.md +++ b/docs/fm-test-portable-shards.md @@ -57,8 +57,10 @@ Each shard is still strictly serial in itself, and separate runners mean no two `.github/workflows/ci.yml` derives the same `n` from `strategy.job-total` rather than a literal, so changing the shard count in either file without the other fails the lane loudly instead of leaving part of the required suite unrun. Assignment is longest-processing-time bin packing over per-script duration hints embedded in `bin/fm-test-run.sh`. -The embedded hints include the slowest measurements retained from the `fm-test-timing-portable-serial-*` artifacts of three green CI runs on 2026-09-01, [33558082172](https://github.com/kunchenguid/firstmate/actions/runs/33558082172), [33523597838](https://github.com/kunchenguid/firstmate/actions/runs/33523597838), and [33463326167](https://github.com/kunchenguid/firstmate/actions/runs/33463326167), the completed-script measurements from [run 34342484144](https://github.com/kunchenguid/firstmate/actions/runs/34342484144), plus the 5121 ms native-Windows focused runner measurement for `tests/fm-pi-windows-shell-invocation.test.sh` from 2026-09-06T21:02Z. -Taking the slowest of several CI runs rather than a single run keeps the balance honest on a slow runner. +The serial hints were refreshed from successful per-script records in the `fm-test-timing-portable-serial-*` artifacts of the complete green [run 35279383618](https://github.com/kunchenguid/firstmate/actions/runs/35279383618) and the available completed shards of [run 35282466441](https://github.com/kunchenguid/firstmate/actions/runs/35282466441) on 2026-09-17. +Together these cover all 176 serial scripts at refresh time; retain the slower successful sample where both exist. +The native-Windows-only `tests/fm-pi-windows-shell-invocation.test.sh` retains its separate 5121 ms measurement from 2026-09-06T21:02Z instead of a portable capability skip. +An unfinished or failed invocation is not a healthy duration sample. A script with no hint gets the conservative `PORTABLE_SERIAL_DEFAULT_WEIGHT_MS` default. Hints only affect balance: the coverage guard keeps the partition complete and disjoint whatever they say, so a stale hint costs a slower shard rather than lost coverage. Balance is still worth keeping current, because enough unmeasured scripts let one shard carry more than twice another shard's real work and reach the job cap while another runner sits idle. @@ -66,10 +68,12 @@ That is not hypothetical: by 2026-09-01 the lane had grown from 116 to 139 scrip `bin/fm-test-run.sh --check-coverage` now reports the unmeasured share as `serial_unhinted=` and refuses past `PORTABLE_SERIAL_MAX_UNHINTED_PERCENT`, so hint drift fails the coverage guard instead of silently pushing one shard into its job cap. Refresh the hints whenever the serial lane gains scripts, rather than waiting for that bound to trip. -`bin/fm-test-run.sh` owns the per-shard packing, so its `--check-coverage` output is the current account of lane size, shard composition, and balance rather than a copied table. -Run 34342484144 observed a shard reach about 20 minutes of passing work, so the 30-minute job cap keeps meaningful hang-tripwire margin for job setup and runner-speed spread. - -The single longest script, `tests/fm-watch-triage.test.sh` at 262626 ms, is the floor for any shard count. +`bin/fm-test-run.sh` owns the per-shard packing, so its `--check-coverage` output is the current account of lane size and coverage rather than a copied inventory. +Nine serial runners pack the refreshed measurements into a longest modeled script sum of 697969 ms (11m38s), with other shards near 10m36s. +The longest script, `tests/fm-watch-triage.test.sh`, legitimately occupies one whole shard and is the indivisible floor for this layout. +This is a packing estimate, not measured new-workflow execution or an end-to-end latency guarantee. +Existing job timeouts remain hang tripwires; they are not the desired healthy duration. +`tests/fm-ci-workflow.test.sh` compares the parsed CI matrix to the executable runner lanes, and the runner rejects parallel `--jobs` on a serial lane even when that shard has only one member. Refresh the CI-derived hints by downloading the per-shard timing artifacts from several green CI runs and replacing the `portable_serial_weight_hints` table in `bin/fm-test-run.sh` with the slowest measured `duration_ms` per `path`: @@ -77,13 +81,14 @@ Refresh the CI-derived hints by downloading the per-shard timing artifacts from for run in ; do gh run download "$run" -R kunchenguid/firstmate --pattern 'fm-test-timing-portable-serial-*' -D "/tmp/fm-serial/$run" done -jq -r '.scripts[] | [.path, .duration_ms] | @tsv' /tmp/fm-serial/*/*.json \ +jq -r '.scripts[] | select(.exit == 0) | [.path, .duration_ms] | @tsv' /tmp/fm-serial/*/*/*.json \ | awk -F'\t' '$2 > m[$1] { m[$1] = $2 } END { for (p in m) print p, m[p] }' \ | LC_ALL=C sort bin/fm-test-run.sh --check-coverage ``` -A timed-out shard uploads no artifact, so pick runs where every serial shard is green or the lane's slowest scripts go unmeasured in exactly the shard that needs them most. +A timed-out shard may upload no artifact, so include a complete green run or the slowest scripts go unmeasured in exactly the shard that needs them most. +Completed shards from a partial run can supplement that complete baseline, but never treat missing tail scripts or the timeout duration as successful samples. Measure native-Windows-only scripts through the focused Git Bash runner and retain that `duration_ms` separately, because the portable CI shards skip them. ## Coverage guard @@ -99,6 +104,18 @@ Portable shards, each portable serial shard, and the Herdr lane upload runner-ge `bin/fm-test-run.sh --aggregate-json` creates the combined summary artifact. `.github/workflows/ci.yml` owns the exact artifact names and aggregation wiring. +## Lint partitions and end-to-end latency + +`bin/fm-lint.sh` owns two canonical CI partitions, each running the same full source-aware ShellCheck analysis with two bounded workers, pinned versions, workflow validation, and backend-purity checks. +Its `--list-files` interface exposes partition membership; `tests/fm-lint.test.sh` verifies complete/disjoint executed roots and unchanged analysis flags. +The workflow uploads each partition's quiet telemetry to distinguish analysis cost, memory use, and host contention. +No fast mode, path skips, reduced checks, or paid runner provisioning is part of this layout. + +The performance objective is a complete green run under fifteen minutes including start delay: roughly twelve minutes of longest-path execution, at most two minutes of runner delay, and less than one minute of other overhead. +The candidate uses fourteen long-lived Linux jobs (nine serial, two parallel, Herdr, two lint), plus short checks and macOS; insufficient shared account capacity can erase the packing gain. +Compare complete before/after runs, preserve cancelled and partial-run evidence, and measure a representative normal-run sample before claiming a P95 improvement. +The workflow retains per-PR supersession without cancelling main pushes or changing the compliance workflow's event semantics. + ## Local entry points [CONTRIBUTING.md](../CONTRIBUTING.md) owns the local test policy and common entry points. @@ -109,7 +126,7 @@ Portable shards, each portable serial shard, and the Herdr lane upload runner-ge | Lane | Bound | Rationale | |---|---|---| | portable parallel 1/2 | See [CI workflow](../.github/workflows/ci.yml) | The workflow owns the parallel cap rationale and its evidence limits. | -| portable serial 1-5 | job `timeout-minutes: 30` | Current runners can take about 20 minutes; the 30-minute cap remains a hang tripwire while leaving margin for job setup and runner-speed spread. | +| portable serial shards | See [CI workflow](../.github/workflows/ci.yml) | Packing estimates are not healthy execution bounds; the existing cap remains a hang tripwire. | | Herdr | family-run step `timeout-minutes: 20`; job `timeout-minutes: 75` backstop | Healthy runs finished around 7 minutes before this lane gained `fm-backend-herdr-focus-flash-e2e`, which measures about 2 minutes against a real lab locally, so the step bound is still the hang tripwire (cleanup and timing artifacts still upload) while the job cap stays a last-resort backstop. Refresh this figure from the lane's uploaded timing artifact. | Timeouts are intended as hang tripwires; a passing coverage guard does not establish a healthy job duration. diff --git a/tests/fm-ci-workflow.test.sh b/tests/fm-ci-workflow.test.sh index fd2f7918493..78795eebe4b 100755 --- a/tests/fm-ci-workflow.test.sh +++ b/tests/fm-ci-workflow.test.sh @@ -151,6 +151,37 @@ CAPS pass "the already-measured lane bounds are unchanged" } +test_ci_matrices_match_executable_partitions() { + ruby -ryaml -ropen3 - "$CI_WORKFLOW" "$ROOT" <<'RUBY' || fail "CI partition contract" +jobs = YAML.load_file(ARGV[0]).fetch("jobs") +root = ARGV[1] +serial = jobs.fetch("tests-portable-serial").fetch("strategy") +raise "serial failures must not cancel other shards" unless serial.fetch("fail-fast") == false +matrix = serial.fetch("matrix") +raise "unexpected serial dimensions" unless matrix.keys == ["shard"] +shards = matrix.fetch("shard") +lanes, status = Open3.capture2(File.join(root, "bin/fm-test-run.sh"), "--list-lanes") +raise "cannot list runner lanes" unless status.success? +actual = lanes.lines.map(&:strip).select { |l| l.match?(/\Aportable-serial-\d+of\d+\z/) } +expected = shards.map { |s| "portable-serial-#{s}of#{shards.length}" } +raise "CI matrix and runner disagree" unless actual.sort == expected.sort +lint = jobs.fetch("lint").fetch("strategy") +raise "lint failures must not cancel another partition" unless lint.fetch("fail-fast") == false +matrix = lint.fetch("matrix") +raise "unexpected lint dimensions" unless matrix.keys == ["partition"] +parts = matrix.fetch("partition") +roots = parts.flat_map do |p| + output, result = Open3.capture2(File.join(root, "bin/fm-lint.sh"), "--partition", "#{p}of#{parts.length}", "--list-files") + raise "unsupported lint partition" unless result.success? + output.lines.map(&:strip) +end +canonical, result = Open3.capture2({"CI" => "true"}, File.join(root, "bin/fm-lint.sh"), "--list-files") +raise "lint matrix loses or duplicates canonical roots" unless result.success? && roots.sort == canonical.lines.map(&:strip).sort +RUBY + pass "CI matrices cover every executable serial lane and canonical lint root exactly once" +} + +test_ci_matrices_match_executable_partitions test_pr_pushes_supersede_within_one_pr test_separate_prs_do_not_cancel_each_other test_main_pushes_are_never_cancelled diff --git a/tests/fm-lint.test.sh b/tests/fm-lint.test.sh index f2fe3a528d4..75edfe85bae 100755 --- a/tests/fm-lint.test.sh +++ b/tests/fm-lint.test.sh @@ -1,17 +1,17 @@ #!/usr/bin/env bash # Parity guard for firstmate's shell-lint definition. # -# bin/fm-lint.sh must be the single owner that BOTH CI -# (.github/workflows/ci.yml) and the pre-push gate (.no-mistakes.yaml -# commands.lint) invoke, so the local lint can never diverge from CI again. +# bin/fm-lint.sh is the single owner invoked by CI +# (.github/workflows/ci.yml) and by the pre-push gate (.no-mistakes.yaml +# commands.lint). CI runs its two full-rigor canonical partitions; the local +# gate uses its context-selected default. Their selection differs deliberately, +# while this owner keeps analysis flags, configuration, and tool versions from +# drifting. # Regression origin: with no commands.lint configured, the local no-mistakes -# lint step never ran the deterministic -# `shellcheck bin/*.sh bin/backends/*.sh tests/*.sh`, so PRs passed local -# validation yet failed that exact check in CI on info/warning findings such as -# SC2015, SC1007, and SC2034. A second axis was tool-version skew: CI's -# ShellCheck floated with the runner image and still emitted SC2015, which -# ShellCheck retired in 0.11.0. fm-lint.sh now pins one exact version and both -# gates resolve it, so command, file set, config, AND version all match. +# lint step never ran the deterministic shell lint, so PRs passed local +# validation yet failed CI on info/warning findings such as SC2015, SC1007, and +# SC2034. A second axis was tool-version skew: CI's ShellCheck floated with the +# runner image and still emitted SC2015, which ShellCheck retired in 0.11.0. set -u # shellcheck source=tests/lib.sh @@ -178,6 +178,48 @@ test_list_files_reports_the_shell_inventory() { pass "fm-lint.sh --list-files reports the complete shell inventory" } +test_canonical_partitions_preserve_full_lint() { + local tmp fakebin all part selected log flags mode rc option + tmp=$(fm_test_tmproot fm-lint-partitions) + fakebin="$tmp/bin" + mkdir -p "$fakebin" + all=$(CI=true "$LINT" --list-files | LC_ALL=C sort) + : > "$tmp/union" + for part in 1of2 2of2; do + selected=$(CI=false GITHUB_ACTIONS=false "$LINT" --partition "$part" --list-files) \ + || fail "partition $part must select full canonical roots even on a local branch" + [ -n "$selected" ] || fail "empty lint partition $part" + printf '%s\n' "$selected" >> "$tmp/union" + [ "$selected" = "$("$LINT" --partition "$part" --list-files)" ] \ + || fail "partition $part is nondeterministic" + log="$tmp/$part.roots" + flags="$tmp/$part.flags" + mode="$tmp/$part.mode" + fm_lint_stub_shellcheck "$fakebin" "$log" + PATH="$fakebin:$PATH" FM_TEST_FLAG_LOG="$flags" FM_TEST_MODE_LOG="$mode" \ + "$LINT" --partition "$part" > "$tmp/$part.out" 2>&1 \ + || fail "canonical partition $part failed: $(cat "$tmp/$part.out")" + [ "$(LC_ALL=C sort "$log")" = "$(printf '%s\n' "$selected" | LC_ALL=C sort)" ] \ + || fail "partition $part executed a different root set than it listed" + [ "$(LC_ALL=C sort -u "$flags")" = "$(printf 'exclude=none\nexternal-sources=yes')" ] \ + || fail "partition $part weakened source-aware analysis" + [ "$(LC_ALL=C sort -u "$mode")" = on ] || fail "partition $part disabled full analysis" + done + [ "$(LC_ALL=C sort "$tmp/union")" = "$all" ] || fail "lint partitions lose or duplicate canonical roots" + for option in 0of2 3of2 1of3; do + rc=0 + "$LINT" --partition "$option" --list-files > "$tmp/refused" 2>&1 || rc=$? + [ "$rc" = 2 ] || fail "invalid partition $option was not refused" + done + rc=0 + "$LINT" --partition 1of2 --fast > "$tmp/refused" 2>&1 || rc=$? + [ "$rc" = 2 ] || fail "partition accepted --fast" + rc=0 + "$LINT" --partition 1of2 bin/fm-lint.sh > "$tmp/refused" 2>&1 || rc=$? + [ "$rc" = 2 ] || fail "partition accepted an explicit subset" + pass "two canonical lint partitions preserve complete source-aware coverage and reject weakened modes" +} + # fm_lint_stub_git : install a git stub for the changed-file mode # tests below. Its answers are driven by env vars the caller sets before # invoking fm-lint.sh, so those tests can steer git state without depending on @@ -1364,6 +1406,7 @@ SH test_help_reports_the_complete_interface test_list_files_reports_the_shell_inventory +test_canonical_partitions_preserve_full_lint test_fast_mode_disables_extended_analysis test_ci_defaults_to_full_analysis test_ci_rejects_explicit_fast_mode diff --git a/tests/fm-mail-check.test.sh b/tests/fm-mail-check.test.sh index 36152594924..736bd0195d5 100644 --- a/tests/fm-mail-check.test.sh +++ b/tests/fm-mail-check.test.sh @@ -256,6 +256,56 @@ test_repeated_failure_that_queued_new_mail_still_wakes() { pass "fm-mail-check: a repeated failure that queued new mail still wakes" } +test_large_poll_output_is_drained() { + # A pipe reader that exits at the first match closes before the producer has + # written this poll's output. Ignored SIGPIPE makes that race observable as + # stderr noise instead of silently terminating a pipeline subprocess. + # Exercise both summary selectors and both wake predicates via the real check. + local tmpbin home shape attempt out expected + tmpbin="$TMP_ROOT/large-poll/bin" + mkdir -p "$tmpbin" + cp "$CHECK" "$tmpbin/" + for lib in fm-timeout-lib.sh fm-pr-lib.sh fm-line-cap-lib.sh fm-check-lib.sh; do + ln -s "$ROOT/bin/$lib" "$tmpbin/$lib" + done + cat > "$tmpbin/fm-mail.sh" <<'SH' +#!/usr/bin/env bash +printf 'fm-mail: woke for 42\n' +case "$FM_TEST_POLL_SHAPE" in + success) + awk 'BEGIN { for (i=0; i<20000; i++) print "poll diagnostic padding padding padding" }' + printf 'fm-mail: woke for 43\n' + ;; + preferred) + printf 'fm-mail: connection refused\n' >&2 + awk 'BEGIN { for (i=0; i<20000; i++) print "fm-mail: later diagnostic padding padding" }' >&2 + exit 1 + ;; + fallback) + printf 'raw connection failure\n' >&2 + awk 'BEGIN { for (i=0; i<20000; i++) print "raw later diagnostic padding padding" }' >&2 + exit 1 + ;; +esac +SH + chmod +x "$tmpbin/fm-mail.sh" + for shape in success preferred fallback; do + home=$(make_home "large-$shape") + case "$shape" in + success) expected='mail: new mail: woke for 43' ;; + preferred) expected='mail: connection refused' ;; + fallback) expected='mail: raw connection failure' ;; + esac + for attempt in 1 2; do + out="$home/out-$attempt.txt" + (trap '' PIPE; run_check "$home" "$out" "$tmpbin/fm-mail-check.sh" FM_TEST_POLL_SHAPE="$shape") + [ "$(cat "$out")" = "$expected" ] || fail "large $shape poll $attempt must emit only its summary: $(cat "$out")" + [ "$(wc -l < "$out" | tr -d '[:space:]')" = 1 ] || fail "large $shape poll must be exactly one line" + done + done + pass "fm-mail-check: large repeated polls drain every reader without output noise" +} + test_repeated_timeout_still_wakes() { # A timeout can kill the poll after wake_for queued mail and before the # woke-for line is printed. Difference-record silence would then leave that @@ -418,6 +468,7 @@ test_fail_closed_poll_after_wake_reports_the_failure test_repeated_status4_fail_closed_still_wakes test_repeated_status2_stays_queued_still_wakes test_repeated_failure_that_queued_new_mail_still_wakes +test_large_poll_output_is_drained test_repeated_timeout_still_wakes test_repeated_heal_failure_stays_silent test_missing_mail_plane_is_reported \ No newline at end of file diff --git a/tests/fm-test-run.test.sh b/tests/fm-test-run.test.sh index bb7c6b3c5fa..b3100151480 100755 --- a/tests/fm-test-run.test.sh +++ b/tests/fm-test-run.test.sh @@ -1142,8 +1142,8 @@ test_portable_serial_shards_partition_the_serial_lane() { shard=1 while [ "$shard" -le "$count" ]; do listed=$("$RUNNER" --list --lane "portable-serial-${shard}of${count}" | wc -l | tr -d ' ') - [ "$listed" -ge 2 ] \ - || fail "portable-serial-${shard}of${count} holds only $listed script(s)" + # One expensive suite can legitimately occupy a whole runner. Non-empty + # coverage is asserted above; script counts are not duration weights. [ "$listed" -le "$cap" ] \ || fail "portable-serial-${shard}of${count} holds $listed of $total scripts" shard=$((shard + 1)) @@ -1223,7 +1223,7 @@ test_jobs_requires_proven_isolated() { rc=$? set -e [ "$rc" -eq 2 ] || fail "--jobs with portable-serial must refuse (exit 2), got $rc" - grep -Fq 'not in the proven-isolated set' "$tmp/err" \ + grep -Fq 'portable serial lanes stay serial' "$tmp/err" \ || fail "--jobs refusal message missing: $(cat "$tmp/err")" set +e "$RUNNER" --jobs 2 tests/fm-afk-inject-e2e.test.sh >"$tmp/out2" 2>"$tmp/err2" @@ -1237,7 +1237,7 @@ test_jobs_requires_proven_isolated() { rc=$? set -e [ "$rc" -eq 2 ] || fail "--jobs with a portable serial shard must refuse, got $rc" - grep -Fq 'not in the proven-isolated set' "$tmp/err3" \ + grep -Fq 'portable serial lanes stay serial' "$tmp/err3" \ || fail "shard --jobs refusal message missing: $(cat "$tmp/err3")" rm -rf "$tmp" pass "--jobs refuses non-proven / stateful selections" diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index c0d37f0cf61..37af8ac641e 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -34,6 +34,62 @@ drain_and_ack() { # --recovery-generation "$generation" } +test_wait_deadline_reaps_a_stopped_child() { + # A stopped TERM-resistant child cannot finish graceful cleanup. The helper waited + # forever after its nominal deadline. An outer process-group deadline keeps + # this regression finite even if that bug returns. + python3 - "$ROOT/tests/wake-helpers.sh" <<'PY' || fail "bounded child cleanup regression" +import os +import signal +import subprocess +import sys + +script = r''' +. "$1" +bash -c 'trap "" TERM; kill -STOP "$$"; exec sleep 300' & +pid=$! +for i in $(seq 1 100); do + state=$(ps -p "$pid" -o stat=) + case "$state" in *T*) break ;; esac + sleep 0.01 +done +case "$state" in *T*) ;; *) kill -KILL "$pid"; exit 23 ;; esac +wait_for_exit "$pid" 2 +rc=$? +[ "$rc" = 124 ] || exit 21 +! kill -0 "$pid" 2>/dev/null || exit 22 +''' +p = subprocess.Popen([os.environ.get("BASH", "bash"), "-c", script, "_", sys.argv[1]], + start_new_session=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) +try: + out, err = p.communicate(timeout=15) +except subprocess.TimeoutExpired: + os.killpg(p.pid, signal.SIGKILL) + p.communicate() + raise SystemExit("wait_for_exit hung after its deadline on a stopped child") +if p.returncode or "survived TERM; sending KILL" not in err: + raise SystemExit(f"cleanup rc={p.returncode}, stdout={out}, stderr={err}") +PY + pass "wait deadline diagnoses and reaps a stopped test child without hanging" +} + +# Preserve the real watcher's trap diagnostics when testing its termination. +# A termination defect should fail this case promptly, not occupy a CI runner +# until the whole job times out and hides every following test. +stop_seed_watcher() { # + local pid=$1 out=$2 status=0 + kill -TERM "$pid" 2>/dev/null || true + wait_for_exit "$pid" 100 || status=$? + if [ "$status" -eq 124 ]; then + cat "$out" >&2 + fail "seed watcher survived TERM; see bounded wait/process/trap evidence above" + fi + if grep -E 'unexpected EOF|syntax error' "$out" >/dev/null; then + cat "$out" >&2 + fail "seed watcher emitted a shell parser error during termination" + fi +} + test_singleton_start() { local dir state fakebin out1 out2 pid1 pid2 live i dir=$(make_case singleton) @@ -575,7 +631,7 @@ test_arm_attaches_and_waits_for_live_fresh_watcher() { out="$dir/watch.out" armout="$dir/arm.out" # A genuinely live watcher with a fresh beacon already holds the singleton. - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" & + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" 2>&1 & wpid=$! i=0 while [ "$i" -lt 60 ]; do @@ -600,8 +656,7 @@ test_arm_attaches_and_waits_for_live_fresh_watcher() { [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$wpid" ] || fail "arm disturbed the healthy watcher's lock" is_live_non_zombie "$armpid" || fail "arm exited while the seed watcher was still healthy" # After the seed dies without a successor, the attached arm must fail loudly. - kill "$wpid" 2>/dev/null || true - wait "$wpid" 2>/dev/null || true + stop_seed_watcher "$wpid" "$out" wait_for_exit "$armpid" 80 status=$? [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "attached arm did not fail after seed died (status $status)" @@ -616,7 +671,7 @@ test_attached_arm_signal_is_recorded_in_cycle_ledger() { fakebin="$dir/fakebin" out="$dir/watch.out" armout="$dir/arm.out" - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" & + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" 2>&1 & wpid=$! i=0 while [ "$i" -lt 60 ]; do @@ -641,8 +696,7 @@ test_attached_arm_signal_is_recorded_in_cycle_ledger() { grep -q "arm_pid=$armpid.*watcher_pid=$wpid.*origin=attached.*exit_code=143.*signal=TERM.*reason=arm-interrupted" "$state/.watch-cycle-exits.log" \ || fail "attached arm signal was not recorded in the lifecycle ledger" is_live_non_zombie "$wpid" || fail "signaling an attached arm terminated the peer watcher" - kill "$wpid" 2>/dev/null || true - wait "$wpid" 2>/dev/null || true + stop_seed_watcher "$wpid" "$out" pass "attached arm signals record a classified lifecycle entry" } @@ -1108,6 +1162,7 @@ test_msys_pid_identity_uses_proc() { pass "MSYS process identity uses compatible /proc fields" } +test_wait_deadline_reaps_a_stopped_child test_singleton_start test_pid_identity_is_locale_invariant test_proc_pid_identity_ignores_wall_clock_and_detects_pid_reuse diff --git a/tests/wake-helpers.sh b/tests/wake-helpers.sh index da83bb3dc91..8e7106d8230 100644 --- a/tests/wake-helpers.sh +++ b/tests/wake-helpers.sh @@ -296,6 +296,9 @@ SH printf '%s\n' "$dir" } +# Only pass a process owned by this test. A deadline must also bound cleanup: +# TERM can be ignored or remain pending on a stopped child, so never follow it +# with an unbounded wait. Keep process evidence before the final owned-PID kill. wait_for_exit() { local pid=$1 limit=${2:-50} i=0 while [ "$i" -lt "$limit" ]; do @@ -306,7 +309,18 @@ wait_for_exit() { sleep 0.1 i=$((i + 1)) done - kill "$pid" 2>/dev/null || true + printf 'wait_for_exit: owned pid %s exceeded %s polls; sending TERM\n' "$pid" "$limit" >&2 + ps -p "$pid" -o pid= -o ppid= -o stat= -o command= >&2 2>/dev/null || true + kill -TERM "$pid" 2>/dev/null || true + i=0 + while [ "$i" -lt 20 ] && is_live_non_zombie "$pid"; do + sleep 0.1 + i=$((i + 1)) + done + if is_live_non_zombie "$pid"; then + printf 'wait_for_exit: owned pid %s survived TERM; sending KILL\n' "$pid" >&2 + kill -KILL "$pid" 2>/dev/null || true + fi wait "$pid" 2>/dev/null || true return 124 } From 5d3acc823f92dcdc484cf12a11458841f8e01ee4 Mon Sep 17 00:00:00 2001 From: ShaDev Date: Thu, 17 Sep 2026 21:26:17 -0500 Subject: [PATCH 002/237] fix(bin): answer Kimi 2.0.0 folder-trust dialog during spawn (#4799) * Handle Kimi workspace trust dialog * no-mistakes(review): Retry Kimi trust Enter and gate ready on dialog markers * no-mistakes(review): Gate Kimi ready on any trust marker and clean captures * no-mistakes(review): Read visible pane for Kimi trust and ready gates * no-mistakes(review): Add per-backend visible-pane capture for Kimi trust gate * no-mistakes(review): Harden Kimi viewport capture and trust dialog detection * no-mistakes(document): Document Kimi spawn refusal on cmux and Orca --- .../references/harness/kimi.md | 12 +- bin/backends/cmux.sh | 16 +- bin/backends/herdr.sh | 9 + bin/backends/tmux.sh | 8 + bin/backends/zellij.sh | 8 + bin/fm-backend.sh | 30 ++ bin/fm-spawn.sh | 128 ++++++- docs/configuration.md | 1 + docs/verification/runtime-backends.md | 1 + tests/fm-kimi-harness.test.sh | 327 +++++++++++++++++- 10 files changed, 516 insertions(+), 24 deletions(-) diff --git a/.agents/skills/harness-adapters/references/harness/kimi.md b/.agents/skills/harness-adapters/references/harness/kimi.md index 8799f8bcfcd..00c6d8be50c 100644 --- a/.agents/skills/harness-adapters/references/harness/kimi.md +++ b/.agents/skills/harness-adapters/references/harness/kimi.md @@ -1,6 +1,6 @@ # Kimi Code -Verified on 2026-07-25 with Kimi Code CLI 0.29.1. +Verified on 2026-09-17 with Kimi Code CLI 2.0.0. ## Operating facts @@ -13,16 +13,18 @@ Verified on 2026-07-25 with Kimi Code CLI 0.29.1. | Exit command | `/exit`. | | Interrupt | Single Escape, which prints `Interrupted by user`. | | Skill invocation | `/`, for example `/no-mistakes`; Firstmate skills are discovered. | -| Autonomy | `--auto`; `-y` and `--yolo` are weaker and are not used. | -| Trust dialog | None observed on a clean first launch in a fresh pooled worktree. | +| Autonomy | `--auto` is the `Never Ask` tier; `-y` and `--yolo` now select the distinct, weaker `Ask When Needed` tier and are not used. | +| Trust dialog | A fresh worktree shows `Trust this folder?` with `Trust this folder` pre-selected; spawn reads the visible pane, recognizes the complete dialog (its title, both navigation-hint tokens `↑↓ navigate` and `Enter select` - matched separately so a hint wrapped in a narrow pane still counts - the selected `❯ Trust this folder`, and `Don't trust`), sends Enter on every poll the complete dialog is still there, verifies that a later visible-pane capture no longer contains it, and then continues the ordinary readiness gate. Trust is never pre-registered in `config.toml`; the dialog is answered live. | | Slash submission | One Enter submits, with no popup swallow or settle hazard. | | Environment marker | None; identity comes from process ancestry command name `kimi`, which `../../../bin/fm-harness.sh` keeps a retained foreign marker from overriding. | | Composer | Bordered box with a bare `>` prompt glyph and no observed ghost or placeholder text. | -| Effort | No verified reasoning-effort flag; `references/common/model-and-effort.md` owns unsupported-value handling. | +| Effort | `kimi provider list --json` exposes per-model `supportEfforts` values `low`, `high`, and `max` plus a `defaultEffort`; the launch flag and mapping remain unverified, so spawn records and omits requested effort per `references/common/model-and-effort.md`. | ## Readiness-gated start -`../../../bin/fm-spawn.sh` launches Kimi bare, waits for the composer box or `Welcome to Kimi Code!`, sends only `Read the brief at and follow it exactly.`, and requires a cleared composer plus either the echoed `✨` submission or nonzero context before accepting delivery. +`../../../bin/fm-spawn.sh` launches Kimi bare, handles the complete 2.0.0 trust dialog when it appears, waits for the composer box or `Welcome to Kimi Code!`, sends only `Read the brief at and follow it exactly.`, and requires a cleared composer plus either the echoed `✨` submission or nonzero context before accepting delivery. +Every trust predicate reads `fm_backend_visible_capture` - the viewport with no scrollback - never the 120-line history read the delivery gate uses: the dialog is a TUI frame, and a history-backed capture would keep reporting it after Kimi redrew past it, storming Enter into a live composer and then failing an already trusted spawn. That primitive is implemented on tmux (`capture-pane -p -S -0`), herdr (`pane read --source visible`, verified against Herdr 0.8.0 in `docs/verification/runtime-backends.md`) and zellij (`action dump-screen --pane-id`, no `--full`), and `FM_BACKEND_VISIBLE_CAPTURE` in `bin/fm-backend.sh` is the one list of them. orca has only a history read; cmux's `read-screen` without `--scrollback` plausibly reads just the viewport but has not been live-verified. A Kimi spawn on either is therefore refused at preflight, before the worktree or pane exists, naming the backend and the missing verified viewport capability, pending that verification for cmux. There is no fallback to the scrollback read. A viewport read that exits nonzero fails readiness immediately with the backend named, rather than being mistaken for a blank screen. A successful but blank viewport read is absence of evidence, not evidence of a cleared dialog: it costs that poll, restarts the two-capture ready count below, and leaves the trust diagnostics where they were. The trust answer is retried until the dialog clears - Kimi swallows keypresses during its startup window, so a single Enter can be dropped - and the re-send is gated on the complete dialog still being on that visible pane, so it cannot fire once the dialog cleared. Trust is accepted only after a later visible-pane capture proves that the dialog cleared; a stuck dialog fails with the observed dialog signals and the answer count in the diagnostic. +Any single marker of the dialog on that visible pane - `Trust this folder` or the negative `Don't trust` option - withholds the ready verdict, because a capture caught mid-redraw and a capture that has painted only the box title both miss the complete dialog while the banner above it would otherwise read as ready. The banner also prints before the dialog paints at all, which no single capture can distinguish from a ready pane, so the verdict additionally requires two consecutive captures that are each ready and free of dialog text; a capture that is not ready, and a blank one, restarts that count, which is what keeps the pre-banner boot captures and redraw frames from spending it. This launch-then-send shape is mandatory because Kimi rejects positional instructions as an unknown command. The path must be absolute because the instructions live outside the task worktree and Kimi reads them there without `--add-dir`. diff --git a/bin/backends/cmux.sh b/bin/backends/cmux.sh index 0d9791216a3..747d3fc2ccd 100644 --- a/bin/backends/cmux.sh +++ b/bin/backends/cmux.sh @@ -512,12 +512,16 @@ fm_backend_cmux_send_text_line() { # [expected-label] return 2 } -# fm_backend_cmux_capture: bounded plain-text surface capture. No herdr-style -# small-N empty-result bug was found (finding #3), but "fetch generous, trim -# locally" is kept anyway: a single read-screen call is still bounded by the -# surface's actual current viewport height regardless of the requested -# --lines value, so a caller asking for more than the viewport can see would -# otherwise silently get less than it asked for with no way to tell why. +# fm_backend_cmux_capture: bounded plain-text surface capture. `--scrollback` +# is this adapter's explicit opt-in to history, so the result can include +# lines that have scrolled out of view - it is not a viewport read, and no +# viewport-only primitive is offered for cmux (see FM_BACKEND_VISIBLE_CAPTURE in +# bin/fm-backend.sh). Finding #3's viewport-height cap was observed on +# read-screen calls; whether a call WITHOUT --scrollback is strictly bounded to +# the viewport is plausible but has not been live-verified. No herdr-style +# small-N empty-result bug was found (finding #3); "fetch generous, trim +# locally" is kept for parity with herdr and so a small caller bound never +# depends on how read-screen clamps a small --lines value. fm_backend_cmux_capture() { # [expected-label] fm_backend_cmux_target_ready "$1" "${3:-}" || return 1 local lines=${2:-200} fetch raw out diff --git a/bin/backends/herdr.sh b/bin/backends/herdr.sh index 41254e26d0f..88a8cb61492 100644 --- a/bin/backends/herdr.sh +++ b/bin/backends/herdr.sh @@ -3016,6 +3016,15 @@ fm_backend_herdr_capture() { # printf '%s' "$out" | tail -n "$lines" } +# fm_backend_herdr_visible_capture: the visible viewport only. `--source +# visible` is herdr's viewport-bounded read, so it needs none of the --lines +# workaround above - the bound is the pane itself, and asking for a line count +# is what triggers the empty-read bug. +fm_backend_herdr_visible_capture() { # + fm_backend_herdr_target_ready "$1" || return 1 + fm_backend_herdr_cli "$FM_BACKEND_HERDR_SESSION" pane read "$FM_BACKEND_HERDR_PANE" --source visible 2>/dev/null +} + fm_backend_herdr_capture_ansi() { # fm_backend_herdr_target_ready "$1" || return 1 local lines=${2:-200} fetch out diff --git a/bin/backends/tmux.sh b/bin/backends/tmux.sh index ae2f33d353e..2bc1c8aa0d7 100644 --- a/bin/backends/tmux.sh +++ b/bin/backends/tmux.sh @@ -42,6 +42,14 @@ fm_backend_tmux_capture() { # tmux capture-pane -p -t "$1" -S -"$2" } +# fm_backend_tmux_visible_capture: the visible viewport only. `-S -0` starts at +# the first line of the pane rather than in its history, so nothing scrolled out +# of view can appear in the result - the guarantee a trust-dialog predicate +# needs, which the scrollback-bounded capture above cannot give. +fm_backend_tmux_visible_capture() { # + tmux capture-pane -p -t "$1" -S -0 +} + # fm_backend_tmux_send_key: one named key. Mirrors fm-send.sh's --key path: # `tmux display-message -p -t "$T" '#{pane_id}' >/dev/null`, then # `tmux send-keys -t "$T" "$2"`. diff --git a/bin/backends/zellij.sh b/bin/backends/zellij.sh index 56478f7db35..a90247899f1 100644 --- a/bin/backends/zellij.sh +++ b/bin/backends/zellij.sh @@ -493,6 +493,14 @@ fm_backend_zellij_capture() { # [expected-label] printf '%s' "$out" | tail -n "$lines" } +# fm_backend_zellij_visible_capture: the visible viewport only. `dump-screen` +# without --full is already viewport-bounded; this primitive keeps the dump +# whole instead of trimming it to a caller's line bound. +fm_backend_zellij_visible_capture() { # [expected-label] + fm_backend_zellij_target_ready "$1" "${2:-}" || return 1 + fm_backend_zellij_cli "$FM_BACKEND_ZELLIJ_SESSION" action dump-screen --pane-id "$FM_BACKEND_ZELLIJ_PANE" 2>/dev/null +} + # --- zellij composer capture and capability primitives ---------------------- # # `zellij action dump-screen --ansi` ("Preserve ANSI styling in the dump diff --git a/bin/fm-backend.sh b/bin/fm-backend.sh index b968038190c..9e6a5730a5f 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -730,6 +730,36 @@ fm_backend_capture() { # [expected-label] esac } +# FM_BACKEND_VISIBLE_CAPTURE: backends with a verified viewport-only read, each +# implementing fm_backend__visible_capture. This one list answers both the +# capability question and the dispatch, so they cannot disagree. cmux is absent +# pending live verification: its `read-screen` without `--scrollback` plausibly +# reads only the viewport, but that has not been observed on a real cmux, and +# the adapter's own capture opts into history with `--scrollback`. orca's +# `terminal read --limit` is a history read with no viewport mode. +FM_BACKEND_VISIBLE_CAPTURE="tmux herdr zellij" + +# fm_backend_visible_capture_supported: whether can read the visible +# viewport WITHOUT scrollback. Callers that must not mistake a scrolled-away +# frame for the live screen ask this first and fail closed on a no. +fm_backend_visible_capture_supported() { # + fm_backend_list_contains "$FM_BACKEND_VISIBLE_CAPTURE" "$1" +} + +# fm_backend_visible_capture: the visible viewport, never scrollback. A backend +# outside FM_BACKEND_VISIBLE_CAPTURE declines here rather than answering with a +# history-backed capture the caller would read as the live screen. +fm_backend_visible_capture() { # [expected-label] + local backend=$1 + shift + fm_backend_visible_capture_supported "$backend" || { + echo "error: backend '$backend' has no verified viewport-bounded capture primitive" >&2 + return 1 + } + fm_backend_source "$backend" || return 1 + "fm_backend_${backend}_visible_capture" "$@" +} + # fm_backend_send_key: one backend-supported named special key. fm_backend_send_key() { # [expected-label] local backend=$1 diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index d9867cca406..fa8da51d9ea 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -287,6 +287,15 @@ # Verified per-harness turn-end hooks are installed automatically where enabled; some live outside the worktree. # Kimi uses one surgically installed Firstmate region in $HOME/.kimi-code/config.toml, # a firstmate-owned global hook and registry, and a gitignored per-task pointer. +# Kimi 2.0.0 also gates a fresh worktree on an interactive folder-trust dialog. +# Its launch-readiness loop reads the visible viewport - so the spawn refuses at +# preflight on a backend with no viewport-bounded capture - recognizes the +# complete dialog, re-selects the already highlighted affirmative option on +# every poll the complete dialog is still there, refuses any ready verdict while +# dialog text is on that pane, and requires two consecutive captures that are +# each ready and dialog-free before the ordinary readiness gates can pass. A +# blank viewport read proves nothing either way: it costs the poll and restarts +# that count. A viewport read that fails outright fails readiness at once. # grok uses a firstmate-owned global hook under ${GROK_HOME:-$HOME/.grok}/hooks # plus a gitignored .fm-grok-turnend worktree pointer and a state token. # muse installs no hook at all - its plugin engine is off in the default build - so @@ -2246,10 +2255,11 @@ effort_flag_for_harness() { # opencode's interactive `opencode --prompt` launch has a verified --model # flag but no verified effort flag. Its `opencode run --variant` flag belongs # to a different, non-interactive launch mode, so fm-spawn does not pass it. - # kimi likewise has no reasoning-effort flag; the requested axis stays in - # task metadata but never reaches the launch command. Cursor encodes effort - # in model ids such as cursor-grok-4.5-high, so it also receives no separate - # effort flag. + # kimi provider catalogs expose supported and default effort values, but a + # launch flag and mapping have not been live-verified; the requested axis + # stays in task metadata but never reaches the launch command. Cursor encodes + # effort in model ids such as cursor-grok-4.5-high, so it also receives no + # separate effort flag. esac } @@ -2277,6 +2287,10 @@ case "$LAUNCH" in *__KIMIBIN__*) KIMI_BIN=$(resolve_kimi_binary) || exit 1 LAUNCH=${LAUNCH//__KIMIBIN__/$(shell_quote "$KIMI_BIN")} + fm_backend_visible_capture_supported "$BACKEND" || { + echo "error: refusing Kimi spawn because backend '$BACKEND' has no verified viewport-bounded capture; Kimi 2.0.0 gates a fresh worktree on a trust dialog that can only be answered and confirmed cleared from a scrollback-free read of the live pane" >&2 + exit 1 + } if [ "$KIND" != secondmate ]; then "$FM_ROOT/bin/fm-kimi-turnend-hook.sh" install || { echo "error: refusing Kimi spawn because the global turn-end hook could not be installed safely" >&2 @@ -3256,6 +3270,18 @@ kimi_capture() { fm_backend_capture "$BACKEND" "$T" 120 "$W" 2>/dev/null || true } +# Trust decisions read the visible pane only. The dialog is a TUI frame, so a +# scrollback-backed capture keeps reporting it long after Kimi redrew past it - +# which would storm Enter into a live composer and then fail an already trusted +# spawn for a dialog that did clear. There is deliberately no fallback to the +# bounded capture: the spawn refuses at preflight on a backend that cannot read +# the viewport, a read that fails outright fails readiness with its exit status +# and the backend's own error on stderr, and only a successful empty read is +# absence of evidence, which the poll loop treats as a skipped poll. +kimi_visible_capture() { + fm_backend_visible_capture "$BACKEND" "$T" "$W" +} + # Kimi launch-readiness and delivery route their composer-emptiness half # through the shared classifier (bin/fm-composer-lib.sh via # fm_backend_composer_state), the same owner every steer and injection guard @@ -3268,17 +3294,99 @@ kimi_composer_is_empty() { [ "$(fm_backend_composer_state "$BACKEND" "$T" "$W" 2>/dev/null)" = empty ] } +# The navigation hint is matched as its two distinctive tokens rather than as +# one row: a pane narrower than the row wraps it, and a wrapped hint is still +# the complete dialog waiting for an answer. +kimi_trust_dialog_is_visible() { # + local pane=$1 + case "$pane" in *'Trust this folder?'*) ;; *) return 1 ;; esac + case "$pane" in *'↑↓ navigate'*) ;; *) return 1 ;; esac + case "$pane" in *'Enter select'*) ;; *) return 1 ;; esac + case "$pane" in *'❯ Trust this folder'*) ;; *) return 1 ;; esac + case "$pane" in *"Don't trust"*) ;; *) return 1 ;; esac +} + +# The complete dialog above decides whether to press Enter. Any single marker +# of it on the visible pane decides whether that pane is safe to call ready: a +# capture caught mid-redraw and one that has painted only the dialog's box +# title both fail the complete-dialog test while the dialog is still up and +# waiting, with Kimi's startup banner sitting above it in that same capture. +# Treating such a pane as ready would type the brief pointer into the dialog +# and lose it. +kimi_trust_marker_is_present() { # + case "$1" in *'Trust this folder'* | *"Don't trust"*) return 0 ;; esac + return 1 +} + +# A successful key send is not evidence that Kimi accepted trust. Only the +# ordinary readiness signals in a later capture prove advancement. +kimi_ready_signal_is_present() { # + case "$1" in *'Welcome to Kimi Code!'*) return 0 ;; esac + kimi_composer_is_empty +} + kimi_wait_for_ready() { - local pane i=0 max=${FM_KIMI_READY_POLLS:-60} interval=${FM_KIMI_POLL_INTERVAL:-0.5} + local pane capture_rc i=0 max=${FM_KIMI_READY_POLLS:-60} interval=${FM_KIMI_POLL_INTERVAL:-0.5} + local trust_enters=0 trust_seen=0 trust_still_visible=0 trust_markers_pending=0 + local ready_captures=0 + KIMI_READY_FAILURE_DETAIL='kimi did not show a verified ready signal before brief delivery' while [ "$i" -lt "$max" ]; do - pane=$(kimi_capture) - if printf '%s\n' "$pane" | grep -Fq 'Welcome to Kimi Code!' || - kimi_composer_is_empty; then - return 0 + capture_rc=0 + pane=$(kimi_visible_capture) || capture_rc=$? + if [ "$capture_rc" -ne 0 ]; then + KIMI_READY_FAILURE_DETAIL="kimi readiness could not read the visible viewport of backend '$BACKEND' (viewport capture exited $capture_rc), so the trust dialog could neither be answered nor ruled out" + return 1 + fi + if [ -z "$pane" ]; then + ready_captures=0 + i=$((i + 1)) + [ "$i" -ge "$max" ] || sleep "$interval" + continue + fi + if kimi_trust_dialog_is_visible "$pane"; then + trust_seen=1 + trust_still_visible=1 + trust_markers_pending=0 + ready_captures=0 + # Kimi swallows keypresses during its startup window - the same hazard + # FM_KIMI_SUBMIT_RETRIES covers for the brief pointer - so the + # affirmative selection is re-sent on every poll the complete dialog is + # still on screen. The dialog's own disappearance is the postcondition: + # once it clears, this branch cannot fire again. + if ! spawn_send_key "$T" Enter; then + KIMI_READY_FAILURE_DETAIL="kimi trust dialog was seen but the affirmative selection could not be submitted" + return 1 + fi + trust_enters=$((trust_enters + 1)) + else + trust_still_visible=0 + if kimi_trust_marker_is_present "$pane"; then + trust_markers_pending=1 + ready_captures=0 + else + trust_markers_pending=0 + # The banner prints before the dialog paints its first frame, so one + # ready-looking capture cannot be told apart from a pane whose dialog is + # one redraw away. Two consecutive captures that are each ready and free + # of dialog text can; any capture that is not ready restarts the count. + if kimi_ready_signal_is_present "$pane"; then + ready_captures=$((ready_captures + 1)) + [ "$ready_captures" -lt 2 ] || return 0 + else + ready_captures=0 + fi + fi fi i=$((i + 1)) [ "$i" -ge "$max" ] || sleep "$interval" done + if [ "$trust_still_visible" -eq 1 ]; then + KIMI_READY_FAILURE_DETAIL="kimi trust dialog did not clear after selecting 'Trust this folder' on $trust_enters poll(s); saw 'Trust this folder?', the navigation hint, selected 'Trust this folder', and the negative Don't trust option" + elif [ "$trust_seen" -eq 1 ]; then + KIMI_READY_FAILURE_DETAIL="kimi trust dialog was answered but the pane never advanced to a verified ready signal; saw 'Trust this folder?', the navigation hint, selected 'Trust this folder', and the negative Don't trust option" + elif [ "$trust_markers_pending" -eq 1 ]; then + KIMI_READY_FAILURE_DETAIL="kimi did not show a verified ready signal before brief delivery; trust dialog text stayed on screen without the complete dialog, so the pane was never safe to answer or to treat as ready" + fi return 1 } @@ -4393,7 +4501,7 @@ fi spawn_send_key "$T" Enter if [ "$HARNESS" = kimi ]; then if ! kimi_wait_for_ready; then - kimi_spawn_fail "kimi did not show a verified ready signal before brief delivery" + kimi_spawn_fail "$KIMI_READY_FAILURE_DETAIL" exit 1 fi KIMI_POINTER="Read the brief at $BRIEF_REAL and follow it exactly." diff --git a/docs/configuration.md b/docs/configuration.md index 15f5f5efb9e..988e909bb57 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -310,6 +310,7 @@ The full cmux home label also includes a short hash of the resolved `FM_ROOT` pa ## Harness support claude, codex, opencode, pi, pi-signed, grok, kimi, cursor, and omp are empirically verified for crewmate and secondmate launches; gemini is verified for crewmate and scout launches only, and [README requirements](../README.md#requirements) own the set supported for the primary session. +`fm-spawn.sh` refuses kimi on cmux and Orca at preflight, because answering Kimi's folder-trust dialog needs a verified viewport-only capture those backends lack; [its adapter reference](../.agents/skills/harness-adapters/references/harness/kimi.md#readiness-gated-start) owns the trust-dialog handling. A cursor secondmate or primary runs the tracked project-scope `.cursor/hooks.json` in its own home and must be launched with `--trust`, or no project hook loads; [`docs/supervision-protocols/cursor.md`](supervision-protocols/cursor.md) owns its supervision protocol. Cursor typed-submit confirmation is verified on tmux and Herdr only. On Zellij, cmux, and Orca a typed-plane Cursor send (a harness-native invocation or an explicit backend target; ordinary text steers ride the durable inbox and exit 0 at enqueue) lands, but `fm-send` reports delivery unconfirmed and exits non-zero because their shared submit core does not consult the busy footer; [runtime backend verification](verification/runtime-backends.md#cursor-agent-cli) owns the evidence and transcript-state boundary. diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index 8cb1627c1dc..f870d561b89 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -902,6 +902,7 @@ The CLI matrix was checked directly: | Literal send | `herdr pane send-text --session ` | Left text unsubmitted until Enter. | | Keys | `herdr pane send-keys enter|escape|ctrl+c --session ` | Enter and Escape worked; Ctrl-C interrupted foreground work. | | Capture | `herdr pane read --source recent --lines N` | Small N could return empty below viewport height; a 200-line request plus local trim was stable. | +| Viewport capture | `herdr pane read --source visible` | Verified on 2026-09-17 against Herdr 0.8.0 (protocol 19): `herdr pane read --help` documents `--source ` with `[possible values: visible, recent, recent-unwrapped, detection]`; `--source visible` exited 0 and returned 51 lines (the viewport) while `--source recent --lines 200` returned 200. This is the viewport-only read behind `fm_backend_herdr_visible_capture`, which Kimi's trust-dialog gate requires. | | Native state | `herdr agent get ` | Working and done transitions were visible on some harnesses; live Claude Code 2.1.236 on Herdr 0.8.0 kept `agent_status=idle` for an entire landed turn, including a multi-second tool call, so submit confirmation falls through to the shared composer verdict. Native `busy` remains positive activity evidence, while native `idle` cannot close a turn and the adapter's semantic lifecycle decides worker state. | | Restart | guarded named-session stop then start | Workspace, tab, pane, and labels persisted; the agent process and registration did not. | | Close | `herdr pane close --session ` | The exact one-pane task tab closed; closing a final tab could remove the workspace. | diff --git a/tests/fm-kimi-harness.test.sh b/tests/fm-kimi-harness.test.sh index 518a614b7eb..e08674d3fe8 100755 --- a/tests/fm-kimi-harness.test.sh +++ b/tests/fm-kimi-harness.test.sh @@ -41,6 +41,26 @@ fake_screen() { ready) printf 'Welcome to Kimi Code!\ncontext: 0%% (0/256k)\n╭────────────────────────────────╮\n│ > │\n╰────────────────────────────────╯\n' ;; + trust) + printf '╭─ Trust this folder? ─╮\n│ ↑↓ navigate · Enter select · Esc exit │\n│ %s │\n│ ❯ Trust this folder │\n│ Don'"'"'t trust │\n╰──────────────────────────────╯\n' "$FM_FAKE_PANE_PATH" + ;; + trust-decoy) + printf 'Trust this folder?\n%s\n❯ Trust this folder\nDon'"'"'t trust\n' "$FM_FAKE_PANE_PATH" + ;; + trust-partial) + printf 'Welcome to Kimi Code!\nTrust this folder?\n%s\nDon'"'"'t trust\n' "$FM_FAKE_PANE_PATH" + ;; + booting) + printf 'shell starting\n$ \n' + ;; + banner-only|banner-first) + printf 'Welcome to Kimi Code!\nstarting in %s\n' "$FM_FAKE_PANE_PATH" + ;; + blank-frame) + ;; + trust-wrapped) + printf '╭─ Trust this folder? ─╮\n│ ↑↓ navigate · │\n│ Enter select · Esc │\n│ exit │\n│ %s │\n│ ❯ Trust this folder │\n│ Don'"'"'t trust │\n╰──────────────────────╯\n' "$FM_FAKE_PANE_PATH" + ;; pointer-typed) printf 'context: 0%% (0/256k)\n╭────────────────────────────────╮\n│ > Read the brief and follow it │\n│ │\n╰────────────────────────────────╯\n' ;; @@ -52,6 +72,12 @@ fake_screen() { ;; esac } +fake_history() { + if [ "${FM_FAKE_KIMI_HISTORY_KEEPS_DIALOG:-no}" = yes ] \ + && [ -s "$FM_FAKE_KIMI_TRUST_ENTER_LOG" ]; then + printf '╭─ Trust this folder? ─╮\n│ ↑↓ navigate · Enter select · Esc exit │\n│ ❯ Trust this folder │\n│ Don'"'"'t trust │\n╰──────────────────────╯\n' + fi +} fake_cursor_y() { case "$state" in pointer-typed) printf '3\n' ;; @@ -82,7 +108,10 @@ case "${1:-}" in ;; *) printf '%s\n' "$literal" >> "$FM_FAKE_POINTER_LOG" - printf 'pointer-typed\n' > "$FM_FAKE_KIMI_STATE" + case "$state" in + trust|trust-wrapped|trust-partial|trust-decoy|booting|banner-only|banner-first|blank-frame) ;; + *) printf 'pointer-typed\n' > "$FM_FAKE_KIMI_STATE" ;; + esac ;; esac exit 0 @@ -92,9 +121,30 @@ case "${1:-}" in case "$state" in launched) if [ "${FM_FAKE_KIMI_READY:-yes}" = yes ]; then - printf 'ready\n' > "$FM_FAKE_KIMI_STATE" + case "${FM_FAKE_KIMI_TRUST:-remembered}" in + fresh) printf 'trust\n' > "$FM_FAKE_KIMI_STATE" ;; + decoy) printf 'trust-decoy\n' > "$FM_FAKE_KIMI_STATE" ;; + partial) printf 'trust-partial\n' > "$FM_FAKE_KIMI_STATE" ;; + late) printf 'booting\n' > "$FM_FAKE_KIMI_STATE" ;; + blink) printf 'banner-first\n' > "$FM_FAKE_KIMI_STATE" ;; + wrapped) printf 'trust-wrapped\n' > "$FM_FAKE_KIMI_STATE" ;; + *) printf 'ready\n' > "$FM_FAKE_KIMI_STATE" ;; + esac fi ;; + trust|trust-wrapped) + printf 'enter\n' >> "$FM_FAKE_KIMI_TRUST_ENTER_LOG" + trust_enters=$(wc -l < "$FM_FAKE_KIMI_TRUST_ENTER_LOG" | tr -d ' ') + case "${FM_FAKE_KIMI_TRUST_CLEARS:-yes}" in + yes) printf 'ready\n' > "$FM_FAKE_KIMI_STATE" ;; + after-second) + [ "$trust_enters" -lt 2 ] || printf 'ready\n' > "$FM_FAKE_KIMI_STATE" + ;; + esac + ;; + ready|delivered) + printf 'enter\n' >> "$FM_FAKE_KIMI_STRAY_ENTER_LOG" + ;; pointer-typed) if [ "${FM_FAKE_KIMI_DELIVERY:-yes}" = yes ]; then if [ "${FM_FAKE_KIMI_SWALLOW_FIRST:-no}" = yes ] \ @@ -121,6 +171,26 @@ case "${1:-}" in esac case "$arg" in -S|-E) prev=$arg ;; *) prev= ;; esac done + if [ "$start" = -0 ] && [ "${FM_FAKE_TMUX_VISIBLE_FAILS:-no}" = yes ]; then + echo "can't find pane" >&2 + exit 1 + fi + case "$start" in + -0|-120) + case "$state" in + booting) printf 'banner-only\n' > "$FM_FAKE_KIMI_STATE" ;; + banner-first) printf 'blank-frame\n' > "$FM_FAKE_KIMI_STATE" ;; + blank-frame) printf 'banner-only\n' > "$FM_FAKE_KIMI_STATE" ;; + banner-only) printf 'trust\n' > "$FM_FAKE_KIMI_STATE" ;; + esac + ;; + esac + if [ "$start" = -0 ] && [ "${FM_FAKE_KIMI_BLANK_AFTER_TRUST:-no}" = yes ] \ + && [ -s "$FM_FAKE_KIMI_TRUST_ENTER_LOG" ] && [ ! -f "$FM_FAKE_KIMI_BLANKED" ]; then + : > "$FM_FAKE_KIMI_BLANKED" + exit 0 + fi + [ "$start" != -120 ] || fake_history case "$start:$end" in *[!0-9:]*|'':*|*:'') fake_screen ;; *) fake_screen | awk -v start="$start" -v end="$end" \ @@ -161,6 +231,8 @@ EOF : > "$case_dir/launch.log" : > "$case_dir/pointer.log" : > "$case_dir/kimi.state" + : > "$case_dir/trust-enter.log" + : > "$case_dir/stray-enter.log" : > "$case_dir/tmux-calls.log" printf '%s\n' "$case_dir|$home|$proj|$wt|$fakebin" } @@ -175,11 +247,19 @@ run_spawn() { FM_FAKE_LAUNCH_LOG="$case_dir/launch.log" \ FM_FAKE_POINTER_LOG="$case_dir/pointer.log" \ FM_FAKE_KIMI_STATE="$case_dir/kimi.state" \ + FM_FAKE_KIMI_TRUST_ENTER_LOG="$case_dir/trust-enter.log" \ + FM_FAKE_KIMI_TRUST="${FM_FAKE_KIMI_TRUST:-remembered}" \ + FM_FAKE_KIMI_TRUST_CLEARS="${FM_FAKE_KIMI_TRUST_CLEARS:-yes}" \ + FM_FAKE_KIMI_HISTORY_KEEPS_DIALOG="${FM_FAKE_KIMI_HISTORY_KEEPS_DIALOG:-no}" \ + FM_FAKE_KIMI_STRAY_ENTER_LOG="$case_dir/stray-enter.log" \ + FM_FAKE_KIMI_BLANK_AFTER_TRUST="${FM_FAKE_KIMI_BLANK_AFTER_TRUST:-no}" \ + FM_FAKE_KIMI_BLANKED="$case_dir/kimi.blanked" \ + FM_FAKE_TMUX_VISIBLE_FAILS="${FM_FAKE_TMUX_VISIBLE_FAILS:-no}" \ FM_FAKE_KIMI_SWALLOWED="$case_dir/kimi.swallowed" \ FM_FAKE_KIMI_SWALLOW_FIRST="${FM_FAKE_KIMI_SWALLOW_FIRST:-no}" \ FM_FAKE_TMUX_CALL_LOG="$case_dir/tmux-calls.log" \ FM_FAKE_BRIEF_REAL="$(cd "$home/data/$id" && pwd -P)/launch-brief.md" \ - FM_KIMI_READY_POLLS=2 FM_KIMI_DELIVERY_POLLS=2 FM_KIMI_POLL_INTERVAL=0 \ + FM_KIMI_READY_POLLS="${FM_KIMI_READY_POLLS:-2}" FM_KIMI_DELIVERY_POLLS=2 FM_KIMI_POLL_INTERVAL=0 \ PATH="$fakebin:$BASE_PATH" \ "$SPAWN" "$id" "$proj" --harness kimi --mode no-mistakes --yolo off "$@" 2>&1 } @@ -522,6 +602,235 @@ test_kimi_readiness_gate_precedes_pointer() { pass "fm-spawn: kimi never sends the brief pointer before an observable ready signal" } +test_kimi_fresh_worktree_trust_is_answered_and_verified() { + local id rec out rc + id=kimi-trust-z9 + rec=$(make_spawn_case trust "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=3 FM_FAKE_KIMI_TRUST=fresh run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "fresh Kimi trust dialog should advance into verified delivery" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not continue after the trust dialog cleared" + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 1 ] \ + || fail "a Kimi trust dialog that cleared on its first answer was answered again" + assert_grep "Read the brief at " "$CASE_DIR/pointer.log" \ + "Kimi brief pointer was not delivered after trust and readiness verification" + pass "fm-spawn: a fresh Kimi worktree answers the exact trust dialog once and verifies advancement" +} + +test_kimi_swallowed_trust_enter_is_retried_until_the_dialog_clears() { + local id rec out rc + id=kimi-trust-swallow-y3 + rec=$(make_spawn_case trust-swallow "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=5 FM_FAKE_KIMI_TRUST=fresh FM_FAKE_KIMI_TRUST_CLEARS=after-second run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "a swallowed first trust Enter should be retried into a verified spawn" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not recover from a swallowed trust keypress" + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 2 ] \ + || fail "Kimi trust dialog was not re-answered exactly until it cleared" + assert_grep "Read the brief at " "$CASE_DIR/pointer.log" \ + "Kimi brief pointer was not delivered after the retried trust answer" + pass "fm-spawn: a swallowed Kimi trust keypress is re-sent until the dialog clears" +} + +test_kimi_banner_before_the_dialog_paints_does_not_pass_readiness() { + local id rec out rc + id=kimi-trust-late-y5 + rec=$(make_spawn_case trust-late "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=5 FM_FAKE_KIMI_TRUST=late run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "a banner captured before the dialog painted should wait, then trust and deliver" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not survive a banner captured before the trust dialog painted" + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi trust dialog painted after the banner was not answered exactly once" + [ "$(wc -l < "$CASE_DIR/pointer.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi brief pointer was not typed exactly once, after the dialog cleared" + assert_grep "Read the brief at " "$CASE_DIR/pointer.log" \ + "Kimi brief pointer was not delivered once the late dialog cleared" + pass "fm-spawn: a Kimi banner captured before the trust dialog paints does not read as ready" +} + +test_kimi_answered_dialog_left_in_history_does_not_restart_the_answer() { + local id rec out rc + id=kimi-trust-history-y6 + rec=$(make_spawn_case trust-history "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=3 FM_FAKE_KIMI_TRUST=fresh FM_FAKE_KIMI_HISTORY_KEEPS_DIALOG=yes run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "an answered trust dialog still in scrollback should not block the spawn" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not complete with the answered trust dialog still in scrollback" + case "$out" in + *"did not clear"*) fail "Kimi reported a stuck trust dialog that had already cleared" ;; + esac + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi answered the trust dialog again from its scrollback copy" + assert_grep "Read the brief at " "$CASE_DIR/pointer.log" \ + "Kimi brief pointer was not delivered past the scrollback copy of the dialog" + pass "fm-spawn: an answered Kimi trust dialog left in scrollback neither re-answers nor fails the spawn" +} + +test_kimi_blank_viewport_frame_costs_only_its_poll() { + local id rec out rc + id=kimi-trust-blank-y7 + rec=$(make_spawn_case trust-blank "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=4 FM_FAKE_KIMI_TRUST=fresh FM_FAKE_KIMI_BLANK_AFTER_TRUST=yes \ + FM_FAKE_KIMI_HISTORY_KEEPS_DIALOG=yes run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "a blank viewport frame should cost one poll, not the spawn" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not survive a blank viewport frame after the trust answer" + case "$out" in + *"did not clear"*) fail "a blank viewport frame was reported as a stuck trust dialog" ;; + esac + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi answered the trust dialog again after a blank viewport frame" + [ ! -s "$CASE_DIR/stray-enter.log" ] \ + || fail "Kimi sent a stray Enter into the live composer after a blank viewport frame" + assert_grep "Read the brief at " "$CASE_DIR/pointer.log" \ + "Kimi brief pointer was not delivered after the blank viewport frame" + pass "fm-spawn: a blank Kimi viewport frame costs its poll and nothing else" +} + +test_kimi_refuses_a_backend_without_a_viewport_capture() { + local id rec out rc + id=kimi-no-viewport-y8 + rec=$(make_spawn_case no-viewport "$id") + read_spawn_record "$rec" + fm_fake_exit0 "$FAKEBIN_DIR" cmux + rc=0 + out=$(FM_BACKEND=cmux run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + [ "$rc" -ne 0 ] || fail "a Kimi spawn on a backend without a viewport capture should refuse" + assert_contains "$out" "backend 'cmux' has no verified viewport-bounded capture" \ + "Kimi refusal did not name the backend and the missing viewport capability" + [ ! -s "$CASE_DIR/trust-enter.log" ] \ + || fail "Kimi pressed Enter on a backend it cannot read the viewport of" + [ ! -s "$CASE_DIR/launch.log" ] \ + || fail "Kimi was launched on a backend without a viewport capture" + pass "fm-spawn: Kimi refuses a backend that cannot read the viewport, before launching" +} + +test_kimi_answers_a_trust_dialog_with_a_wrapped_hint() { + local id rec out rc + id=kimi-trust-wrapped-y9 + rec=$(make_spawn_case trust-wrapped "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=3 FM_FAKE_KIMI_TRUST=wrapped run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "a trust dialog whose hint wrapped in a narrow pane should be answered" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not survive a trust dialog with a wrapped navigation hint" + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi did not answer a trust dialog with a wrapped hint exactly once" + assert_grep "Read the brief at " "$CASE_DIR/pointer.log" \ + "Kimi brief pointer was not delivered after the wrapped-hint dialog cleared" + pass "fm-spawn: a Kimi trust dialog with its hint wrapped across rows is answered normally" +} + +test_kimi_blank_frame_between_banners_restarts_the_ready_count() { + local id rec out rc + id=kimi-trust-blink-z4 + rec=$(make_spawn_case trust-blink "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=6 FM_FAKE_KIMI_TRUST=blink run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + expect_code 0 "$rc" "banners split by a blank frame should not read as two ready captures" + assert_contains "$out" "spawned $id harness=kimi" \ + "Kimi spawn did not wait out a blank frame before the trust dialog painted" + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi did not answer the trust dialog that painted after the blank frame" + [ "$(wc -l < "$CASE_DIR/pointer.log" | tr -d ' ')" = 1 ] \ + || fail "Kimi brief pointer was typed before the trust dialog painted" + pass "fm-spawn: a blank Kimi frame between banners restarts the two-capture ready count" +} + +test_kimi_failed_viewport_read_fails_readiness_at_once() { + local id rec out rc + id=kimi-viewport-fail-z5 + rec=$(make_spawn_case viewport-fail "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_KIMI_READY_POLLS=3 FM_FAKE_TMUX_VISIBLE_FAILS=yes FM_FAKE_KIMI_TRUST=fresh run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + [ "$rc" -ne 0 ] || fail "a Kimi spawn whose viewport read fails should fail" + assert_contains "$out" "could not read the visible viewport of backend 'tmux'" \ + "failed Kimi viewport read was reported as something other than a capture failure" + [ ! -s "$CASE_DIR/trust-enter.log" ] \ + || fail "Kimi pressed Enter without being able to read the viewport" + [ ! -s "$CASE_DIR/pointer.log" ] || fail "Kimi pointer was sent without a readable viewport" + pass "fm-spawn: a failed Kimi viewport read fails readiness with the backend named" +} + +test_kimi_partial_trust_dialog_blocks_the_ready_verdict() { + local id rec out rc + id=kimi-trust-partial-y4 + rec=$(make_spawn_case trust-partial "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_FAKE_KIMI_TRUST=partial run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + [ "$rc" -ne 0 ] || fail "a banner above an unanswered trust dialog should not pass readiness" + assert_contains "$out" "trust dialog text stayed on screen without the complete dialog" \ + "partially rendered Kimi trust dialog lacked its concrete failure reason" + [ ! -s "$CASE_DIR/pointer.log" ] \ + || fail "Kimi pointer was sent while trust dialog markers were still on screen" + [ ! -s "$CASE_DIR/trust-enter.log" ] \ + || fail "Kimi answered a trust dialog it could not fully read" + pass "fm-spawn: Kimi refuses the ready verdict while trust dialog markers remain" +} + +test_kimi_stuck_trust_dialog_fails_before_delivery() { + local id rec out rc + id=kimi-trust-stuck-y1 + rec=$(make_spawn_case trust-stuck "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_FAKE_KIMI_TRUST=fresh FM_FAKE_KIMI_TRUST_CLEARS=no run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + [ "$rc" -ne 0 ] || fail "a Kimi trust dialog that never clears should fail" + assert_contains "$out" "kimi trust dialog did not clear after selecting 'Trust this folder'" \ + "stuck Kimi trust dialog lacked its concrete failure reason" + assert_contains "$out" "navigation hint, selected 'Trust this folder'" \ + "stuck Kimi trust diagnostic did not name the observed dialog signals" + [ "$(wc -l < "$CASE_DIR/trust-enter.log" | tr -d ' ')" -gt 1 ] \ + || fail "stuck Kimi trust dialog was not re-answered while it stayed on screen" + [ ! -s "$CASE_DIR/pointer.log" ] || fail "Kimi pointer was sent through a stuck trust dialog" + assert_grep 'failed: kimi trust dialog did not clear' "$HOME_DIR/state/$id.status" \ + "stuck Kimi trust dialog did not leave a supervisor-visible failure" + pass "fm-spawn: a Kimi trust dialog must visibly clear before brief delivery" +} + +test_kimi_trust_detection_requires_the_complete_dialog() { + local id rec out rc + id=kimi-trust-decoy-y2 + rec=$(make_spawn_case trust-decoy "$id") + read_spawn_record "$rec" + rc=0 + out=$(FM_FAKE_KIMI_TRUST=decoy run_spawn \ + "$CASE_DIR" "$HOME_DIR" "$PROJ_DIR" "$WT_DIR" "$FAKEBIN_DIR" "$id") || rc=$? + [ "$rc" -ne 0 ] || fail "an incomplete Kimi trust lookalike should not pass readiness" + assert_contains "$out" "trust dialog text stayed on screen without the complete dialog" \ + "incomplete Kimi trust lookalike did not report the unanswerable dialog text" + [ ! -s "$CASE_DIR/trust-enter.log" ] \ + || fail "Kimi answered an incomplete trust lookalike with Enter" + [ ! -s "$CASE_DIR/pointer.log" ] || fail "Kimi pointer was sent through a trust lookalike" + pass "fm-spawn: Kimi trust detection requires every observed dialog signal" +} + test_kimi_detection_uses_ancestry_after_markers() { local dir fakebin cfg out dir="$TMP_ROOT/detection" @@ -693,6 +1002,18 @@ test_kimi_falls_back_to_expanded_home_binary test_kimi_missing_binary_refuses_before_pane_creation test_kimi_unconfirmed_delivery_fails_loudly test_kimi_readiness_gate_precedes_pointer +test_kimi_fresh_worktree_trust_is_answered_and_verified +test_kimi_swallowed_trust_enter_is_retried_until_the_dialog_clears +test_kimi_banner_before_the_dialog_paints_does_not_pass_readiness +test_kimi_answered_dialog_left_in_history_does_not_restart_the_answer +test_kimi_blank_viewport_frame_costs_only_its_poll +test_kimi_refuses_a_backend_without_a_viewport_capture +test_kimi_answers_a_trust_dialog_with_a_wrapped_hint +test_kimi_blank_frame_between_banners_restarts_the_ready_count +test_kimi_failed_viewport_read_fails_readiness_at_once +test_kimi_partial_trust_dialog_blocks_the_ready_verdict +test_kimi_stuck_trust_dialog_fails_before_delivery +test_kimi_trust_detection_requires_the_complete_dialog test_kimi_detection_uses_ancestry_after_markers test_kimi_session_lock_identity test_kimi_busy_signature_is_scoped_to_spinner_lines From 9bc051ff43c6e4d23c163ee8f1d87551a11050c0 Mon Sep 17 00:00:00 2001 From: Umer Date: Fri, 18 Sep 2026 07:05:19 +0400 Subject: [PATCH 003/237] fix(bin): report a dead-agent record once instead of escalating forever (#4775) * fix(bin): report a record whose agent is gone once instead of escalating forever The wedge escalation path never asked whether there was still an agent to be wedged. A wedge is something stuck that might recover, so re-alarming it earns its cost; an agent that is gone never moves again, its pane never churns, the idle timer never resets, and the escalate path clears its own timer and re-arms with nothing bounding the count. Observed on a live fleet: two finished lanes reached 226 and 203 consecutive escalations, roughly one every FM_STALE_ESCALATE_SECS, indefinitely - about 400 notifications a day from two lanes with no agent running at all. On one, fm-control.sh exit answered already-stopped and fm-crew-state.sh read "failed - run failed". Closing the Herdr pane did not stop it either: with the pane genuinely gone and herdr pane read returning pane_not_found, the count kept climbing, because the poll is driven by the record's window= line rather than by the pane. The cost is not the repetition but that it drowns the alarms that matter. fm_backend_agent_state already separates a thinking agent from a gone one at process level. In the branch that was about to escalate, read it once and treat only its two recovery-grade verdicts - dead (endpoint present, no agent in it) and missing (endpoint authoritatively absent) - as proof, reporting that record once and not re-escalating it while it stays that way. Every other verdict, including alive, ambiguous, unreadable, unverified, and a read that failed outright, keeps the identical schedule, reason, and escalation count, so a genuinely wedged live agent is unaffected. The probe costs at most one backend read per window per threshold, the same budget the declared-wait consult and the worktree write probe already take. The report decides nothing about the record's fate: both lanes still held unlanded work and teardown refusing them was correct, so retiring, relaunching, or cleaning up stays with the supervisor. The once-only marker is owned entirely by that function and is dropped by the same read the moment the endpoint stops reading gone, so a replacement launched into the same window escalates normally and its own later death is reported again. Related, and not closed by this: #4412, #4482, #4316. Tests drive the real watcher against a record whose endpoint does not exist and pin both directions: dead and missing report once and never advance the count across later thresholds, while alive, ambiguous, and unreadable endpoints keep escalating with the identical reason and a climbing count. * fix(bin): bind the once-only dead report to the pane it reported Review of the parent commit found a reachable sequence where a later death in the same window lost its promised report. The marker was keyed on the verdict string alone and dropped only when a threshold probe read a non-gone verdict, but probes run only at thresholds: a replacement launched into the same window that dies without ever being probed alive - it crashes at startup, or works and then crashes - was absorbed by the previous death's marker. The pane's first sight yielded only the generic stale wake and every later threshold matched the stale marker, so the second death never got the detailed once-report that both the function's own comment and docs/architecture.md promise. Record the verdict together with the pane hash it was reported for, and absorb a repeat only while both still match. A replacement churns the pane, which resets the stale suppressor, wedge timer, and escalation count while no reset site touches this marker, so the pane half is what tells the second death apart from the first. The live-probe drop stays as it was. Clearing the marker at those reset sites instead would re-open unbounded re-alarming for a dead pane whose display ever ticks, which is the exact defect the parent commit exists to close. The noise bound is unchanged: an unchanged dead pane still absorbs on every later threshold and never advances the escalation count, and every verdict short of proof still escalates exactly as before. * no-mistakes(review): Key the dead-record once-marker on the busy incarnation token * no-mistakes(document): Document dead-record escalation cap in stale-pane config entry * no-mistakes(document): Add busy-state inventory line to AGENTS.md * no-mistakes(document): Document dead-record probe on busy-turn-bound wedge path --- AGENTS.md | 3 +- bin/fm-watch.sh | 109 ++++++++++-- docs/architecture.md | 8 +- docs/configuration.md | 2 +- tests/fm-watch-triage.test.sh | 303 +++++++++++++++++++++++++++++++++- 5 files changed, 405 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 97d6b40d914..c4f62af7dbc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,6 +100,7 @@ state/ runtime records and signals; gitignored .status appended by crewmates: ": " wake-event lines, not current-state truth .turn-ended touched by turn-end hooks .progress touched for observed native-harness activity inside one Pi turn; bin/fm-busy-event.sh owns its generation binding and bin/fm-watch.sh reads it beside turn-ended for the busy-age bound only, never as a completed turn + .busy-state .busy-gen semantic busy-state record (one line, atomically replaced) and its per-incarnation gen sidecar; bin/fm-busy-event.sh is the only writer and bin/fm-busy-lib.sh owns the record format and classification; arming again replaces the previous incarnation so late events carrying its gen are rejected as stale; removed by retire and teardown .grok-turnend-token firstmate-owned grok hook registry token for the task; removed by teardown .kimi-turnend-token firstmate-owned Kimi hook registry token for the task; removed by teardown .gemini-settings.json firstmate-owned per-task Gemini settings carrying the busy-state and turn-end hooks, reached through GEMINI_CLI_SYSTEM_SETTINGS_PATH so nothing is written into the project's own .gemini/; removed by teardown @@ -148,7 +149,7 @@ state/ runtime records and signals; gitignored .watch.lock .wake-queue.lock watcher singleton and queue serialization locks .claude-autoarm.lock .claude-autoarm-epoch .claude-autoarm-failure-notified .claude-autoarm-failure-alarmed .turnend-claude-blocks .turnend-claude-blocks.lock Claude Stop auto-arm single-flight, epoch, failure-episode, attended-alarm, guard-budget, and budget-lock records; never touch .cursor-park-owner .cursor-park-owner.lock .turnend-cursor-blocks Cursor stop-hook owner record, publication and commit lock, and bounded repair-nag budget; never touch - .hash-* .count-* .stale-* .stale-since-* .churn-since-* .paused-* .wedge-escalations-* .writing-* .waiting-* .seen-* .hb-surfaced-* .last-* .heartbeat-streak watcher internals; never touch + .hash-* .count-* .stale-* .stale-since-* .churn-since-* .paused-* .wedge-escalations-* .dead-reported-* .writing-* .waiting-* .seen-* .hb-surfaced-* .last-* .heartbeat-streak watcher internals; never touch .watch-triage.log watcher's absorbed-wake debug log (size-capped); never relied on, safe to delete .last-watcher-beat watcher liveness beacon, touched every poll (including while absorbing benign wakes); guard scripts read it .subsuper-* .supervise-daemon.* sub-supervisor internals; never touch diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 5b529b382b2..7d27366eb6d 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -50,6 +50,11 @@ # the run step cannot show; that deferral still # re-surfaces once per PAUSE_RESURFACE_SECS, and a pane # that writes nothing keeps the unchanged schedule. +# A pane whose recorded endpoint holds no agent at all is +# not a wedge and is reported ONCE instead of escalating +# on that cadence forever (wedge_dead_record); only the +# two recovery-grade verdicts license it, and every other +# verdict escalates unchanged. # A genuinely busy pane # (window_is_busy true) is exempt from the above, but # only up to BUSY_TURN_MAX_SECS with no completed turn @@ -61,10 +66,11 @@ # plain reason once per declaration, while captain-held # work stays silent until return # (busy_turn_bound_check owns that split); -# 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 +# every other pane goes through the same wedge timer, +# the dead-record probe above included, and surfaces +# with the identical "stale: ..." reason, escalation +# count, and demand-deep-inspection marker for a live +# agent, for human inspection only - never an automatic # interrupt, signal, or restart of the worker or its # tool process. # stale: (unread firstmate instruction: ...) @@ -1024,6 +1030,73 @@ clear_write_tracking() { # rm -f "$STATE/.writing-since-$key" "$STATE/.writing-resurfaced-$key" } +# The question the wedge timer never asked before it alarmed: is there still an +# agent here to BE wedged? A wedge is something stuck that might recover, so +# re-alarming it earns its cost; an agent that is gone never moves again, its pane +# never churns, the idle timer never resets, and the escalate path below clears its +# own timer and re-arms with nothing bounding the count. +# docs/architecture.md owns that contract and why only these two verdicts license +# it; what the code needs stated here is the rest. +# +# fm_backend_agent_state (bin/fm-backend.sh) owns the vocabulary and the +# process-level proof behind it. Every verdict short of proof - `alive`, +# `ambiguous`, `unreadable`, `unverified`, or a read that failed outright - keeps +# the unchanged escalation schedule, reason and count, so this narrows WHICH panes +# escalate and never how loudly the ones that still do. +# +# Deliberately NOT a deferral like the two above it. They restart the idle timer +# because the pane might still be working; this is terminal for as long as the +# endpoint stays gone, because there is nothing left to re-probe on a cadence and a +# repeat is exactly the noise it exists to stop. WHICH verdict fired is named for +# the same reason wedge_wait_evidence names its verb: the two ask the supervisor +# for different things. +# +# The marker is owned entirely by this function and records the verdict together +# with the agent incarnation it was reported for: the task's per-incarnation busy +# gen (bin/fm-busy-lib.sh, state/.busy-gen), which changes exactly when the +# agent is replaced, so a repeat is absorbed only while BOTH still match, a read +# that stops being gone still drops it, and no other reset site has to know this +# file exists. The incarnation half re-arms a relaunch: a successor's own later +# death is reported in full even when its dead display hashes identically to the +# reported one. Only when no incarnation token is readable for the task does the +# pane hash stand in as the discriminator - an unreadable token must never mean +# re-report on every threshold, so that fallback keeps today's hash-keyed absorb, +# with the residual that a record-less successor dying into a byte-identical dead +# display stays absorbed. Under one unchanged incarnation a dead pane's static +# display absorbs on every threshold either way. +# Returns 0 when it has handled the window, 1 to escalate on the unchanged path. +wedge_dead_record() { # + local win=$1 since_file=$2 label=$3 age=$4 hash=$5 task=$6 key marker agent_state detail reason gen id + key=$(window_key "$win") + marker="$STATE/.dead-reported-$key" + agent_state=$(fm_backend_agent_state "$(window_backend "$win")" "$win" 2>/dev/null) || agent_state=unreadable + case "$agent_state" in + dead) detail='the endpoint is still there with no agent running in it' ;; + missing) detail='the recorded endpoint is gone' ;; + *) rm -f "$marker"; return 1 ;; + esac + # Re-arm the idle timer on BOTH paths below, so the backend probe above stays on + # its once-per-STALE_ESCALATE_SECS budget instead of running on every poll. + date +%s > "$since_file" + id=$hash + if gen=$(fm_busy_current_gen "$STATE" "$task"); then + id=$gen + fi + if [ "$(cat "$marker" 2>/dev/null || true)" = "$agent_state $id" ]; then + triage_log "absorbed $label (agent $agent_state, already reported once, idle ${age}s): $win" + return 0 + fi + reason="stale: $win (idle ${age}s, agent $agent_state - $detail, so this is not a wedge; reported once and not re-escalated while it stays that way - reconcile this record, and check for unlanded work before any cleanup)" + # Append before the marker, for the reason stale_wait_record gives: a marker + # written ahead of a failed append outlives it, and the next sighting would then + # absorb the retry - the one way this bound could swallow the report outright + # rather than deliver it once. + fm_wake_append stale "$win" "$reason" || exit 1 + printf '%s %s' "$agent_state" "$id" > "$marker" + clear_write_tracking "$key" + wake "$reason" +} + # Repeat-poll wedge-timer bookkeeping for an already-classified stale hash # absorbed as provably-working - repairs a missing/corrupt timer (self-heals a # watcher restart between recording the hash and recording the timer), or @@ -1032,13 +1105,16 @@ clear_write_tracking() { # # both places a hash can be absorbed this way: the plain non-terminal path, # and the stale_is_terminal-overridden path (a captain-relevant status-log # line that an active run/busy pane outranked). -# The wait-evidence consult (wedge_wait_evidence, one status-line read) and the -# worktree write probe run ONLY here, inside the at-threshold branch that is -# about to escalate: at most one each per window per STALE_ESCALATE_SECS, never -# per poll. The wait consult runs first, because a pane whose worker already said -# why it is quiet has nothing to prove through its worktree. -wedge_timer_check() { # - local win=$1 since_file=$2 label=$3 escalation_file=$4 task=$5 since age n reason evidence +# The wait-evidence consult (wedge_wait_evidence, one status-line read), the +# worktree write probe, and the dead-record probe (wedge_dead_record) run ONLY +# here, inside the at-threshold branch that is about to escalate: at most one each +# per window per STALE_ESCALATE_SECS, never per poll. The wait consult runs first, +# because a pane whose worker already said why it is quiet has nothing to prove +# through its worktree. The dead-record probe runs last of the three, so the two +# cheaper deferrals keep the panes they already own on their existing bounded +# cadences and only a pane that would otherwise alarm pays for a backend read. +wedge_timer_check() { # + local win=$1 since_file=$2 label=$3 escalation_file=$4 task=$5 hash=$6 since age n reason evidence since=$(cat "$since_file" 2>/dev/null || true) case "$since" in ''|*[!0-9]*) @@ -1059,6 +1135,9 @@ wedge_timer_check() { # /dev/null || echo 0) + 1 )) echo "$n" > "$escalation_file" reason="stale: $win (idle ${age}s, possible wedge, escalation $n)" @@ -1207,7 +1286,7 @@ busy_turn_bound_check() { # "$sf" - wedge_timer_check "$w" "$ssf" "non-terminal stale (provably working after a declared pause)" "$ewf" "$task" + wedge_timer_check "$w" "$ssf" "non-terminal stale (provably working after a declared pause)" "$ewf" "$task" "$h" triage_log "absorbed non-terminal stale (provably working): $w" ;; *) handle_paused_stale "$w" "$task" "$h" ;; esac else - wedge_timer_check "$w" "$ssf" "non-terminal stale" "$ewf" "$task" + wedge_timer_check "$w" "$ssf" "non-terminal stale" "$ewf" "$task" "$h" fi fi fi diff --git a/docs/architecture.md b/docs/architecture.md index e5e55c0be82..7cdc3ff6d5e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -32,7 +32,13 @@ A pane holding a file newer than the start of its own quiet window, anywhere in That deferral re-surfaces on the same `FM_PAUSE_RESURFACE_SECS` cadence as a declared wait, with a reason naming the write evidence rather than a wedge, and it is bounded to one pruned, depth-bounded, wall-clock-bounded walk (`FM_WORKTREE_WRITE_PRUNE`, `FM_WORKTREE_WRITE_MAXDEPTH`, `FM_WORKTREE_WRITE_TIMEOUT`) taken only in the branch that was about to escalate, never on every poll. Every absence of write evidence, including a missing worktree record, a torn-down worktree, a walk that outlives its wall-clock bound on a hung mount, and a failed walk, leaves the existing escalation schedule untouched, so a crew that writes nothing still escalates exactly as before. A secondmate's recorded worktree is never probed for write activity, because it is a provisioned firstmate home whose own supervision keeps writing inside it whether or not the mate produces anything, so its panes keep escalating on the unchanged schedule. -A busy pane is otherwise exempt from staleness, but only until its last completed turn or explicit native-harness progress reaches `FM_BUSY_TURN_MAX_SECS` (`bin/fm-watch.sh` owns marker selection); past that bound it is routed through the same wedge escalation, with the identical reason, escalation count, worktree-write deferral, and `demand-deep-inspection` marker, for inspection only - never an automatic interrupt, signal, or restart. +A pane whose recorded endpoint holds no agent at all is not a wedge suspect: a wedge is something stuck that might recover, while an agent that is gone never moves again, so its pane never churns, the idle timer never resets, and the escalation ladder had no ceiling at all - two finished lanes on one live fleet reached 226 and 203 consecutive escalations, roughly one every `FM_STALE_ESCALATE_SECS`, which is what drowns the alarms that matter. +In the same branch that was about to escalate, `bin/fm-backend.sh`'s recovery-grade `fm_backend_agent_state` is read once, and only its `dead` and `missing` verdicts - an endpoint still present with no agent running in it, and an endpoint authoritatively absent - report that record once and then stop re-escalating it while it stays that way. +Every other verdict, including `alive`, `ambiguous`, `unreadable`, `unverified`, and a read that failed outright, keeps the identical escalation schedule, reason, and count, so a genuinely wedged live agent is unaffected. +The report decides nothing about the record's fate, because such a lane routinely still holds unlanded work that teardown is right to refuse; retiring, relaunching, or cleaning it up stays with the supervisor. +The once-marker records the agent incarnation it was reported for - the task's per-incarnation busy gen (`state/.busy-gen`, minted by `bin/fm-busy-event.sh arm`, which changes exactly when the agent is replaced) - together with the verdict, so it re-arms when that endpoint reads live again and when the agent is replaced: a successor dying in the same window is reported again even when no threshold probe reads it alive in between and its dead display hashes identically to the one already reported. +When no busy incarnation token is readable for the task (it was never armed, or its sidecar is unreadable), the marker falls back to keying on the pane hash: that keeps the once-per-display absorb for a record-less task rather than re-reporting on every threshold, at the residual cost that such a successor dying into a byte-identical dead display stays absorbed. +A busy pane is otherwise exempt from staleness, but only until its last completed turn or explicit native-harness progress reaches `FM_BUSY_TURN_MAX_SECS` (`bin/fm-watch.sh` owns marker selection); past that bound it is routed through the same wedge escalation, with the identical reason, escalation count, worktree-write deferral, and `demand-deep-inspection` marker for a live agent and the same dead-record report when the endpoint is proven gone, for inspection only - never an automatic interrupt, signal, or restart. A crew that declared an external wait (`paused:`) or a verified captain-held transfer is the one exception to that bound: its busy verdict supplies liveness while identifying the long-running foreground call as the declared wait, so it takes the bounded `FM_PAUSE_RESURFACE_SECS` recheck instead of a wedge escalation, except that a captain-held transfer is not rechecked while the away-posture record exists. Lifting the declaration restores the unchanged busy-pane wedge path, while a pane that is no longer busy returns to the existing idle declared-wait classification. While the legacy daemon flag is active, a busy pane that crosses the bound under a declared external wait is handed to the daemon as the plain wake identity instead of taking that recheck in the watcher, because the daemon owns triage there and a wake already decorated as a possible wedge would override the daemon's own declared-wait verdict; an undeclared busy pane past the bound still takes the wedge escalation. diff --git a/docs/configuration.md b/docs/configuration.md index 988e909bb57..f23b741243e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1118,7 +1118,7 @@ FM_SIGNAL_GRACE=30 # seconds to coalesce nearby status and turn-end signals FM_TURNEND_CHURN_ABSORB_SECS=900 # longest one endpoint's bare turn-ends may be deferred on pane-churn evidence alone; only consulted when config/turnend-churn-absorb is present FM_CAPTAIN_RE='done:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' # captain-relevant status regex; nonterminal progress verbs remain excluded even when their prose matches FM_CLASSIFY_PAUSED_VERB=paused # leading status verb for a declared external wait; excluded from FM_CAPTAIN_RE and distinct from blocked -FM_STALE_ESCALATE_SECS=240 # idle seconds before a provably-working stale pane escalates, unless that pane's own worker declared a wait that has not elapsed, which takes the FM_PAUSE_RESURFACE_SECS recheck below instead; stale panes whose crew is not provably working surface immediately unless admitted directly to the declared-wait cadence, while a live idle declared wait still surfaces once before that cadence bounds repeats +FM_STALE_ESCALATE_SECS=240 # idle seconds before a provably-working stale pane escalates, unless that pane's own worker declared a wait that has not elapsed, which takes the FM_PAUSE_RESURFACE_SECS recheck below instead; stale panes whose crew is not provably working surface immediately unless admitted directly to the declared-wait cadence, while a live idle declared wait still surfaces once before that cadence bounds repeats; at that same escalation moment a recovery-grade agent-state probe (docs/architecture.md owns that dead-record contract) reports a pane whose endpoint is proven `dead` or `missing` once and stops re-escalating it while it stays that way FM_BUSY_TURN_MAX_SECS=3600 # maximum age without a completed turn or explicit native-harness progress (bin/fm-watch.sh owns marker selection), before the same wedge escalation used for a provably-working non-busy stale takes over; inspection-only, never an automatic interrupt or restart; a declared external wait or attended verified captain-held transfer takes the FM_PAUSE_RESURFACE_SECS recheck below instead FM_PAUSE_RESURFACE_SECS=14400 # four hours between bounded rechecks of a declared external wait or verified captain-held transfer, and between repeated new-hash stale alarms for an ordinary crew task with an open backlog captain call; a structured until time can make an external-wait recheck occur sooner but cannot extend this bound; this includes a live idle pane after its first inconclusive stale wake, a provably-working pane whose own unelapsed declared wait defers its FM_STALE_ESCALATE_SECS escalation, and a live busy pane past FM_BUSY_TURN_MAX_SECS, while the away-mode daemon uses the same setting and ages its window against the crew's own latest status line rather than pane busy state; a captain-held transfer is never rechecked while the away-posture record exists FM_SECONDMATE_WAKE_STALL_SECS=180 # minimum interval with no change of the oldest actionable foreign wake-queue row (it advances as the mate drains, and a queue reprovisioned under the same task id starts a fresh interval at whatever sequence it restarts) before an endpoint-recorded local secondmate produces one durable parent wake-loop-stall notification for that no-progress episode; a mate that is provably inside an active turn (an exact busy verdict) does not escalate until that same no-progress interval reaches FM_BUSY_TURN_MAX_SECS above, declared external-wait pause rows are excluded, and zero or invalid values use 180 diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index b4a904b27b2..29d6633cf12 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -2488,13 +2488,18 @@ test_live_paused_until_controls_recheck_time() { # the real 240s default only changes how long that takes. # `exit` requires the watcher to surface and exit, `absorb` requires it to # survive whole poll cycles at the threshold. Returns 1 when it does the other. +# The endpoint this lane's window resolves to is a live grok agent unless a case +# drives it elsewhere with FM_TEST_PANE_COMMAND (the pane's foreground command) +# and FM_TEST_TMUX_WINDOWS (the session inventory the recorded window must appear +# in), which is how the dead-endpoint cases below reach `dead` and `missing`. wedge_threshold_round() { # local state=$1 fakebin=$2 out=$3 capture=$4 window=$5 verdict=$6 mode=$7 pid cycles=0 PATH="$fakebin:$PATH" FM_FAKE_TMUX_WINDOW="$window" FM_FAKE_TMUX_CAPTURE="$capture" \ - FM_FAKE_TMUX_CURRENT_COMMAND=grok FM_FAKE_CREW_STATE="$verdict" \ + FM_FAKE_TMUX_CURRENT_COMMAND="${FM_TEST_PANE_COMMAND-grok}" \ + FM_FAKE_TMUX_WINDOWS="${FM_TEST_TMUX_WINDOWS-}" FM_FAKE_CREW_STATE="$verdict" \ FM_WATCH_HANDLING_SUCCESSOR=1 \ FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" \ - FM_PAUSE_RESURFACE_SECS="${FM_TEST_PAUSE_RESURFACE:-999}" FM_STALE_ESCALATE_SECS=1 \ + FM_PAUSE_RESURFACE_SECS="${FM_TEST_PAUSE_RESURFACE:-999}" FM_STALE_ESCALATE_SECS="${FM_TEST_STALE_ESCALATE:-1}" \ FM_POLL=1 FM_SIGNAL_GRACE=1 \ FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" >> "$out" & pid=$! @@ -2711,6 +2716,295 @@ test_wedge_threshold_recheck_names_the_captain_for_a_held_lane() { } +# --- a record whose agent is GONE reports once, instead of alarming forever --- +# Observed on a live fleet: two finished lanes reached 226 and 203 CONSECUTIVE +# wedge escalations, one alarm roughly every FM_STALE_ESCALATE_SECS, indefinitely - +# from lanes with no agent running at all. `bin/fm-control.sh exit` answered +# `already-stopped` and `bin/fm-crew-state.sh` read `failed - run failed`. Closing +# the pane did not stop it either: with the pane gone (`herdr pane read` -> +# `pane_not_found`) the count still climbed, because the poll is driven by the +# durable record's `window=` line, not by the pane. The escalate path clears its +# own idle timer and re-arms with nothing bounding the count, and a dead agent's +# pane never churns to reset it, so the ladder had no ceiling. The cost is not the +# repetition: it is that ~400 notifications a day from two finished lanes drown +# the alarms that matter, and the captain stopped reading them. +# +# fm_backend_agent_state already separated an agent that is THINKING from one that +# is gone; the escalation path simply never asked it. Both directions are pinned +# below, for the reason the declared-wait cases above give: a bound proved only in +# the quiet direction is indistinguishable from deleting wedge detection. +# Related, and deliberately NOT closed by this: upstream #4412, #4482, #4316. + +# The two endpoint verdicts that are PROOF an agent is gone, as the lane fixture +# above reaches them: `dead` is the recorded window still present in the session +# inventory with a bare shell in front of it (the husk a crashed agent leaves), +# and `missing` is an inventory that no longer carries that window at all. +gone_endpoint_env() { # -> assignments for the round below + case "$1" in + dead) FM_TEST_PANE_COMMAND=bash FM_TEST_TMUX_WINDOWS=fm-wedge ;; + missing) FM_TEST_PANE_COMMAND=bash FM_TEST_TMUX_WINDOWS=fm-someone-else ;; + esac +} + +test_gone_endpoint_reports_once_instead_of_escalating_forever() { + local dir state fakebin out capture window key verdict round + local failed='state: failed · source: run-step · run failed' + window="test:fm-wedge"; key=$(printf '%s' "$window" | tr ':/.' '___') + for verdict in dead missing; do + dir=$(wedge_threshold_fixture "gone-endpoint-$verdict" 'working: still compiling' 0) + state="$dir/state"; fakebin="$dir/fakebin"; out="$dir/watch.out"; capture="$dir/pane.txt" + gone_endpoint_env "$verdict" + export FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "a $verdict endpoint was never reported at the wedge threshold: $(cat "$out")" + grep -F "agent $verdict" "$out" >/dev/null \ + || fail "the $verdict report did not name the endpoint verdict: $(cat "$out")" + grep -F 'possible wedge' "$out" >/dev/null \ + && fail "a $verdict endpoint was still reported as a possible wedge: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 1 ] \ + || fail "a $verdict endpoint queued $(wedge_stale_wakes "$state" "$window") wakes instead of one" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "a $verdict endpoint advanced the wedge escalation count" + ack_stopped_cycle "$state" || fail "could not acknowledge the $verdict report" + + # The defect itself: every later threshold repeated the alarm, 226 times over. + # Each of these rounds is several thresholds, and every one must stay quiet. + round=1 + while [ "$round" -le 3 ]; do + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" absorb \ + || fail "a $verdict endpoint re-alarmed on later threshold $round: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 0 ] \ + || fail "a $verdict endpoint queued a repeat wake on round $round: $(cat "$state/.wake-queue")" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "a $verdict endpoint advanced the escalation count on round $round" + round=$((round + 1)) + done + unset FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + done + pass "a record whose endpoint is dead or missing reports itself once and is never re-escalated" +} + +# The load-bearing direction. A genuinely wedged LIVE agent must escalate exactly +# as it did before, and so must every verdict short of proof: an unattributable +# foreground process (`ambiguous`) and an unreadable endpoint keep the identical +# schedule, reason and count, because neither shows the agent is gone. +test_live_and_unproven_endpoints_still_wedge_escalate() { + local dir state fakebin out capture window key spec verdict comm inventory + local working='state: working · source: run-step · ci running' + window="test:fm-wedge"; key=$(printf '%s' "$window" | tr ':/.' '___') + for spec in 'alive|grok|fm-wedge' 'ambiguous|node|fm-wedge' 'unreadable||fm-wedge'; do + verdict=${spec%%|*}; comm=${spec#*|}; inventory=${comm#*|}; comm=${comm%%|*} + dir=$(wedge_threshold_fixture "wedge-live-$verdict" 'working: still compiling' 0) + state="$dir/state"; fakebin="$dir/fakebin"; out="$dir/watch.out"; capture="$dir/pane.txt" + FM_TEST_PANE_COMMAND=$comm FM_TEST_TMUX_WINDOWS=$inventory + export FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$working" exit \ + || fail "an $verdict endpoint stopped escalating at the wedge threshold: $(cat "$out")" + grep -F 'possible wedge, escalation 1' "$out" >/dev/null \ + || fail "an $verdict endpoint lost its wedge reason: $(cat "$out")" + [ "$(cat "$state/.wedge-escalations-$key" 2>/dev/null || true)" = 1 ] \ + || fail "an $verdict endpoint did not advance the escalation count" + ack_stopped_cycle "$state" || fail "could not acknowledge the $verdict escalation" + + # And it keeps escalating, with the count climbing exactly as it always did. + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$working" exit \ + || fail "an $verdict endpoint escalated only once: $(cat "$out")" + grep -F 'possible wedge, escalation 2' "$out" >/dev/null \ + || fail "an $verdict endpoint did not keep counting: $(cat "$out")" + ack_stopped_cycle "$state" || fail "could not acknowledge the second $verdict escalation" + unset FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + done + pass "a live wedged agent, an unattributable one, and an unreadable endpoint escalate unchanged" +} + +# Reporting once must not mean reporting once forever: a replacement launched into +# the same window has to get the full alarm back, and its own later death has to be +# reported again rather than silenced by the record of the first one. +test_gone_report_rearms_when_the_endpoint_comes_back() { + local dir state fakebin out capture window key + local failed='state: failed · source: run-step · run failed' + local working='state: working · source: run-step · ci running' + window="test:fm-wedge"; key=$(printf '%s' "$window" | tr ':/.' '___') + dir=$(wedge_threshold_fixture gone-rearm 'working: still compiling' 0) + state="$dir/state"; fakebin="$dir/fakebin"; out="$dir/watch.out"; capture="$dir/pane.txt" + + gone_endpoint_env missing; export FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "the gone endpoint was never reported: $(cat "$out")" + [ -s "$state/.dead-reported-$key" ] || fail "the once-only report left no record of itself" + ack_stopped_cycle "$state" || fail "could not acknowledge the first gone report" + + # A replacement is launched into the same window and then wedges for real. + FM_TEST_PANE_COMMAND=grok FM_TEST_TMUX_WINDOWS=fm-wedge + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$working" exit \ + || fail "a replacement agent's wedge was swallowed by the earlier gone report: $(cat "$out")" + grep -F 'possible wedge, escalation' "$out" >/dev/null \ + || fail "a replacement agent did not escalate as a wedge: $(cat "$out")" + [ ! -e "$state/.dead-reported-$key" ] \ + || fail "the once-only record survived an endpoint that reads live again" + ack_stopped_cycle "$state" || fail "could not acknowledge the replacement's wedge escalation" + + # And when the replacement dies too, that death is reported in full. + gone_endpoint_env dead + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "a second death in the same window was never reported: $(cat "$out")" + grep -F 'agent dead' "$out" >/dev/null \ + || fail "a second death was not reported as a gone endpoint: $(cat "$out")" + ack_stopped_cycle "$state" || fail "could not acknowledge the second gone report" + unset FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + pass "the once-only gone report re-arms when the endpoint comes back, and reports a later death again" +} + +# The swallow the once-marker must be bound against: death #1 is reported, then a +# replacement launches into the same window - churning the pane hash, which +# resets the stale suppressor, wedge timer and escalation count while NO reset +# site touches the once-marker - and then the replacement itself dies and the +# pane settles static at ITS hash. The relaunch round ends before any threshold, +# so no backend probe ever read the replacement alive; no incarnation token is +# armed for this fixture, so the marker's pane-hash fallback is all that can tell +# this death apart from the one already reported, and the second death must +# report in full, while later thresholds on the SAME dead pane stay +# silent and never advance the escalation count. +test_second_death_after_a_same_window_relaunch_reports_in_full() { + local dir state fakebin out capture window key + local failed='state: failed · source: run-step · run failed' + local working='state: working · source: run-step · ci running' + window="test:fm-wedge"; key=$(printf '%s' "$window" | tr ':/.' '___') + dir=$(wedge_threshold_fixture gone-relaunch-swallow 'working: still compiling' 0) + state="$dir/state"; fakebin="$dir/fakebin"; out="$dir/watch.out"; capture="$dir/pane.txt" + + # Death #1: the endpoint is gone and reported once, in full. + gone_endpoint_env missing; export FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "the first death was never reported: $(cat "$out")" + grep -F 'agent missing' "$out" >/dev/null \ + || fail "the first death report did not name the endpoint verdict: $(cat "$out")" + [ -s "$state/.dead-reported-$key" ] || fail "the first death left no once-record" + ack_stopped_cycle "$state" || fail "could not acknowledge the first death report" + + # A replacement launches: the pane churns and the bookkeeping resets, but the + # round ends before the fresh timer could reach a threshold, so no probe runs + # and the once-record survives the churn untouched. + FM_TEST_PANE_COMMAND=grok FM_TEST_TMUX_WINDOWS=fm-wedge + printf '%s\n' 'waiting on the build queue' > "$capture" + : > "$out" + FM_TEST_STALE_ESCALATE=999 wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$working" absorb \ + || fail "a replacement launch churned the pane without absorbing: $(cat "$out")" + grep -F 'possible wedge' "$out" >/dev/null \ + && fail "the relaunch round escalated before its fresh window elapsed: $(cat "$out")" + [ -s "$state/.dead-reported-$key" ] \ + || fail "the relaunch churn dropped the first death's once-record" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "the relaunch churn left a wedge escalation count behind" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 0 ] \ + || fail "the relaunch churn queued a wake: $(cat "$state/.wake-queue")" + + # The replacement dies too, without any intervening probe reading it alive: + # the second death must still produce its own detailed report naming the + # verdict, and must not be absorbed by the first death's record. + gone_endpoint_env missing + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "a second death after a same-window relaunch was never reported: $(cat "$out")" + grep -F 'agent missing' "$out" >/dev/null \ + || fail "the second death was not reported as a gone endpoint: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 1 ] \ + || fail "the second death queued $(wedge_stale_wakes "$state" "$window") wakes instead of one" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "the second death advanced the wedge escalation count" + ack_stopped_cycle "$state" || fail "could not acknowledge the second death report" + + # And later thresholds on the same unchanged dead pane stay silent: the + # bound still holds once the replacement's own death is the reported one. + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" absorb \ + || fail "an unchanged dead pane re-alarmed after the second report: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 0 ] \ + || fail "an unchanged dead pane queued a repeat wake: $(cat "$state/.wake-queue")" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "an unchanged dead pane advanced the escalation count" + unset FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + pass "a second death after a same-window relaunch reports in full without a live probe, and an unchanged dead pane stays silent" +} + +# The collision the pane-hash discriminator cannot see: a successor whose dead +# display is BYTE-IDENTICAL to the death already reported - the common case, +# since a dead husk display is deterministic (a bare shell in the same cwd, +# restored empty scrollback). The successor dies without any threshold probe +# reading it alive, so the pane never churns and no hash change can announce the +# replacement; only the busy incarnation, re-armed through the real writer +# (bin/fm-busy-event.sh arm, exactly as a relaunch replaces the previous one), +# can tell this death from the reported one. It must report in full, while later +# thresholds on the same dead pane under the SAME incarnation still absorb and +# never advance the escalation count. +test_identical_dead_display_of_a_successor_still_reports() { + local dir state fakebin out capture window key + local failed='state: failed · source: run-step · run failed' + window="test:fm-wedge"; key=$(printf '%s' "$window" | tr ':/.' '___') + dir=$(wedge_threshold_fixture identical-dead-display 'working: still compiling' 0) + state="$dir/state"; fakebin="$dir/fakebin"; out="$dir/watch.out"; capture="$dir/pane.txt" + + # The lane's busy contract is armed at spawn, so the first death's once-record + # is keyed on that incarnation. + "$ROOT/bin/fm-busy-event.sh" arm "$state" wedge >/dev/null \ + || fail "could not arm the lane's busy incarnation" + + # Death #1: the endpoint is gone and reported once, in full. + gone_endpoint_env missing; export FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "the first death was never reported: $(cat "$out")" + grep -F 'agent missing' "$out" >/dev/null \ + || fail "the first death report did not name the endpoint verdict: $(cat "$out")" + [ -s "$state/.dead-reported-$key" ] || fail "the first death left no once-record" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 1 ] \ + || fail "the first death queued $(wedge_stale_wakes "$state" "$window") wakes instead of one" + ack_stopped_cycle "$state" || fail "could not acknowledge the first death report" + + # A successor occupies the lane: the relaunch re-arms the busy incarnation + # through the real writer, and the successor stays quiet under the threshold + # for a round, so no probe reads it alive and the pane never churns - the + # display captured here and in the death rounds is byte-identical throughout. + "$ROOT/bin/fm-busy-event.sh" arm "$state" wedge >/dev/null \ + || fail "could not re-arm the successor's busy incarnation" + : > "$out" + FM_TEST_STALE_ESCALATE=999 wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" absorb \ + || fail "the successor's quiet round was never absorbed: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 0 ] \ + || fail "the successor's quiet round queued a wake: $(cat "$state/.wake-queue")" + + # The successor dies into the same byte-identical display. A pane-hash marker + # absorbs this death silently; the incarnation half must report it in full. + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" exit \ + || fail "a byte-identical dead display absorbed the successor's death: $(cat "$out")" + grep -F 'agent missing' "$out" >/dev/null \ + || fail "the successor's death was not reported as a gone endpoint: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 1 ] \ + || fail "the successor's death queued $(wedge_stale_wakes "$state" "$window") wakes instead of one" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "the successor's death advanced the wedge escalation count" + ack_stopped_cycle "$state" || fail "could not acknowledge the successor's death report" + + # Later thresholds on the same unchanged dead pane under the SAME incarnation + # stay silent: the once-only bound still holds within one incarnation. + : > "$out" + wedge_threshold_round "$state" "$fakebin" "$out" "$capture" "$window" "$failed" absorb \ + || fail "an unchanged dead pane re-alarmed under the same incarnation: $(cat "$out")" + [ "$(wedge_stale_wakes "$state" "$window")" -eq 0 ] \ + || fail "an unchanged dead pane queued a repeat wake: $(cat "$state/.wake-queue")" + [ ! -e "$state/.wedge-escalations-$key" ] \ + || fail "an unchanged dead pane advanced the escalation count" + unset FM_TEST_PANE_COMMAND FM_TEST_TMUX_WINDOWS + pass "a successor's byte-identical dead display reports in full, and the same incarnation still absorbs" +} + + # --- work the captain is already holding: pane churn must not re-alarm ------- # The other record of a legitimate wait. The declared-wait bound above reads the # status LINE, and a delivered task's line stays `done: PR ...` while the wait @@ -5196,6 +5490,11 @@ test_stale_terminal_status_overridden_by_active_run test_nonterminal_stale_provably_working_absorbed_then_escalated test_wedge_escalation_marks_demand_deep_inspection_after_threshold test_wedge_escalation_resets_when_pane_becomes_active +test_gone_endpoint_reports_once_instead_of_escalating_forever +test_live_and_unproven_endpoints_still_wedge_escalate +test_gone_report_rearms_when_the_endpoint_comes_back +test_second_death_after_a_same_window_relaunch_reports_in_full +test_identical_dead_display_of_a_successor_still_reports test_busy_pane_below_turn_age_bound_is_absorbed test_busy_pane_stable_hash_escalates_past_turn_age_bound test_busy_pane_changing_hash_escalates_past_turn_age_bound From daaffdb5116e264ca1e3b6bd2e0839a88adf5231 Mon Sep 17 00:00:00 2001 From: Jon Roosevelt Date: Fri, 18 Sep 2026 14:48:15 -0400 Subject: [PATCH 004/237] fix(bin): create captain-hold rows when Beads requires due (#4854) Captain holds have no due semantics and are a hold kind, not a Beads issue type. The create path now waives due.required and maps to native type task. Co-authored-by: Cursor --- bin/fm-captain-hold.sh | 25 ++++++++++++------ docs/configuration.md | 3 +++ tests/fm-captain-hold-lifecycle.test.sh | 34 +++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/bin/fm-captain-hold.sh b/bin/fm-captain-hold.sh index facc86505cb..0b7e3f9c1cb 100755 --- a/bin/fm-captain-hold.sh +++ b/bin/fm-captain-hold.sh @@ -40,12 +40,18 @@ # task first when no work item exists to hold (--title required to create; the # optional --origin records provenance in the new task's body and supplies the # default repo from that origin's metadata). Prefer holding the work item the -# question gates over minting a new row. The command records a UTC `Captain -# hold set:` timestamp in the task body: repeating an active hold preserves the -# existing timestamp, while re-holding released work starts a new lifecycle. -# A task already closed is refused rather than reopened. `--until` records the -# captain's own deferral date through `tasks-axi hold --until`, so a "revisit -# later" answer is stored as a date instead of a live card. +# question gates over minting a new row. Creating a missing row uses +# `tasks-axi add --kind captain`: that kind is backlog metadata, and the Beads +# adapter maps it to native issue type `task`. Captain holds have no due +# semantics (`--until` is the optional hold deferral), so the create waives +# Beads `due.required` through `BD_DUE_REQUIRED` rather than inventing a due +# date or registering a `types.custom` captain issue type. The command records +# a UTC `Captain hold set:` timestamp in the task body: repeating an active +# hold preserves the existing timestamp, while re-holding released work starts +# a new lifecycle. A task already closed is refused rather than reopened. +# `--until` records the captain's own deferral date through `tasks-axi hold +# --until`, so a "revisit later" answer is stored as a date instead of a live +# card. # # `answer` records the captain's exact words and resolves the call in the same # act. It requires a non-empty captain decision file of at most 8192 bytes and @@ -864,11 +870,14 @@ command_hold() { [ -n "$repo" ] || repo=firstmate validate_one_line repo "$repo" [ -z "$origin" ] || body=$(printf 'Origin: %s' "$origin") + # tasks-axi add never passes --due. Beads due.required would refuse this + # create, and captain holds have no due semantics, so waive it for this + # call only. --kind captain stays metadata; Beads native type is task. if [ -n "$body" ]; then - tasks_axi add "$id" "$title" --kind captain --repo "$repo" --body "$body" >/dev/null \ + BD_DUE_REQUIRED=false tasks_axi add "$id" "$title" --kind captain --repo "$repo" --body "$body" >/dev/null \ || fail "could not create task $id" else - tasks_axi add "$id" "$title" --kind captain --repo "$repo" >/dev/null \ + BD_DUE_REQUIRED=false tasks_axi add "$id" "$title" --kind captain --repo "$repo" >/dev/null \ || fail "could not create task $id" fi fi diff --git a/docs/configuration.md b/docs/configuration.md index f23b741243e..2ab5ddc66fd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -98,6 +98,9 @@ Both choices are local to each Firstmate home and are not part of secondmate inh The tracked `.tasks.toml` pins the default `tasks-axi` markdown backend to `data/backlog.md`, with `done_keep = 10` and an archive at `data/done-archive.md`. A home may instead select another tasks-axi adapter such as Beads through its own `.tasks.toml` or `TASKS_AXI_BACKEND`; firstmate still uses only tasks-axi verbs for routine backlog reads and mutations, and the adapter maps `start` and evidence-bearing `done` transitions to its native statuses and evidence fields. +Captain-hold row creation is owned by [`bin/fm-captain-hold.sh`](../bin/fm-captain-hold.sh) `hold`: when no work item exists, it creates an ordinary backlog row (`--kind captain` metadata; Beads native type `task`) and then applies the captain hold. +Captain rows have no Beads due semantics, so that create path waives a Beads `due.required` setting rather than passing a synthetic `--due`; `--until` remains the optional hold deferral. +Do not register a Beads `types.custom` `captain` type for this: captain is a hold kind, and the fleet Beads `due.required` policy for ordinary work stays in the federated beads config. When the automatic transition gate applies, dispatch and completion are not separate operator actions: each moves its work item inside the same run that creates or removes the task's record, so the ordinary successful path cannot leave the backlog and live task set out of sync ([`bin/fm-backlog-transition-lib.sh`](../bin/fm-backlog-transition-lib.sh)). Under that gate, dispatch accepts only an unheld, unblocked Queued or In flight item in this home; a missing, Done, held, or dependency-blocked item is refused before any endpoint or local copy is created. Completion refuses to report success until the item is closed, and session start reconciles this home's own books after an interrupted run. diff --git a/tests/fm-captain-hold-lifecycle.test.sh b/tests/fm-captain-hold-lifecycle.test.sh index fd496a82cd4..5f06db5b4b6 100755 --- a/tests/fm-captain-hold-lifecycle.test.sh +++ b/tests/fm-captain-hold-lifecycle.test.sh @@ -626,6 +626,39 @@ SH pass "captain-hold mutations address the beads backend without a markdown override" } +# A Beads workspace with due.required and no types.custom captain type is the +# live fleet shape. hold must still create a fresh captain row there: waive +# due rather than invent one, and map to native type task rather than register +# a Beads captain issue type. +test_hold_creates_a_captain_row_when_beads_requires_due_without_custom_type() { + local fixture home beads id show issue_type + require_tasks_axi_beads "captain-hold create under due.required without types.custom" || return 0 + fixture=$(make_beads_home due-required-no-custom-type) + home=${fixture%%|*} + beads=${fixture##*|} + printf '\ndue:\n required: true\n' >> "$beads/config.yaml" + if bdrow "$beads" create "raw task" --id fm-raw-task --type task --json >/dev/null 2>&1; then + fail "bd created a task without --due; the due.required fixture is not in force" + fi + if bdrow "$beads" create "raw captain" --id fm-raw-captain --type captain --due 2099-01-01 --json >/dev/null 2>&1; then + fail "bd accepted --type captain; the fixture still has a types.custom captain registration" + fi + id=fm-fresh-captain-call + run_captain "$home" hold "$id" --title "Choose the sample route" \ + --reason "captain must decide" --repo sample >/dev/null \ + || fail "hold could not create a captain row under due.required without types.custom" + show=$(tasks_in "$home" show "$id") || fail "the created captain row is missing" + assert_contains "$show" "hold_kind: captain" \ + "the created row is not captain-held" + assert_contains "$show" "kind: captain" \ + "the created row lost its captain backlog kind" + issue_type=$(bdrow "$beads" show "$id" --json \ + | jq -r 'if type == "array" then .[0].issue_type else .issue_type end') + [ "$issue_type" = task ] \ + || fail "create did not map to native Beads type task, got ${issue_type:-empty}" + pass "hold creates a captain row when Beads requires due and has no captain type" +} + # Reproduces the loss exactly with privacy-safe synthetic names: the investigation # and visual review have ended, the only genuine unresolved captain call is report # prose, no held backlog item or open status exists, and the authoritative @@ -4042,3 +4075,4 @@ test_complete_accepts_a_migrated_inventory_on_beads test_verify_names_the_unresolvable_legacy_id_once test_verify_resolves_a_pre_collapse_key_through_its_derived_marker test_captain_hold_mutations_address_the_beads_backend +test_hold_creates_a_captain_row_when_beads_requires_due_without_custom_type From 1bb72cc5f88014c86e3d03244efa0bb26c22d001 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:12:32 -0700 Subject: [PATCH 005/237] fix: disable compact adviser for spawned agents (#4877) * feat(bin): launch every spawned agent with the compact adviser disabled Every crewmate, scout, and secondmate Firstmate launches now starts with COMPACT_ADVISER_DISABLE=1, on a fresh spawn and on a relaunch alike, so an unattended session never activates the compact adviser. The value is unconditional: no configuration file gates it and there is no override, unlike the trace carrier beside it. Three carriers deliver it, because no single one covers every launch shape. The pane shell receives an export beside GOTMPDIR, so the agent's own children inherit it too. The launch command carries an explicit assignment, prepended outermost so it wins over any ambient value the pane already held. The cleared launch environment sets it again at the `env -i` boundary and keeps COMPACT_ADVISER_DISABLE in the fixed operational floor, which is what preserves the switch when config/launch-env-allowlist empties the environment, and what delivers it on a remote host that never had the value. bin/fm-control.sh relaunch, the bootstrap secondmate relaunch, and the remote secondmate transport all rebuild their launch through bin/fm-spawn.sh, so they inherit the same floor. The captain's own primary session is untouched. The two new suites drive the real spawn and then execute the launch command the pane actually received, with the harness replaced by a probe that prints its own environment, rather than matching script text. They cover ship and secondmate launches with the allowlist absent and enabled, the pane export and its ordering, fm-control.sh relaunch, and the full parent to remote-host chain. * no-mistakes(review): Export compact-adviser disable across compound launches * no-mistakes(document): Document spawned-agent compact-adviser environment guarantee --- bin/fm-spawn.sh | 37 +- bin/fm-test-run.sh | 2 + docs/configuration.md | 10 +- tests/fixtures.sh | 23 +- tests/fm-kimi-harness.test.sh | 4 +- ...awn-compact-adviser-disable-remote.test.sh | 187 ++++++++++ .../fm-spawn-compact-adviser-disable.test.sh | 346 ++++++++++++++++++ tests/fm-spawn-dispatch-profile.test.sh | 9 +- 8 files changed, 608 insertions(+), 10 deletions(-) create mode 100755 tests/fm-spawn-compact-adviser-disable-remote.test.sh create mode 100755 tests/fm-spawn-compact-adviser-disable.test.sh diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index fa8da51d9ea..9afb13960c2 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -244,7 +244,10 @@ # TMUX TMUX_PANE HERDR_ENV HERDR_SESSION HERDR_SOCKET_PATH HERDR_PANE_ID # CMUX_WORKSPACE_ID CMUX_SURFACE_ID CMUX_TAB_ID CMUX_PANEL_ID CMUX_SOCKET_PATH # ZELLIJ ZELLIJ_SESSION_NAME ZELLIJ_PANE_ID FM_ZELLIJ_SESSION, plus the task -# marker FM_TASK_ID that ship and scout panes receive above. +# marker FM_TASK_ID that ship and scout panes receive above, plus the +# compact-adviser kill switch COMPACT_ADVISER_DISABLE, which the floor also +# pins to 1 with a literal assignment so it survives the cleared environment +# even on a host that never had it set. # An enabled task trace also retains TRACEPARENT. Explicit Firstmate launch # assignments still apply inside the filtered environment. Raw commands must # be POSIX sh compatible under this opt-in; the absent-file path is unchanged. @@ -4412,6 +4415,19 @@ if [ "$KIND" = secondmate ]; then # injected carrier and this on/off snapshot are guaranteed to agree. LAUNCH="FM_ROOT_OVERRIDE= FM_STATE_OVERRIDE= FM_DATA_OVERRIDE= FM_PROJECTS_OVERRIDE= FM_CONFIG_OVERRIDE= FM_PUBLIC_FOLLOWUP_PRIMARY_HOME=$sq_primary_home FM_HOME=$sq_home FM_TRACE_CONTEXT=$SPAWN_TRACE_EFFECTIVE FM_SUPERVISION_MODEL=$supervision_model $LAUNCH" fi +# Every agent this fleet launches - crewmate, scout, and secondmate, on a fresh +# spawn and on a relaunch alike - runs with the compact-adviser kill switch on. +# This is an export statement rather than a forwarded ambient name or a +# command-prefix assignment, so it carries the value across an entire compound +# raw launch expression. A pane that never had it, and a remote host whose +# transport never carried it, both still start the agent with it set. It is +# unconditional, with no config file or flag gating it, and is inserted outside +# every generated launch prefix; relaunch trace cleanup may execute first but +# cannot change this value. The cleared-environment floor in the +# LAUNCH_ENV_PREFIX construction below sets it again at the `env -i` boundary, +# so under an enabled allowlist the switch is established before the wrapping +# `/bin/sh` starts rather than only inside the command that shell runs. +LAUNCH="export COMPACT_ADVISER_DISABLE=1; $LAUNCH" if [ -z "$SPAWN_TRACEPARENT" ] && [ "$RELAUNCH" -eq 1 ]; then LAUNCH="unset TRACEPARENT; $LAUNCH" fi @@ -4446,6 +4462,10 @@ spawn_record_traceparent() { # process (go build, go test, ...) inherit it. Sent before the launch command so # the env is set when the agent starts; the brief sleep lets the export land. spawn_send_text_line "$T" "export GOTMPDIR=$TASK_TMP/gotmp" +# Export the compact-adviser kill switch into the pane shell through the same +# pre-launch channel, so later commands in that shell inherit it too. The launch +# command independently establishes the value for the agent process itself. +spawn_send_text_line "$T" "export COMPACT_ADVISER_DISABLE=1" # Mark the pane as a task worker so bin/fm-test-run.sh can refuse to run the # suite in the repository's primary checkout. Ship and scout workers are the # ones assigned an isolated worktree; a secondmate runs its own home instead. @@ -4473,11 +4493,14 @@ if [ -n "$SPAWN_TRACEPARENT" ]; then fi if [ "$LAUNCH_ENV_ENABLED" = 1 ]; then LAUNCH_ENV_PREFIX='/usr/bin/env -i' + # COMPACT_ADVISER_DISABLE is the intentional declarative floor-membership + # entry; the explicit COMPACT_ADVISER_DISABLE=1 assignment below is the + # authoritative setter. for env_name in HOME PATH USER LOGNAME SHELL TERM COLORTERM LANG LC_ALL LC_CTYPE \ TMPDIR TMP TEMP GOTMPDIR TMUX TMUX_PANE HERDR_ENV HERDR_SESSION HERDR_SOCKET_PATH \ HERDR_PANE_ID CMUX_WORKSPACE_ID CMUX_SURFACE_ID CMUX_TAB_ID CMUX_PANEL_ID \ CMUX_SOCKET_PATH ZELLIJ ZELLIJ_SESSION_NAME ZELLIJ_PANE_ID FM_ZELLIJ_SESSION \ - FM_TASK_ID \ + FM_TASK_ID COMPACT_ADVISER_DISABLE \ $LAUNCH_ENV_NAMES; do # Only validated names enter shell syntax. Values expand once, quoted, in # the pane shell and never become source text or spawn-process snapshots. @@ -4485,6 +4508,16 @@ if [ "$LAUNCH_ENV_ENABLED" = 1 ]; then printf -v env_arg '${%s+"%s=$%s"}' "$env_name" "$env_name" "$env_name" LAUNCH_ENV_PREFIX="$LAUNCH_ENV_PREFIX $env_arg" done + # COMPACT_ADVISER_DISABLE is retained by the floor loop above, which forwards + # whatever the pane export set, and then pinned here to the one value Firstmate + # launches on. The literal assignment comes last deliberately: `env` applies + # assignments left to right, so this one wins over a forwarded pane value, and + # it still delivers the switch on a pane whose export never landed. Unlike the + # trace carrier below it carries no gate, so it is appended unconditionally. + # Setting it here rather than relying on the assignment already carried by + # $LAUNCH is what gives the wrapping `/bin/sh` itself the switch, not only the + # agent command it runs. + LAUNCH_ENV_PREFIX="$LAUNCH_ENV_PREFIX COMPACT_ADVISER_DISABLE=1" if [ -n "$SPAWN_TRACEPARENT" ]; then # shellcheck disable=SC2016 LAUNCH_ENV_PREFIX="$LAUNCH_ENV_PREFIX "'${TRACEPARENT+"TRACEPARENT=$TRACEPARENT"}' diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index b939101c943..44e8da93bcc 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -371,6 +371,8 @@ family_for_basename() { fm-send-inbox.test.sh|fm-spawn-batch.test.sh|\ fm-spawn-dispatch-profile.test.sh|fm-claude-trust.test.sh|\ fm-trace-context-spawn.test.sh|fm-spawn-worktree-settle.test.sh|\ + fm-spawn-compact-adviser-disable.test.sh|\ + fm-spawn-compact-adviser-disable-remote.test.sh|\ fm-teardown-endpoint-safety.test.sh) printf '%s\n' backend-dispatch ;; diff --git a/docs/configuration.md b/docs/configuration.md index 2ab5ddc66fd..67b868a49d0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -370,7 +370,7 @@ The [Claude adapter reference](../.agents/skills/harness-adapters/references/har ## Worker launch environment (config/launch-env-allowlist) The optional local, gitignored `config/launch-env-allowlist` limits the ambient environment passed to newly launched workers, scouts, and secondmates, including relaunches. -With no file, launch behavior is unchanged: selected harness markers are cleared, while the provider, long-lived terminal daemon, and shell initialization determine which other variables reach the worker. +With no file, ambient inheritance remains unfiltered: selected harness markers are cleared, while the provider, long-lived terminal daemon, and shell initialization determine which other variables reach the worker. Do not assume every worker inherits the invoking Firstmate process's current environment. The file is inherited into secondmate homes through the [primary-authoritative configuration contract](../.agents/skills/secondmate-provisioning/SKILL.md). Changes apply to subsequent launches; existing processes keep their environment. @@ -388,7 +388,7 @@ OPENAI_API_KEY SSH_AUTH_SOCK ``` -Firstmate retains basic home, executable search, terminal, locale, temporary-directory, and backend routing variables, plus its explicit launch assignments, its ship and scout task marker, and enabled task trace. +Firstmate retains basic home, executable search, terminal, locale, temporary-directory, and backend routing variables, plus its explicit launch assignments, its ship and scout task marker, the compact-adviser kill switch described below, and enabled task trace. [`fm-spawn.sh --help`](../bin/fm-spawn.sh) owns the exact retained names and parsing mechanics. Other ambient names must be listed explicitly, including custom credential-store locations, proxy settings, and certificate overrides when required by the selected tools. The command shell and worker may still create their own variables. @@ -413,6 +413,12 @@ The filter runs at the worker command boundary, after the terminal daemon and pa This is not a sandbox: it cannot revoke same-user access to credential files, prevent tools or later shells from loading credentials again, or isolate processes from the same user's other processes. Regression coverage executes emitted launch commands with synthetic nonsecret values in [`tests/fm-spawn-dispatch-profile.test.sh`](../tests/fm-spawn-dispatch-profile.test.sh). +Every crewmate, scout, and secondmate Firstmate launches starts with `COMPACT_ADVISER_DISABLE=1` in its environment, on a fresh spawn and on a relaunch alike, so an unattended session never activates the compact adviser. +This guarantee also covers raw launch commands, remote secondmates, and launches filtered by `config/launch-env-allowlist`; it does not depend on the destination environment already containing the variable. +Firstmate provides no configuration or flag to change this value. +This applies only to agents Firstmate launches; the captain's own primary Firstmate session is never given the variable. +[`fm-spawn.sh --help`](../bin/fm-spawn.sh) owns the delivery mechanics, with focused regression coverage in [`tests/fm-spawn-compact-adviser-disable.test.sh`](../tests/fm-spawn-compact-adviser-disable.test.sh) and [`tests/fm-spawn-compact-adviser-disable-remote.test.sh`](../tests/fm-spawn-compact-adviser-disable-remote.test.sh). + Every claude launch's inline `--settings` JSON also carries `"attribution":{"commit":"","pr":"","sessionUrl":false}`, so a spawned worker never writes a Co-Authored-By trailer, Claude-Session link, or generated-with line into a commit or PR body regardless of which settings scopes end up loaded. ## Crew dispatch profiles (config/crew-dispatch.json) diff --git a/tests/fixtures.sh b/tests/fixtures.sh index 043d350012e..a28ef4f9e29 100755 --- a/tests/fixtures.sh +++ b/tests/fixtures.sh @@ -94,7 +94,9 @@ fm_test_fake_gh_axi() { # fm_test_fake_tmux_spawn # Spawn-world tmux: pane_current_path from FM_FAKE_PANE_PATH, session named # firstmate, window ops succeed, send-keys succeed. When FM_FAKE_LAUNCH_LOG is -# set, each send-keys -l payload is appended one per line. Optional +# set, each send-keys -l payload is appended one per line. When FM_FAKE_PANE_LOG +# is set, each send-keys TEXT-LINE payload (the pre-launch pane exports, which +# carry no -l) is appended there instead, one per line in send order. Optional # FM_FAKE_DUPLICATE_WINDOW is printed from list-windows. # # The pane path defaults to empty when FM_FAKE_PANE_PATH is unset. Window @@ -127,6 +129,25 @@ case "${1:-}" in prev=$a done fi + # The pre-launch pane exports ride the text-line form + # (`send-keys -t Enter`), which carries no -l flag, so a + # suite that asserts on what the pane shell received opts in with its own + # log. Skip the flags, the target, and the trailing key so only the payload + # is recorded, one per line, in send order. + if [ -n "${FM_FAKE_PANE_LOG:-}" ]; then + shift + skip_next= + literal= + for a in "$@"; do + if [ -n "$skip_next" ]; then skip_next=; continue; fi + case "$a" in + -t) skip_next=1; continue ;; + -l) literal=1; continue ;; + Enter|C-m) continue ;; + *) [ -n "$literal" ] || printf '%s\n' "$a" >> "$FM_FAKE_PANE_LOG" ;; + esac + done + fi exit 0 ;; esac diff --git a/tests/fm-kimi-harness.test.sh b/tests/fm-kimi-harness.test.sh index e08674d3fe8..1cdba59392a 100755 --- a/tests/fm-kimi-harness.test.sh +++ b/tests/fm-kimi-harness.test.sh @@ -286,7 +286,7 @@ test_kimi_launch_then_send_is_verified() { assert_contains "$out" "spawned $id harness=kimi" "kimi spawn did not report success" launch=$(cat "$CASE_DIR/launch.log") - [ "$launch" = "env -u CURSOR_AGENT -u CURSOR_INVOKED_AS -u GEMINI_CLI '$FAKEBIN_DIR/kimi' --model 'kimi-code/k3' --auto" ] \ + [ "$launch" = "export COMPACT_ADVISER_DISABLE=1; env -u CURSOR_AGENT -u CURSOR_INVOKED_AS -u GEMINI_CLI '$FAKEBIN_DIR/kimi' --model 'kimi-code/k3' --auto" ] \ || fail "kimi launch did not use the absolute binary, model, and --auto only: $launch" assert_not_contains "$launch" "--effort" "kimi launch emitted a nonexistent effort flag" assert_not_contains "$launch" "turn-ended" "kimi launch embedded a turn-end path" @@ -545,7 +545,7 @@ test_kimi_falls_back_to_expanded_home_binary() { rc=$? expect_code 0 "$rc" "Kimi HOME fallback spawn should succeed" launch=$(cat "$CASE_DIR/launch.log") - [ "$launch" = "env -u CURSOR_AGENT -u CURSOR_INVOKED_AS -u GEMINI_CLI '$fallback' --auto" ] \ + [ "$launch" = "export COMPACT_ADVISER_DISABLE=1; env -u CURSOR_AGENT -u CURSOR_INVOKED_AS -u GEMINI_CLI '$fallback' --auto" ] \ || fail "Kimi fallback did not expand HOME into an absolute executable: $launch" pass "fm-spawn: Kimi fallback expands the active HOME" } diff --git a/tests/fm-spawn-compact-adviser-disable-remote.test.sh b/tests/fm-spawn-compact-adviser-disable-remote.test.sh new file mode 100755 index 00000000000..b4ce6f459b9 --- /dev/null +++ b/tests/fm-spawn-compact-adviser-disable-remote.test.sh @@ -0,0 +1,187 @@ +#!/usr/bin/env bash +# tests/fm-spawn-compact-adviser-disable-remote.test.sh - the compact-adviser +# kill switch must reach a second mate that Firstmate launches on another host. +# +# A remote second mate never reaches the local spawn path covered by +# tests/fm-spawn-compact-adviser-disable.test.sh: bin/fm-spawn.sh routes it +# through spawn_remote_secondmate, which hands the launch across the transport +# to the remote host's own fm-spawn. These assertions drive that real chain - +# parent fm-spawn -> fm-on -> the real remote entrypoint -> +# fm-remote-secondmate-control -> the remote host's fm-spawn - against a fake +# herdr CLI, so what the remote pane received is observable, and then execute +# that received command with a probe harness to read back the environment the +# remote agent would have started with. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# shellcheck source=tests/remote-herdr-fixture.sh +. "$(dirname "${BASH_SOURCE[0]}")/remote-herdr-fixture.sh" + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P) +TMP_ROOT=$(fm_test_tmproot fm-remote-compact-adviser) +mkdir -p "$TMP_ROOT" +TMP_ROOT=$(cd "$TMP_ROOT" && pwd -P) +PARENT="$TMP_ROOT/parent" +REMOTE_ROOT="$TMP_ROOT/remote-root" +REMOTE_HOME="$TMP_ROOT/remote-home" +FAKEBIN=$(fm_fakebin "$TMP_ROOT/fake") +PROBEBIN="$TMP_ROOT/probebin" +HERDR_LOG="$TMP_ROOT/remote-herdr.log" +HERDR_STATE="$TMP_ROOT/remote-herdr.state" +CLAIMS="$TMP_ROOT/claims" +mkdir -p "$PARENT/data" "$PARENT/state" "$PARENT/config" "$PARENT/projects" \ + "$REMOTE_ROOT" "$CLAIMS" "$PROBEBIN" "$TMP_ROOT/pane-home" +trap 'FM_HOME="$PARENT" FM_PROCEVENT_CLAIM_ROOT="$CLAIMS" "$ROOT/bin/fm-procevent.sh" sweep-home >/dev/null 2>&1 || true; if [ -f "$TMP_ROOT/remote-jobs/worker.pid" ]; then kill "$(cat "$TMP_ROOT/remote-jobs/worker.pid")" 2>/dev/null || true; fi; rm -rf -- "$TMP_ROOT"' EXIT + +# A synthetic value the remote launch must override rather than inherit, so a +# launch that only forwarded the ambient environment cannot pass as a floor. +CONTRARY=0 + +# The remote host's tracked code root is this branch, as a real git repository: +# fm-on and the remote entrypoint both require the dispatched command to be +# tracked there, and the remote side runs the real scripts under test. +( + cd "$ROOT" || exit + tar --exclude=.git --exclude=.no-mistakes --exclude=data --exclude=state --exclude=config -cf - . +) | (cd "$REMOTE_ROOT" && tar -xf -) + +# The remote host's own non-second-mate tooling only has to stay resolvable; +# the second mate itself always launches on Herdr, whose fixture logs every +# invocation verbatim. +cat > "$REMOTE_ROOT/bin/tmux" <<'SH' +#!/usr/bin/env bash +exit 0 +SH +chmod +x "$REMOTE_ROOT/bin/tmux" +install_remote_herdr_fixture "$REMOTE_ROOT" "$HERDR_STATE" "$HERDR_LOG" \ + "$TMP_ROOT/herdr-send-fail" "$TMP_ROOT/herdr.sock" +git -C "$REMOTE_ROOT" init -q -b main +git -C "$REMOTE_ROOT" config user.email test@example.com +git -C "$REMOTE_ROOT" config user.name Test +git -C "$REMOTE_ROOT" add . +git -C "$REMOTE_ROOT" commit -qm 'remote fixture root' + +cat > "$FAKEBIN/fake-ssh" <<'SH' +#!/usr/bin/env bash +while [ "$#" -gt 0 ]; do + case "$1" in -o) shift 2 ;; --) shift; break ;; *) exit 90 ;; esac +done +host=$1 +entry=$2 +shift 2 +[ "$host" = remote-mac ] || exit 91 +[ "$entry" = fm-remote-entrypoint.sh ] || exit 92 +cd "$FM_FAKE_REMOTE_CWD" || exit 93 +# The readiness gate is answered here rather than by the real doctor, which +# would inspect the RUNNER's own account; tests/fm-remote-doctor.test.sh owns +# the doctor's behavior against controlled account fixtures. +if printf '%s' "$4" | base64 --decode 2>/dev/null | tr '\0' '\n' | head -1 | grep -q '^fm-remote-doctor.sh$'; then + printf 'ok: remote second-mate readiness confirmed on this host\n' + exit 0 +fi +exec "$FM_FAKE_REMOTE_ENTRYPOINT" "$@" +SH +chmod +x "$FAKEBIN/fake-ssh" + +# The harness the remote pane would have started, replaced by a probe that +# reports the one environment fact under test. +cat > "$PROBEBIN/codex" <<'SH' +#!/bin/sh +printf '%s\n' "${COMPACT_ADVISER_DISABLE-unset}" +SH +chmod +x "$PROBEBIN/codex" + +printf 'codex\n' > "$PARENT/config/secondmate-harness" +printf 'tmux\n' > "$PARENT/config/backend" +printf 'codex\n' > "$PARENT/config/crew-harness" +printf '## In flight\n\n## Queued\n\n## Done\n' > "$PARENT/data/backlog.md" +printf '%s\n' "$$" > "$PARENT/state/.lock" + +remote_env() { + FM_HOME="$PARENT" \ + FM_ROOT_OVERRIDE="$REMOTE_ROOT" \ + FM_PROCEVENT_CLAIM_ROOT="$CLAIMS" \ + FM_SSH_BIN="$FAKEBIN/fake-ssh" \ + FM_FAKE_REMOTE_ENTRYPOINT="$REMOTE_ROOT/bin/fm-remote-entrypoint.sh" \ + FM_REMOTE_JOB_PLATFORM_OVERRIDE=Linux \ + FM_REMOTE_JOB_STATE_ROOT="$TMP_ROOT/remote-jobs" \ + FM_FAKE_REMOTE_CWD="$TMP_ROOT" \ + FM_SEND_SETTLE=0 FM_SEND_SLEEP=0 \ + "$@" +} + +# What the remote pane received, read back from the fixture's verbatim log. The +# fixture logs one line per invocation as the joined argv, so each payload sits +# between the pane id and the trailing session selector. The Herdr adapter sends +# a pre-launch export as a `pane run` line and the launch command itself as the +# unsubmitted literal `pane send-text`. +remote_pane_payload() { # + sed -n "s/^pane $1 [^ ]* \\(.*\\) --session [^ ]*\$/\\1/p" "$HERDR_LOG" +} +remote_launch_command() { + remote_pane_payload send-text | grep 'encode launch-brief' | tail -1 +} +remote_pane_exports() { + remote_pane_payload run | grep '^export ' +} + +# Provision and register the remote route from the captain-facing primary. +FM_SECONDMATE_CHARTER='Own iOS delivery on the build Mac.' \ + FM_SECONDMATE_SCOPE='iOS implementation and Xcode validation' \ + remote_env "$ROOT/bin/fm-remote-home-seed.sh" ios remote-mac "$REMOTE_ROOT" "$REMOTE_HOME" --no-projects >/dev/null \ + || fail "remote seed did not provision the route under test" + +run_remote_launch() { #