From 98d80a1616467b522d454261b0833aa538c467ca Mon Sep 17 00:00:00 2001 From: Tiago Date: Mon, 5 Oct 2026 11:26:56 -0300 Subject: [PATCH 01/13] feat(bin): record no-mistakes pipeline spend per task at teardown, opt-in (#5354) * feat(bin): record each task's no-mistakes pipeline spend at cleanup no-mistakes keeps every pipeline agent invocation's token usage only in its local agent_invocations records, and cold pipeline agents (review, test, document) leave no session log, so a task's review-loop cost never reached Firstmate's records and could not be attributed after cleanup. bin/fm-pipeline-spend.sh attributes a task's runs by the repository no-mistakes resolves for the task copy, the task branch, and the branch's creation (a relaunch mints a new spawn_gen while the branch keeps validating), then sums every invocation, failed and cancelled included. Token fields use no-mistakes' per-round deltas so resumed review rounds are not counted twice, and unrecorded values stay unknown rather than zero. show prints the record read-only; record appends it once per task incarnation to data/pipeline-spend.jsonl. Teardown records it for every ship task it cleans up, before deleting the branch and the task record. fm_nm_state_db becomes the one owner of where no-mistakes' state database lives, shared with the capped run-inventory reader. * no-mistakes(review): Drop spend show command and timeout override * style(bin): rewrap fm-pipeline-spend header comment * Make pipeline spend recording opt-in * feat(bin): record each task's no-mistakes pipeline spend at cleanup no-mistakes keeps every pipeline agent invocation's token usage only in its local agent_invocations records, and cold pipeline agents (review, test, document) leave no session log, so a task's review-loop cost never reached Firstmate's records and could not be attributed after cleanup. bin/fm-pipeline-spend.sh attributes a task's runs by the repository no-mistakes resolves for the task copy, the task branch, and the branch's creation (a relaunch mints a new spawn_gen while the branch keeps validating), then sums every invocation, failed and cancelled included. Token fields use no-mistakes' per-round deltas so resumed review rounds are not counted twice, and unrecorded values stay unknown rather than zero. show prints the record read-only; record appends it once per task incarnation to data/pipeline-spend.jsonl. Teardown records it for every ship task it cleans up, before deleting the branch and the task record. fm_nm_state_db becomes the one owner of where no-mistakes' state database lives, shared with the capped run-inventory reader. * no-mistakes(review): Drop spend show command and timeout override * style(bin): rewrap fm-pipeline-spend header comment * Make pipeline spend recording opt-in * no-mistakes(document): Note pipeline-spend opt-in gate in teardown comments * no-mistakes(ci): I fixed all three Greptile findings following your decision. Both test suites pass: tests/fm-teardown.test.sh (105 ok, exit 0) and tests/fm-pipeline-spend.test.sh (8 ok). I didn't run either new test against the old code, so the claim that they fail before the fix is from reading the code, not a run. Nothing was committed or pushed. **ci-1 / ci-2 (bin/fm-teardown.sh):** The rule is that every owned ship task in a home that has opted in gets a durable spend record before its task record is deleted. The `[ -d "$WT" ]` check skipped the recorder when the worktree folder was already gone, so no record was written. I removed that check and kept the other conditions: the task must be a ship task, teardown must own its worktree, and `config/pipeline-spend` must exist. `bin/fm-pipeline-spend.sh` already writes an unavailable-source line when the worktree is missing. The new test `test_teardown_records_unavailable_spend_for_a_gone_worktree` in tests/fm-teardown.test.sh sets up an owned ship task whose worktree is missing, with recording turned on. It checks that teardown succeeds, writes a line with `source=="unavailable"`, `total==null` and a reason saying the task copy is gone, and still removes the task record. Under the old check no ledger would have been written. **ci-3 (bin/fm-nm-run-lib.sh):** When `NM_HOME` and `HOME` were both unset, `fm_nm_state_db` fell back to `${HOME:-}/.no-mistakes`, which resolves to `/.no-mistakes`. It now uses `root=~/.no-mistakes`. Bash expands `~` to the account's home directory even when `HOME` is unset, which matches the previous Python `Path.home()` lookup. The new test `test_state_db_without_nm_home_or_home_uses_the_account_home` in tests/fm-pipeline-spend.test.sh runs the function with both variables unset and compares the result to the home directory from the system's password database. The old code would have returned `/.no-mistakes/state.sqlite`. shellcheck reports nothing new on the changed files. I added one `disable=SC2016` comment in the new test, where the single-quoted `$1` is meant to expand in the child shell --- .../skills/operational-home-layout/SKILL.md | 2 + bin/fm-nm-run-lib.sh | 27 +- bin/fm-pipeline-spend.sh | 298 +++++++++++++++ bin/fm-teardown.sh | 11 + bin/fm-test-run.sh | 6 +- docs/configuration.md | 8 +- docs/scripts.md | 1 + tests/fm-pipeline-spend.test.sh | 355 ++++++++++++++++++ tests/fm-teardown.test.sh | 95 +++++ 9 files changed, 791 insertions(+), 12 deletions(-) create mode 100755 bin/fm-pipeline-spend.sh create mode 100755 tests/fm-pipeline-spend.test.sh diff --git a/.agents/skills/operational-home-layout/SKILL.md b/.agents/skills/operational-home-layout/SKILL.md index f51340058bb..82fa94fc061 100644 --- a/.agents/skills/operational-home-layout/SKILL.md +++ b/.agents/skills/operational-home-layout/SKILL.md @@ -40,6 +40,7 @@ config/lavish-axi-host optional one-line per-machine Lavish server address; LOC config/brief-include.md optional standing worker instructions appended verbatim as the last section of every ship and scout scaffold; LOCAL, gitignored, and not inherited; keep its text out of `## Firstmate spec`; see docs/configuration.md "Home brief include" config/fleet-ledger optional presence flag opting this home in to the default-off fleet activity ledger state/fleet-ledger.jsonl that outside tools can follow; LOCAL, gitignored, and not inherited; see docs/fleet-ledger.md config/wait-no-turns optional presence flag opting this home into default-off waiting-worker behavior (brief waiting section, foreground pipeline drive, pending-reply hold, one fire-and-forget retry ring); LOCAL, gitignored, and not inherited; see docs/configuration.md "Waiting worker spends no turns" +config/pipeline-spend optional presence flag opting this home in to default-off per-task no-mistakes spend recording in data/pipeline-spend.jsonl; LOCAL, gitignored, and not inherited; see docs/configuration.md "No-mistakes pipeline spend" config/turnend-churn-absorb optional presence flag opting this home into the default-off absorb of bare turn-end wakes on pane churn; LOCAL, gitignored, and not inherited; see docs/configuration.md "Turn-end pane-churn absorb" config/wedge-defer-parked-gate optional presence flag opting this home into the default-off deferral of a wedge escalation for a lane parked at a validation gate awaiting the supervisor's own still-open decision; LOCAL, gitignored, and not inherited; see docs/configuration.md "Parked-gate wait deferral" config/cmux-socket-password optional cmux control-socket password; LOCAL, gitignored; read fresh on every cmux CLI call and passed through without ever overriding an operator's own ambient CMUX_SOCKET_PASSWORD when absent (docs/cmux-backend.md "Setup") @@ -55,6 +56,7 @@ data/ personal fleet records; LOCAL, gitignored as a whole secondmates.md local and remote secondmate routing table; firstmate-private, maintained by the secondmate seed helpers (section 6) /brief.md per-task crewmate brief, or per-secondmate charter brief when kind=secondmate /report.md scout task deliverable, written by the crewmate; survives teardown + pipeline-spend.jsonl optional per-task no-mistakes pipeline spend, written only when config/pipeline-spend is present; bin/fm-pipeline-spend.sh owns the schema projects/ cloned repos; gitignored; read-only except under hard rule 1's concrete captain-approved project operation exception state/ runtime records and signals; gitignored .status append-only wake events, not current-state truth; bin/fm-classify-lib.sh owns their syntax diff --git a/bin/fm-nm-run-lib.sh b/bin/fm-nm-run-lib.sh index f0e4e83b2ee..dcb188244e6 100644 --- a/bin/fm-nm-run-lib.sh +++ b/bin/fm-nm-run-lib.sh @@ -65,6 +65,20 @@ fm_nm_strip_quotes() { fm_nm_trim "$s" } +# Path of no-mistakes' local state database as the CLI would see it from +# worktree $1: /state.sqlite, with NM_HOME defaulting to +# ~/.no-mistakes and a relative NM_HOME resolving from that worktree. Readers +# open it with SQLite's mode=ro, so a missing database is never created. +fm_nm_state_db() { # + local root=${NM_HOME:-} + [ -n "$root" ] || root=~/.no-mistakes + case "$root" in + /*) ;; + *) root="$1/$root" ;; + esac + printf '%s/state.sqlite\n' "$root" +} + # Scalar value of a TOON key in captured `axi status` output $1. fm_nm_field() { # printf '%s\n' "$1" | sed -n "s/^[[:space:]]*$2:[[:space:]]*\(.*\)/\1/p" | head -1 @@ -126,8 +140,8 @@ fm_nm_run_status_class() { # # Select from a complete `no-mistakes axi` overview with the existing awk # toolchain. A capped overview requires an optional Python 3 sqlite3 reader -# for a read-only same-branch query of NM_HOME/state.sqlite (default: -# ~/.no-mistakes/state.sqlite; relative NM_HOME resolves from the worktree). +# for a read-only same-branch query of the state database fm_nm_state_db +# locates for the worktree. # Repo identity is the overview's own top-level `repo:` line, which every axi # release emits: it is the `working_path` the CLI itself resolved for the # queried worktree. That is NOT the task worktree path in general - a linked @@ -235,7 +249,7 @@ fm_nm_select_run() { # [timeout_secs] incomplete\|*) available_ids=${selection#*|} ;; *) printf '%s\n' "$selection"; return ;; esac - if ! inventory=$(fm_nm_bounded "$3" "$timeout_secs" python3 - "$1" "$2" "$3" "$available_ids" 2>/dev/null <<'PY' + if ! inventory=$(fm_nm_bounded "$3" "$timeout_secs" python3 - "$1" "$2" "$available_ids" "$(fm_nm_state_db "$3")" 2>/dev/null <<'PY' import json import os import re @@ -244,7 +258,7 @@ import sys from contextlib import closing from pathlib import Path -branch, overview, worktree, available_ids = sys.argv[1:] +branch, overview, available_ids, database = sys.argv[1:] ids = available_ids.split(", ") if available_ids else [] try: repos = [line[6:].strip() for line in overview.splitlines() if line.startswith("repo: ")] @@ -253,10 +267,7 @@ try: repo_path = json.loads(repos[0]) if repos[0].startswith('"') else repos[0] if not isinstance(repo_path, str) or not os.path.isabs(repo_path): raise ValueError - root = Path(os.environ.get("NM_HOME") or Path.home() / ".no-mistakes") - if not root.is_absolute(): - root = Path(worktree) / root - with closing(sqlite3.connect((root / "state.sqlite").as_uri() + "?mode=ro", uri=True, timeout=30)) as db: + with closing(sqlite3.connect(Path(database).as_uri() + "?mode=ro", uri=True, timeout=30)) as db: db.execute("BEGIN") repo = db.execute("SELECT id FROM repos WHERE working_path = ?", (repo_path,)).fetchall() if len(repo) != 1: diff --git a/bin/fm-pipeline-spend.sh b/bin/fm-pipeline-spend.sh new file mode 100755 index 00000000000..23f621c61c1 --- /dev/null +++ b/bin/fm-pipeline-spend.sh @@ -0,0 +1,298 @@ +#!/usr/bin/env bash +# fm-pipeline-spend.sh - attribute a task's no-mistakes pipeline spend to the +# task and keep it in Firstmate's own records. +# +# Usage: +# fm-pipeline-spend.sh record +# +# record appends the task's pipeline spend as one JSON object on one line of +# data/pipeline-spend.jsonl, at most once per task incarnation (task id plus +# the record's spawn_gen): repeating it for an incarnation already in the +# ledger appends nothing, so a retried cleanup never counts a task twice. +# Recording is disabled unless config/pipeline-spend is present; in that case +# this command exits before reading task metadata, no-mistakes state, or ledger. +# When enabled, bin/fm-teardown.sh calls record for every ship task whose +# local copy it cleans up, before it deletes the task branch this script +# attributes runs by and before it removes state/.meta. The ledger is +# private and gitignored with the rest of data/. +# Exit status: 0 when a record was recorded or already present, its source is +# unavailable, or recording is disabled; 1 when the task record is missing, +# names a secondmate, or the record could not be built or written; 2 for bad usage. +# +# Source. no-mistakes keeps each agent invocation's token usage only in its +# local state database, one agent_invocations row per invocation; its +# environment reference documents those fields, and `no-mistakes stats --run +# ` renders the same rows as a human table. There is no machine-readable +# export yet, so this script reads the database read-only (mode=ro), located +# by bin/fm-nm-run-lib.sh's fm_nm_state_db, and bounded by 30 seconds per +# no-mistakes or database call. +# +# Attribution. A task's runs are the runs no-mistakes recorded for the task +# copy's repository and current branch since that branch was created: +# - repository: the `repo:` line `no-mistakes axi` prints from the task copy, +# which is the CLI's own resolution (a pooled worker copy resolves to the +# registered primary clone), matched exactly against repos.working_path; +# - branch: the task copy's current branch, the one bin/fm-crew-state.sh +# reads; +# - since: the oldest surviving reflog entry of that branch. spawn_gen cannot +# bound the task, because a relaunch mints a new one while the same branch +# keeps validating. Teardown deletes the branch, so a later task that +# reuses the id and branch name starts a fresh reflog and never inherits an +# earlier task's runs. With no reflog, every run on the branch counts and +# since is null. +# Two live tasks sharing one branch name in one repository would both count +# its runs; each record lists run ids, so such an overlap stays visible. +# +# Counting. Every invocation of those runs counts, whatever its exit status +# (ok, error, cancelled), and each token field is no-mistakes' own, summed +# without reinterpretation (whether input includes cache reads differs by +# agent; see no-mistakes' environment reference): +# - input_tokens, output_tokens, cache_read_tokens sum the per-round +# delta_* columns, because a resumed session's raw counters are cumulative +# for some agents (codex) and summing them would count earlier review +# rounds again. A row with no delta (written before the delta columns +# existed) falls back to its raw counter only when its session_mode is not +# `resumed`, since no-mistakes defines a cold, started, or fallback row's +# delta as its raw counter. +# - cache_creation_tokens sums the raw counter, which has no per-round +# delta, so it counts only for a row whose counters are proven +# per-invocation: not resumed, or every delta equal to its raw counter. +# - reasoning is not summed: no-mistakes counts it inside output and keeps no +# per-round delta for it. +# A value no-mistakes did not record is unknown, never zero: each token field +# carries its known total and how many invocations were unknown, so a total +# with unknown > 0 is a lower bound. +# +# Record schema (this header is its one owner). One JSON object: +# task, spawn_gen the task id and the incarnation recorded in its meta +# recorded_at UTC time the record was built +# source "no-mistakes-state", or "unavailable" when the runs +# could not be read; reason then says why, total +# is null, and runs and purposes are empty +# repo, branch, since the attribution above (since as UTC time or null) +# total a tally over every counted invocation +# runs[] {id, status, created_at} plus a tally, oldest first +# purposes[] {purpose} plus a tally, by no-mistakes' purpose +# name (review, review-fix, test, document, ci, ...) +# A tally is {invocations, exit: {: count}, duration_ms, +# input_tokens, output_tokens, cache_read_tokens, cache_creation_tokens}, and +# each token field is {total, unknown}. +set -eu + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +FM_ROOT="${FM_ROOT_OVERRIDE:-$(cd "$SCRIPT_DIR/.." && pwd)}" +FM_HOME="${FM_HOME:-$FM_ROOT}" +STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" +DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" +CONFIG="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" +# shellcheck source=bin/fm-pr-lib.sh +. "$SCRIPT_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-nm-run-lib.sh +. "$SCRIPT_DIR/fm-nm-run-lib.sh" + +usage() { + sed -n '2,/^set -eu$/s/^# \{0,1\}//p' "$0" +} +fail() { + printf 'fm-pipeline-spend: %s\n' "$*" >&2 + exit 1 +} + +case "${1:-}" in + -h|--help) usage; exit 0 ;; + record) ;; + *) usage >&2; exit 2 ;; +esac +[ "$#" -eq 2 ] || { usage >&2; exit 2; } +ID=$2 +fm_task_id_path_safe "$ID" || { echo "fm-pipeline-spend: invalid task id" >&2; exit 2; } +[ -e "$CONFIG/pipeline-spend" ] || exit 0 +TIMEOUT=30 + +META="$STATE/$ID.meta" +[ -f "$META" ] && [ ! -L "$META" ] || fail "no task record for $ID" +meta_value() { # + grep "^$1=" "$META" 2>/dev/null | tail -1 | cut -d= -f2- || true +} +[ "$(meta_value kind)" != secondmate ] || fail "$ID is a secondmate, not a task" +command -v python3 >/dev/null 2>&1 || fail "python3 is required to read no-mistakes' state database" + +WT=$(meta_value worktree) +SPAWN_GEN=$(meta_value spawn_gen) +BRANCH= +SINCE= +REPO= +DB= +REASON= +if [ -z "$WT" ] || [ ! -d "$WT" ]; then + REASON="the task copy ${WT:-} is gone" +elif ! BRANCH=$(git -C "$WT" symbolic-ref --quiet --short HEAD 2>/dev/null) || [ -z "$BRANCH" ]; then + BRANCH= + REASON="the task copy is not on a branch" +elif ! command -v no-mistakes >/dev/null 2>&1; then + REASON="no-mistakes is not installed" +else + # `--format=%gd --date=unix` prints @{} newest first; the last + # line is the oldest surviving entry, normally the branch's creation. + SINCE=$(git -C "$WT" reflog show --date=unix --format=%gd "refs/heads/$BRANCH" -- 2>/dev/null \ + | tail -1 | sed -n 's/.*@{\([0-9][0-9]*\)}$/\1/p') || SINCE= + OVERVIEW=$(fm_nm_run_checked "$WT" "$TIMEOUT" axi) || true + REPO=$(fm_nm_strip_quotes "$(printf '%s\n' "$OVERVIEW" | sed -n 's/^repo:[[:space:]]*//p' | head -1)") + if [ -z "$REPO" ]; then + REASON="no-mistakes resolved no repository for the task copy" + FIRST_LINE=$(printf '%s\n' "$OVERVIEW" | sed -n '/^error:/{p;q;}') + [ -n "$FIRST_LINE" ] || FIRST_LINE=$(printf '%s\n' "$OVERVIEW" | sed -n '/[^[:space:]]/{p;q;}') + [ -z "$FIRST_LINE" ] || REASON="$REASON: $FIRST_LINE" + else + DB=$(fm_nm_state_db "$WT") + fi +fi + +[ -d "$DATA" ] || fail "data directory $DATA is missing" +LEDGER="$DATA/pipeline-spend.jsonl" + +RUN_DIR=$WT +[ -n "$RUN_DIR" ] && [ -d "$RUN_DIR" ] || RUN_DIR=$STATE +fm_nm_bounded "$RUN_DIR" "$TIMEOUT" python3 - "$LEDGER" "$ID" "$SPAWN_GEN" \ + "$REPO" "$BRANCH" "$SINCE" "$DB" "$REASON" <<'PY' || fail "could not build the pipeline spend record for $ID" +import fcntl +import json +import os +import sqlite3 +import sys +import time +from contextlib import closing +from pathlib import Path + +ledger, task, spawn_gen, repo, branch, since, database, reason = sys.argv[1:] +COUNTERS = ("input", "output", "cache_read") +TOKENS = tuple(f + "_tokens" for f in COUNTERS) + ("cache_creation_tokens",) + + +def utc(epoch): + return time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime(epoch)) + + +def tally(): + t = {"invocations": 0, "exit": {}, "duration_ms": 0} + for field in TOKENS: + t[field] = {"total": 0, "unknown": 0} + return t + + +def add(t, row, tokens): + t["invocations"] += 1 + t["exit"][row["exit_status"]] = t["exit"].get(row["exit_status"], 0) + 1 + t["duration_ms"] += row["duration_ms"] or 0 + for field in TOKENS: + if tokens[field] is None: + t[field]["unknown"] += 1 + else: + t[field]["total"] += tokens[field] + + +def invocation_tokens(row): + resumed = row["session_mode"] == "resumed" + proven = True # every per-round delta recorded and equal to its raw counter + tokens = {} + for counter in COUNTERS: + raw, delta = row[counter + "_tokens"], row["delta_" + counter + "_tokens"] + if delta is None: + proven = False + tokens[counter + "_tokens"] = None if resumed else raw + else: + proven = proven and raw == delta + tokens[counter + "_tokens"] = delta + per_invocation = not resumed or proven + tokens["cache_creation_tokens"] = row["cache_creation_tokens"] if per_invocation else None + return tokens + + +def read_spend(): + path = Path(database) + if not path.is_absolute(): + raise ValueError("state database path %s is not absolute" % database) + with closing(sqlite3.connect(path.as_uri() + "?mode=ro", uri=True, timeout=30)) as db: + db.row_factory = sqlite3.Row + db.execute("BEGIN") + columns = {r["name"] for r in db.execute("PRAGMA table_info(agent_invocations)")} + if not columns: + raise ValueError("no agent_invocations table") + repo_rows = db.execute("SELECT id FROM repos WHERE working_path = ?", (repo,)).fetchall() + if len(repo_rows) != 1: + raise ValueError("no repository %s" % repo) + runs = db.execute( + "SELECT id, status, created_at FROM runs WHERE repo_id = ? AND branch = ? AND created_at >= ? " + "ORDER BY created_at, id", + (repo_rows[0]["id"], branch, int(since) if since else 0), + ).fetchall() + wanted = ("run_id", "purpose", "session_mode", "exit_status", "duration_ms") + TOKENS + tuple( + "delta_" + c + "_tokens" for c in COUNTERS + ) + select = ", ".join(c if c in columns else "NULL AS " + c for c in wanted) + invocations = [] + for run in runs: + invocations.extend(db.execute( + "SELECT %s FROM agent_invocations WHERE run_id = ? ORDER BY started_at, id" % select, + (run["id"],), + ).fetchall()) + total = tally() + per_run = {run["id"]: tally() for run in runs} + per_purpose = {} + for row in invocations: + tokens = invocation_tokens(row) + add(total, row, tokens) + add(per_run[row["run_id"]], row, tokens) + add(per_purpose.setdefault(row["purpose"], tally()), row, tokens) + return { + "total": total, + "runs": [ + dict({"id": run["id"], "status": run["status"], "created_at": utc(run["created_at"])}, **per_run[run["id"]]) + for run in runs + ], + "purposes": [dict({"purpose": name}, **per_purpose[name]) for name in sorted(per_purpose)], + } + + +record = { + "task": task, + "spawn_gen": spawn_gen or None, + "recorded_at": utc(time.time()), + "source": "no-mistakes-state", + "reason": None, + "repo": repo or None, + "branch": branch or None, + "since": utc(int(since)) if since else None, +} +if not reason: + try: + spend = read_spend() + except (ValueError, OSError, sqlite3.Error) as err: + reason = "cannot read no-mistakes state: %s" % err +if reason: + record.update(source="unavailable", reason=reason, total=None, runs=[], purposes=[]) +else: + record.update(spend) +line = json.dumps(record, separators=(",", ":")) + +try: + fd = os.open(ledger, os.O_RDWR | os.O_CREAT | os.O_APPEND | os.O_NOFOLLOW, 0o600) + with os.fdopen(fd, "r+", encoding="utf-8") as f: + fcntl.flock(f, fcntl.LOCK_EX) + content = f.read() + for existing in content.splitlines(): + try: + row = json.loads(existing) + except ValueError: + continue + if isinstance(row, dict) and row.get("task") == task and row.get("spawn_gen") == record["spawn_gen"]: + print("already recorded %s %s" % (task, spawn_gen or "-")) + sys.exit(0) + f.write(("\n" if content and not content.endswith("\n") else "") + line + "\n") + f.flush() + os.fsync(f.fileno()) +except OSError as err: + sys.stderr.write("fm-pipeline-spend: cannot write %s: %s\n" % (ledger, err)) + sys.exit(1) +print("recorded %s %s" % (task, spawn_gen or "-")) +PY diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 8b9603d7e2d..c85b66f8502 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -292,6 +292,12 @@ # root still exists, so the account's healthy LaunchAgent worker and every # live remote secondmate worker are out of scope. Best effort: a sweep # failure never blocks this teardown. +# After Fix 1 and Fix 2, when config/pipeline-spend opts this home in, a ship +# task whose local copy this teardown owns has its no-mistakes pipeline spend +# recorded by bin/fm-pipeline-spend.sh, which owns the attribution and the +# ledger. It runs before the task branch it attributes runs by is deleted and +# before state/.meta is removed, and is best effort: a failure warns and +# never blocks cleanup. set -eu SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -3582,6 +3588,11 @@ if [ "$KIND" != secondmate ] && teardown_owns_worktree; then elif [ "$KIND" != secondmate ]; then reap_task_worktree_processes tasktmp "$TASK_TMP" fi +if [ "$KIND" = ship ] && teardown_owns_worktree && [ -e "$CONFIG/pipeline-spend" ]; then + FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" FM_DATA_OVERRIDE="$DATA" FM_CONFIG_OVERRIDE="$CONFIG" \ + "$SCRIPT_DIR/fm-pipeline-spend.sh" record "$ID" >/dev/null \ + || echo "warning: could not record $ID's no-mistakes pipeline spend; cleanup continues" >&2 +fi # Fix 3 (see script header): sweep remote job workers abandoned by an already # pruned code root. Best effort - a sweep failure never blocks this teardown. diff --git a/bin/fm-test-run.sh b/bin/fm-test-run.sh index 2b7044dc182..0676f6dc517 100755 --- a/bin/fm-test-run.sh +++ b/bin/fm-test-run.sh @@ -396,8 +396,8 @@ family_for_basename() { fm-teardown-endpoint-safety.test.sh) printf '%s\n' backend-dispatch ;; - fm-check-unregister.test.sh|fm-pr-check-security.test.sh|fm-pr-merge.test.sh|\ - fm-pr-reviewers.test.sh|fm-pr-state.test.sh|\ + fm-check-unregister.test.sh|fm-pipeline-spend.test.sh|fm-pr-check-security.test.sh|\ + fm-pr-merge.test.sh|fm-pr-reviewers.test.sh|fm-pr-state.test.sh|\ fm-review-diff.test.sh|fm-teardown.test.sh|fm-x-mode.test.sh) printf '%s\n' pr-forge ;; @@ -1603,7 +1603,7 @@ families_for_changed_path() { printf '%s\n' "__script__:fm-procevent-quota.test.sh" ;; bin/fm-pr-*|bin/fm-merge-local.sh|bin/fm-teardown.sh|bin/fm-review-diff.sh|\ - bin/fm-x-*|bin/fm-check*) + bin/fm-x-*|bin/fm-check*|bin/fm-pipeline-spend.sh) printf '%s\n' pr-forge ;; bin/fm-nm-run-lib.sh) diff --git a/docs/configuration.md b/docs/configuration.md index 5513cfabd6f..b37cc82c2aa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -77,7 +77,7 @@ Each effective `FM_HOME` contains private operational directories. - Project and secondmate registries. - Captain preferences and optional shared captain preferences. -- Learnings, backlog, briefs, and scout reports. +- Learnings, backlog, briefs, scout reports, and the optional per-task no-mistakes pipeline-spend ledger. - Explicitly installed content-addressed extension packages under `data/extensions/packages/`. `state/` holds runtime records: @@ -580,6 +580,12 @@ With it present, ship and scout briefs gain the `# Waiting` section and the fore With the file absent, generated briefs omit the waiting section and the no-poll inbox line, the drive text backgrounds the call, recovery sends during an open decision, and a fire-and-forget steer is not owed a retry ring. The flag is a home-local preference and is not inherited by secondmate homes. +## No-mistakes pipeline spend (config/pipeline-spend) + +The optional local, gitignored `config/pipeline-spend` presence flag opts this home into recording per-task no-mistakes pipeline spend in `data/pipeline-spend.jsonl` during teardown. +When the flag is absent, teardown skips recording and the recorder exits before reading task metadata, no-mistakes state, or the spend ledger. +An existing ledger is left untouched while recording is disabled. + ## Turn-end pane-churn absorb (config/turnend-churn-absorb) The optional local, gitignored `config/turnend-churn-absorb` presence flag opts this home into a default-off third form of positive work evidence in watcher triage. diff --git a/docs/scripts.md b/docs/scripts.md index 8d656ec203e..3e2f475c830 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -101,6 +101,7 @@ The shared no-mistakes gate lifecycle boundary is summarized in [architecture.md | `fm-supervise-daemon.sh` | Presence-gated away-mode sub-supervisor: self-handle routine wakes, guard injection by the detected primary harness, escalate batched digests, alert on failed delivery | | `fm-crew-state.sh` | Print one deterministic current-state line for a crew | | `fm-nm-run-lib.sh` | Single owner of shared no-mistakes run-attribution primitives and rules | +| `fm-pipeline-spend.sh` | Attribute a task's no-mistakes pipeline spend to the task and keep it in the private spend ledger | | `fm-tangle-lib.sh` | Shared default-branch resolution and primary-checkout tangle classification | | `fm-timeout-lib.sh` | Single owner of hard-bounded command execution and its fallback watchdog | | `fm-timing-lib.sh` | Single owner of the deferred network stage's per-step elapsed-time records, inert unless a run asks for them | diff --git a/tests/fm-pipeline-spend.test.sh b/tests/fm-pipeline-spend.test.sh new file mode 100755 index 00000000000..18006309d9c --- /dev/null +++ b/tests/fm-pipeline-spend.test.sh @@ -0,0 +1,355 @@ +#!/usr/bin/env bash +# Behavior tests for bin/fm-pipeline-spend.sh: a task's no-mistakes pipeline +# spend reaches Firstmate's own records, attributed to the task, through the +# script's public record command, reading back the ledger line it writes. Each +# enabled case seeds a real SQLite state database shaped like no-mistakes' own +# (repos, runs, agent_invocations) under +# a private NM_HOME, a real git task copy whose branch reflog starts at a known +# time, and a fake no-mistakes CLI that only names the resolved repository. +set -eu + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +fm_git_identity +TMP_ROOT=$(fm_test_tmproot fm-pipeline-spend) +SPEND="$ROOT/bin/fm-pipeline-spend.sh" +NOW=$(date +%s) +# The task branch is created well in the past so run timestamps can sit on +# either side of it; spawn_gen is minted later, as a relaunch would. +BRANCH_EPOCH=$((NOW - 100000)) +SPAWN_EPOCH=$((BRANCH_EPOCH + 5000)) +BRANCH_ISO=$(TZ=UTC0 date -d "@$BRANCH_EPOCH" +%Y-%m-%dT%H:%M:%SZ 2>/dev/null \ + || TZ=UTC0 date -r "$BRANCH_EPOCH" +%Y-%m-%dT%H:%M:%SZ) + +# make_case : echo a case dir holding a Firstmate home (home/state, +# home/data), a project clone, a task copy (wt) on branch fm/task created at +# BRANCH_EPOCH, the task's meta, an empty NM_HOME (nm), and a fake +# no-mistakes whose `axi` prints `repo: $FAKE_NM_REPO`, or the CLI's +# uninitialized-repository error when FAKE_NM_REPO is empty. +make_case() { + local d=$TMP_ROOT/$1 + mkdir -p "$d/home/state" "$d/home/data" "$d/home/config" "$d/nm" "$d/fakebin" + : > "$d/home/config/pipeline-spend" + fm_git_init_commit "$d/project" + GIT_COMMITTER_DATE="@$BRANCH_EPOCH +0000" git -C "$d/project" worktree add -q -b fm/task "$d/wt" + fm_write_meta "$d/home/state/task.meta" \ + "window=firstmate:fm-task" \ + "endpoint_task_id=task" \ + "worktree=$d/wt" \ + "project=$d/project" \ + "kind=ship" \ + "mode=no-mistakes" \ + "spawn_gen=s$SPAWN_EPOCH.1.abc" + cat > "$d/fakebin/no-mistakes" <<'SH' +#!/usr/bin/env bash +[ -z "${FAKE_NM_LOG:-}" ] || printf '%s\n' "$*" >> "$FAKE_NM_LOG" +[ "$*" = axi ] || exit 1 +if [ -n "${FAKE_NM_REPO:-}" ]; then + printf 'bin: no-mistakes\nrepo: %s\ncurrent_branch: fm/task\n' "$FAKE_NM_REPO" + exit 0 +fi +printf "error: repo not initialized (run 'no-mistakes init' first)\n" +printf 'help[1]: Run `no-mistakes init` to set up the gate in this repository\n' +exit 1 +SH + chmod +x "$d/fakebin/no-mistakes" + printf '%s\n' "$d" +} + +# seed_db : build nm/state.sqlite +# from stdin rows, "-" meaning NULL and times given as offsets from +# BRANCH_EPOCH: +# repo +# run +# inv +# current carries today's columns, pre-delta the original table from before +# no-mistakes added the delta columns, and no-invocations no table at all. +seed_db() { + SEED_ROWS=$(cat) python3 - "$1/nm/state.sqlite" "$2" "$BRANCH_EPOCH" <<'PY' +import os +import sqlite3 +import sys + +database, schema, base = sys.argv[1], sys.argv[2], int(sys.argv[3]) +db = sqlite3.connect(database) +db.executescript(""" + CREATE TABLE repos (id TEXT PRIMARY KEY, working_path TEXT NOT NULL UNIQUE); + CREATE TABLE runs (id TEXT PRIMARY KEY, repo_id TEXT NOT NULL, branch TEXT NOT NULL, + status TEXT NOT NULL, created_at INTEGER NOT NULL); +""") +tokens = ["input_tokens", "output_tokens", "cache_read_tokens", "cache_creation_tokens"] +deltas = ["delta_input_tokens", "delta_output_tokens", "delta_cache_read_tokens"] +columns = ["id", "run_id", "step_name", "round", "purpose", "agent", "session_mode", + "started_at", "completed_at", "duration_ms", "exit_status"] + tokens +if schema == "current": + columns += ["reasoning_tokens"] + deltas +if schema != "no-invocations": + db.execute("CREATE TABLE agent_invocations (%s)" % ", ".join(columns)) +value = lambda v: None if v == "-" else int(v) +for n, line in enumerate(os.environ["SEED_ROWS"].splitlines()): + f = line.split() + if not f: + continue + if f[0] == "repo": + db.execute("INSERT INTO repos VALUES (?, ?)", (f[1], f[2])) + elif f[0] == "run": + db.execute("INSERT INTO runs VALUES (?, ?, ?, ?, ?)", (f[1], f[2], f[3], f[4], base + int(f[5]))) + elif f[0] == "inv": + row = {"id": "inv%03d" % n, "run_id": f[1], "step_name": f[2], "round": 1, "purpose": f[2], + "agent": "fake", "session_mode": f[3], "started_at": base + n, "completed_at": base + n, + "duration_ms": int(f[5]), "exit_status": f[4], "reasoning_tokens": 999} + row.update(zip(tokens, map(value, f[6:10]))) + row.update(zip(deltas, map(value, f[10:13]))) + db.execute("INSERT INTO agent_invocations VALUES (%s)" % ", ".join("?" * len(columns)), + [row[c] for c in columns]) +db.commit() +PY +} + +# spend [task-id]: run the script against the case's +# home, NM_HOME, and fake no-mistakes. +spend() { + local d=$1 + env -u FM_STATE_OVERRIDE -u FM_DATA_OVERRIDE FM_HOME="$d/home" NM_HOME="$d/nm" \ + FAKE_NM_REPO="${FAKE_NM_REPO-$d/project}" FAKE_NM_LOG="$d/nm-invocations" PATH="$d/fakebin:$PATH" \ + "$SPEND" "$2" "${3:-task}" +} + +# recorded : record the case's task and print the ledger line it +# appended. +recorded() { + spend "$1" record >/dev/null && tail -1 "$1/home/data/pipeline-spend.jsonl" +} + +field() { # + printf '%s\n' "$1" | jq -cS "$2" +} + +test_known_spend_including_failed_and_cancelled_is_attributed_to_the_task() { + local d out + d=$(make_case known) + seed_db "$d" current <> "$ledger" + printf 'spawn_gen=s%s.2.def\n' "$NOW" >> "$d/home/state/task.meta" + spend "$d" record >/dev/null || fail "record failed for a new incarnation" + assert_equals 3 "$(wc -l < "$ledger" | tr -d ' ')" 'a new incarnation appends its own line' + assert_equals "\"s$NOW.2.def\"" "$(tail -1 "$ledger" | jq -c .spawn_gen)" 'the new line names the new incarnation' + + # An unavailable source is still recorded, so its absence is explicit. + d=$(make_case record-unavailable) + FAKE_NM_REPO='' spend "$d" record >/dev/null || fail "record failed for an unavailable source" + assert_equals '"unavailable"' "$(jq -c .source "$d/home/data/pipeline-spend.jsonl")" 'an unavailable record is kept' + pass 'record appends one line per task incarnation' +} + +test_disabled_record_does_not_read_or_create_spend_data() { + local d + d=$(make_case disabled) + rm -f "$d/home/config/pipeline-spend" "$d/home/state/task.meta" + rm -rf "$d/home/data" + spend "$d" record >/dev/null || fail "record failed while the feature was disabled" + assert_absent "$d/home/data/pipeline-spend.jsonl" 'disabled recording created spend data' + assert_absent "$d/nm-invocations" 'disabled recording called no-mistakes' + pass 'an absent opt-in flag bypasses task, pipeline, and ledger reads and writes' +} + +# With neither NM_HOME nor HOME set, the state database still resolves under +# the account's home directory, as the CLI's own lookup does, never /.no-mistakes. +test_state_db_without_nm_home_or_home_uses_the_account_home() { + local want got + want=$(python3 -c 'import os, pwd; print(pwd.getpwuid(os.getuid()).pw_dir)')/.no-mistakes/state.sqlite + # shellcheck disable=SC2016 # $1 expands in the child shell + got=$(env -u NM_HOME -u HOME bash -c '. "$1/bin/fm-nm-run-lib.sh"; fm_nm_state_db /wt' _ "$ROOT") + assert_equals "$want" "$got" 'an unset HOME falls back to the account home directory' + pass 'the state database resolves under the account home when NM_HOME and HOME are unset' +} + +test_refusals() { + local d rc + d=$(make_case refusals) + set +e + spend "$d" record missing >/dev/null 2>&1; rc=$? + set -e + expect_code 1 "$rc" 'a task with no record' + fm_write_meta "$d/home/state/mate.meta" "kind=secondmate" "worktree=$d/wt" + set +e + spend "$d" record mate >/dev/null 2>&1; rc=$? + set -e + expect_code 1 "$rc" 'a secondmate' + assert_absent "$d/home/data/pipeline-spend.jsonl" 'a refused record wrote the ledger' + set +e + spend "$d" record ../task >/dev/null 2>&1; rc=$? + set -e + expect_code 2 "$rc" 'an unsafe task id' + pass 'missing tasks, secondmates, and unsafe ids are refused' +} + +test_known_spend_including_failed_and_cancelled_is_attributed_to_the_task +test_repeated_review_rounds_in_a_resumed_session_are_not_double_counted +test_absent_spend_is_zero_or_unavailable_never_invented +test_older_state_without_delta_columns_counts_only_provable_rounds +test_record_appends_once_per_task_incarnation +test_disabled_record_does_not_read_or_create_spend_data +test_state_db_without_nm_home_or_home_uses_the_account_home +test_refusals diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 40f1ee03da7..462c3447202 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -121,6 +121,9 @@ SH # plain text with no run id and no quoting - see the ledger fixtures below), # and `runs` appends its own invocation to FM_FAKE_NM_RUNS_LOG when set, so # a test can prove whether the ledger fallback ever engaged. + # The bare `axi` overview answers FM_FAKE_AXI_OVERVIEW verbatim (empty by + # default, so no repository resolves and the pipeline-spend record is + # written as unavailable). # This keeps every case hermetic - without it, `command -v no-mistakes` # would fall through to whatever real binary happens to be on the test # runner's own PATH. Tests exercising the run-abort path override @@ -132,6 +135,8 @@ case "${1:-}" in axi) shift case "${1:-}" in + '') + printf '%s\n' "${FM_FAKE_AXI_OVERVIEW:-}" ;; status) shift run_id="" @@ -3187,6 +3192,93 @@ land_shippable_commit() { git -C "$case_dir/project" fetch -q origin } +# Cleanup keeps the task's no-mistakes pipeline spend in this home's records +# (bin/fm-pipeline-spend.sh) while the task branch that attributes its runs and +# the task record still exist, then removes both as before. +test_teardown_records_the_task_pipeline_spend() { + local case_dir rc=0 ledger + case_dir=$(make_case pipeline-spend) + write_meta "$case_dir" no-mistakes ship + : > "$case_dir/config/pipeline-spend" + land_shippable_commit "$case_dir" + mkdir -p "$case_dir/nm" + python3 - "$case_dir/nm/state.sqlite" "$case_dir/project" "$(date +%s)" <<'PY' +import sqlite3 +import sys + +database, project, created = sys.argv[1], sys.argv[2], int(sys.argv[3]) +db = sqlite3.connect(database) +db.executescript(""" + CREATE TABLE repos (id TEXT PRIMARY KEY, working_path TEXT NOT NULL UNIQUE); + CREATE TABLE runs (id TEXT PRIMARY KEY, repo_id TEXT NOT NULL, branch TEXT NOT NULL, + status TEXT NOT NULL, created_at INTEGER NOT NULL); + CREATE TABLE agent_invocations (id TEXT, run_id TEXT, purpose TEXT, session_mode TEXT, + started_at INTEGER, exit_status TEXT, duration_ms INTEGER, input_tokens INTEGER, + output_tokens INTEGER, cache_read_tokens INTEGER, cache_creation_tokens INTEGER, + delta_input_tokens INTEGER, delta_output_tokens INTEGER, delta_cache_read_tokens INTEGER); +""") +db.execute("INSERT INTO repos VALUES ('r1', ?)", (project,)) +db.execute("INSERT INTO runs VALUES ('01RUN', 'r1', 'fm/task-x1', 'completed', ?)", (created,)) +db.execute("INSERT INTO agent_invocations VALUES ('i1', '01RUN', 'review', 'cold', ?, 'ok', 100, 7, 8, 9, 10, 7, 8, 9)", + (created,)) +db.execute("INSERT INTO agent_invocations VALUES ('i2', '01RUN', 'review', 'cold', ?, 'cancelled', 50, " + "NULL, NULL, NULL, NULL, NULL, NULL, NULL)", (created + 1,)) +db.commit() +PY + ( + export NM_HOME="$case_dir/nm" FM_FAKE_AXI_OVERVIEW="repo: $case_dir/project" + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + ) || rc=$? + + expect_code 0 "$rc" "pipeline-spend: teardown should succeed" + ledger=$case_dir/data/pipeline-spend.jsonl + assert_present "$ledger" "pipeline-spend: teardown left no pipeline spend record" + jq -e ' + .task == "task-x1" and .spawn_gen == "teardown-test-task-x1" + and .source == "no-mistakes-state" and .branch == "fm/task-x1" + and [.runs[].id] == ["01RUN"] + and .total.invocations == 2 and .total.exit == {"ok": 1, "cancelled": 1} + and .total.input_tokens == {"total": 7, "unknown": 1} + ' "$ledger" >/dev/null || fail "pipeline-spend: the recorded spend is wrong: $(cat "$ledger")" + assert_absent "$case_dir/state/task-x1.meta" "pipeline-spend: teardown kept the task record" + ! git -C "$case_dir/project" show-ref --verify --quiet refs/heads/fm/task-x1 \ + || fail "pipeline-spend: teardown kept the task branch" + pass "teardown records the task's pipeline spend before removing its branch and record" +} + +test_teardown_skips_pipeline_spend_when_disabled() { + local case_dir rc=0 + case_dir=$(make_case pipeline-spend-disabled) + write_meta "$case_dir" no-mistakes ship + land_shippable_commit "$case_dir" + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$? + expect_code 0 "$rc" "pipeline-spend-disabled: teardown should succeed" + assert_absent "$case_dir/data/pipeline-spend.jsonl" \ + "pipeline-spend-disabled: teardown created a spend ledger without opt-in" + assert_absent "$case_dir/state/task-x1.meta" \ + "pipeline-spend-disabled: teardown kept the task record" + pass 'teardown skips all pipeline-spend recording when the home has not opted in' +} + +# An owned ship task whose local copy is already gone still leaves a durable +# account: the recorder writes an unavailable-source line before the record goes. +test_teardown_records_unavailable_spend_for_a_gone_worktree() { + local case_dir rc=0 ledger + case_dir=$(make_case pipeline-spend-gone) + write_windowless_legacy_meta "$case_dir" no-mistakes ship "$case_dir/missing-wt" + : > "$case_dir/config/pipeline-spend" + seed_backlog_in_flight "$case_dir" + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" || rc=$? + expect_code 0 "$rc" "pipeline-spend-gone: teardown should succeed" + ledger=$case_dir/data/pipeline-spend.jsonl + assert_present "$ledger" "pipeline-spend-gone: teardown left no pipeline spend record" + jq -e '.task == "task-x1" and .source == "unavailable" and .total == null + and (.reason | contains("is gone"))' "$ledger" >/dev/null \ + || fail "pipeline-spend-gone: the recorded spend is wrong: $(cat "$ledger")" + assert_absent "$case_dir/state/task-x1.meta" "pipeline-spend-gone: teardown kept the task record" + pass "teardown records unavailable pipeline spend for an owned ship task whose copy is gone" +} + test_parked_own_run_is_aborted_before_teardown() { local case_dir rc head case_dir=$(make_case parked-run-abort) @@ -4433,6 +4525,9 @@ test_transient_index_lock_clears_after_first_attempt_and_retry_succeeds test_persistent_index_lock_exhausts_retries_and_refuses_loudly test_empty_retry_wait_uses_default_without_aborting test_fractional_legacy_retry_wait_refuses_without_arithmetic_error +test_teardown_records_the_task_pipeline_spend +test_teardown_skips_pipeline_spend_when_disabled +test_teardown_records_unavailable_spend_for_a_gone_worktree test_parked_own_run_is_aborted_before_teardown test_parked_own_run_concludes_on_passed_with_override_after_abort test_parked_own_run_concludes_on_passed_with_skips_after_abort From 6a7b3919f9ef6b3eb2f3ce4b7c87bfdaa4514c82 Mon Sep 17 00:00:00 2001 From: lakshyads05 Date: Mon, 5 Oct 2026 21:24:03 +0400 Subject: [PATCH 02/13] feat(bin): start tasks from a named base branch (#6442) * feat(bin): start tasks from a named base branch Spawns always reset a task's pooled copy to origin's default branch, so work that belongs on a feature, integration, or release branch started from the wrong code and opened its PR against the default. fm-brief.sh --base-branch records the base in the brief, fm-spawn.sh resets the copy to origin/ and records base_branch= in task meta, and the worker targets its PR at that branch. Review diffs, the cleanup content check, and scout promotion read the recorded base. local-only and Gerrit deliveries refuse a named base. * no-mistakes(review): Anchor brief base-branch parse on scaffold Setup line * no-mistakes(review): Require explicit --base-branch on spawn, matching brief lines * no-mistakes(document): Note base-branch reset in fm-spawn freshness header * no-mistakes(ci): I fixed all four Greptile findings the way you specified. The affected suites pass locally: fm-brief, fm-dod-lib, fm-spawn-pool-base-freshen, fm-review-diff, fm-teardown and fm-task-delivery. shellcheck on the changed files reports only info-level notes, no warnings or errors. I didn't revert the code to watch each new test fail before the fix. - **ci-1 (brief text blocked or redirected spawns):** In bin/fm-dod-lib.sh, `fm_brief_base_branches` now only accepts a `Base branch: ` line that comes directly after the base-variant Setup sentence fm-brief.sh writes. Any other `Base branch:` line is ignored. `--base-branch` is still the only authority, and the existing checks in fm-spawn.sh are unchanged. With the flag, the spawn is refused unless a matching pair exists and no pair names a different branch. Without the flag, it is refused only if such a pair exists. The header comment in fm-spawn.sh is updated to match. - The `brief_with_base` test helper now writes the real two-line pair. - The decoy refusal case now uses a full decoy pair in the captain's intent. - New test `test_prose_base_branch_line_is_ignored`: a brief with no base whose intent quotes `Base branch: release/1.2` spawns normally with no `base_branch=` recorded. A spawn with `--base-branch feature/hub` whose intent names release/1.2 starts from `origin/feature/hub` and records it. - **ci-2 (scout base not checked against the forge):** fm-spawn.sh now looks up the project's registered forge with `bin/fm-project-mode.sh --forge`, like the ship path does, before `fm_base_branch_valid` runs. Previously it passed `none`. New test `test_scout_base_branch_refused_on_gerrit_forge`: a scout with a base on a forge=gerrit project is refused at spawn and no meta file is written. - **ci-3 (base not shell-quoted in worker commands):** `fm_dod_block` now quotes the base with `printf %q` in the `--base` and `--base-branch` arguments, and the branch name in the prose stays readable. The test in fm-brief.test.sh uses the base `release/$HOTFIX` and checks that both direct-PR and no-mistakes briefs render `release/\$HOTFIX` in the commands. - **ci-4 (ambiguous PR sentence):** The direct-PR sentence now reads "open a PR with `gh-axi` that is ready for review, not a draft, against the base branch `X` (`--base X`), not the repository default." A brief with no base renders exactly as before. The existing assertion in fm-brief.test.sh is updated * no-mistakes(ci): Both real failures on PR 6442 (run 37064756768) come from CI tooling and a Pi version bump, not from the base-branch feature. Fixes for both already exist on upstream main, so I applied those two commits to the working tree with `git cherry-pick --no-commit`. Nothing is committed or pushed, and no feature file changed. - **Lint 2:** ShellCheck ran out of memory on `bin/fm-teardown.sh` (rss about 8 GB, exit 251, "shellcheck: out of memory"). The rule that broke is that the lint gate must finish on any valid shell root without exceeding the runner's memory ceiling. Upstream commit d719ef3 (#6443) fixes this in `bin/fm-lint.sh`: a root that hits the memory ceiling is retried without `--external-sources`. It also updates `tests/fm-lint.test.sh` and `docs/fm-test-portable-shards.md`. - **Behavior portable serial 6:** `tests/fm-calm-pi-extension.test.sh` failed with "grep disappeared from /export calm.html HTML while calm mode was on". Pi 1.0.1 renamed its HTML export renderer lookup, and upstream commit fede619 (#6530) updates the test to accept the new name. This change is test-only. Both commits applied cleanly. The four touched files are `bin/fm-lint.sh`, `docs/fm-test-portable-shards.md`, `tests/fm-lint.test.sh` and `tests/fm-calm-pi-extension.test.sh`. None of them is in the feature diff, so the feature contract is unchanged. **Local checks:** - `tests/fm-lint.test.sh` passes. - `bin/fm-lint.sh` on `bin/fm-teardown.sh`, `bin/fm-spawn.sh` and `bin/fm-dod-lib.sh` exits 0 with full ShellCheck analysis. - `tests/fm-calm-pi-extension.test.sh` exits 0 with 7 ok and 0 not ok. One case skipped because the Pi package isn't installed locally, so the Pi 1.0.1 export path that failed in CI couldn't be reproduced here. CI needs to confirm it. - I didn't reproduce the 8 GB OOM locally. The PR has to be updated through the pipeline with a normal push: no force push, no replacement PR, no merge --- .agents/skills/scout-completion/SKILL.md | 2 +- AGENTS.md | 1 + bin/fm-brief.sh | 39 ++++++- bin/fm-dod-lib.sh | 70 ++++++++++-- bin/fm-promote.sh | 13 ++- bin/fm-review-diff.sh | 5 +- bin/fm-spawn.sh | 86 ++++++++++++--- bin/fm-teardown.sh | 9 +- docs/architecture.md | 1 + tests/fm-brief.test.sh | 71 ++++++++++++ tests/fm-dod-lib.test.sh | 43 ++++++++ tests/fm-review-diff.test.sh | 23 ++++ tests/fm-spawn-pool-base-freshen.test.sh | 135 +++++++++++++++++++++++ tests/fm-teardown.test.sh | 47 ++++++++ 14 files changed, 506 insertions(+), 39 deletions(-) diff --git a/.agents/skills/scout-completion/SKILL.md b/.agents/skills/scout-completion/SKILL.md index 3f3e98c9e87..de7759e6c1d 100644 --- a/.agents/skills/scout-completion/SKILL.md +++ b/.agents/skills/scout-completion/SKILL.md @@ -13,4 +13,4 @@ A report may recommend implementation but does not authorize it. Before treating the investigation or any visual review as complete, load `captain-hold-lifecycle`; teardown enforces that shared completion gate. When a scout's deliverable is a visual artifact the captain will iterate on, keep it alive and follow the crew-hosted Lavish board contract in `docs/configuration.md` rather than arming or polling the board from firstmate. When implementation is separately authorized, promote the existing scout through `bin/fm-promote.sh` rather than creating a duplicate task. -The promoted worker must inventory scratch state, return to a clean default-branch base, carry over only intended fix changes, create the ship branch, and follow the project's selected delivery path while leaving scratch commits and debug edits behind and turning a reproduced bug into the regression test. +The promoted worker must inventory scratch state, return to a clean copy of the task's base (its recorded base branch, else the default branch), carry over only intended fix changes, create the ship branch, and follow the project's selected delivery path while leaving scratch commits and debug edits behind and turning a reproduced bug into the regression test. diff --git a/AGENTS.md b/AGENTS.md index 507a7f51498..fc3fbb67cc7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -189,6 +189,7 @@ Resolve every ship task's concrete delivery mode and `yolo` merge posture at int Pass the mode explicitly to the brief, and pass both values explicitly to the spawn and any scout promotion; each command refuses to guess the values it consumes. A current explicit captain instruction wins; otherwise the project's registry entry is the captain's standing posture, and dropping below its rigor needs a reason you can state. Resolve the project's registered ship-branch prefix the same way, via `bin/fm-project-mode.sh --branch-prefix `, and pass it explicitly to the brief, ship spawn, and scout promotion as `--branch-prefix` (default `fm/` needs no flag). +When the work must start from and target a branch other than the project's default, such as a named feature or release branch, pass it to the ship or scout brief and spawn as `--base-branch `; any promotion reads it from task meta. On a `no-mistakes-prod-only` project, classify the task's surface: internal-only tooling, automation, contributor or operator process, and release or submission work ships `direct-PR`, while product-facing, mixed, and uncertain work ships `no-mistakes`; never infer internal-only from file location or project name. An unregistered project or absent registry resolves to `no-mistakes` with yolo off, and the registration gap goes to the captain. Record the resulting mode, `yolo` merge posture, and the one-line reason for any deviation in the backlog item note. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 871673d56bd..e3e56dde772 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -14,8 +14,8 @@ # charters still use a single `{TASK}` charter fill. Firstmate may adjust other # sections when the task genuinely deviates (e.g. working an existing external # PR instead of shipping a new one). -# Usage: fm-brief.sh --mode [--branch-prefix ] [--forge [--shape squash]] [--herdr-lab] -# fm-brief.sh --scout [--herdr-lab] +# Usage: fm-brief.sh --mode [--branch-prefix ] [--base-branch ] [--forge [--shape squash]] [--herdr-lab] +# fm-brief.sh --scout [--base-branch ] [--herdr-lab] # fm-brief.sh --secondmate {...|--no-projects} # --scout writes the scout contract instead: the deliverable is a report at # data//report.md (no branch, no push, no PR) and the worktree is scratch. @@ -59,6 +59,14 @@ # standing per-project preference, and firstmate resolves it per task at intake # and passes the explicit flag. Refused on --scout and --secondmate: a scout # makes no branch and a charter is not a delivery contract. +# --base-branch starts the task from origin's instead of the +# repository default, for work that belongs on a named integration, feature, or +# release branch. It writes a "Base branch: " line under `# Setup`, which +# bin/fm-spawn.sh requires to agree with the same --base-branch it is passed to +# choose the copy's starting point, and a ship's +# Definition of done then targets that branch with its pull request. +# bin/fm-dod-lib.sh's fm_base_branch_valid owns which deliveries accept one. +# Refused on --secondmate. # --forge names the project's forge, defaults to none, and is orthogonal to --mode # exactly as the registry's `forge=` token is. It is the captain's confirmed # registry binding, read from data/projects.md at intake and passed here; this @@ -187,6 +195,8 @@ MODE= MODE_SET=0 BRANCH_PREFIX=fm/ BRANCH_PREFIX_SET=0 +BASE_BRANCH= +BASE_BRANCH_SET=0 FORGE=none FORGE_SET=0 SHAPE= @@ -201,6 +211,7 @@ for a in "$@"; do case "$want_value" in mode) MODE=$a; MODE_SET=1 ;; branch-prefix) BRANCH_PREFIX=$a; BRANCH_PREFIX_SET=1 ;; + base-branch) BASE_BRANCH=$a; BASE_BRANCH_SET=1 ;; forge) FORGE=$a; FORGE_SET=1 ;; shape) SHAPE=$a; SHAPE_SET=1 ;; *) echo "error: internal parser state for --$want_value" >&2; exit 1 ;; @@ -217,6 +228,8 @@ for a in "$@"; do --mode=*) MODE=${a#--mode=}; MODE_SET=1 ;; --branch-prefix) want_value="branch-prefix" ;; --branch-prefix=*) BRANCH_PREFIX=${a#--branch-prefix=}; BRANCH_PREFIX_SET=1 ;; + --base-branch) want_value="base-branch" ;; + --base-branch=*) BASE_BRANCH=${a#--base-branch=}; BASE_BRANCH_SET=1 ;; --forge) want_value=forge ;; --forge=*) FORGE=${a#--forge=}; FORGE_SET=1 ;; --shape) want_value=shape ;; @@ -280,6 +293,13 @@ elif [ "$FORGE_SET" -eq 1 ] || [ "$SHAPE_SET" -eq 1 ]; then echo "error: --forge and --shape apply only to ship briefs; a scout delivers a report and a secondmate charter is not a delivery contract" >&2 exit 1 fi +if [ "$BASE_BRANCH_SET" -eq 1 ]; then + if [ "$KIND" = secondmate ] || [ -z "$BASE_BRANCH" ]; then + echo "error: --base-branch takes a branch name and applies only to ship and scout briefs" >&2 + exit 1 + fi + fm_base_branch_valid "$BASE_BRANCH" "$MODE" "$FORGE" "fm-brief.sh --base-branch" || exit 1 +fi ID=${POS[0]} BRANCH="$BRANCH_PREFIX$ID" if ! git check-ref-format --branch "$BRANCH" >/dev/null 2>&1; then @@ -566,6 +586,13 @@ IFS= read -r -d '' SHARED_INFRA_RULE <<'EOF' || true EOF SHARED_INFRA_RULE=${SHARED_INFRA_RULE%$'\n'} +if [ -n "$BASE_BRANCH" ]; then + SETUP_BASE="You are in a disposable git worktree of $REPO, at a detached HEAD on a clean copy of its base branch. +Base branch: $BASE_BRANCH" +else + SETUP_BASE="You are in a disposable git worktree of $REPO, at a detached HEAD on a clean default branch." +fi + if [ "$KIND" = scout ]; then if "$SCRIPT_DIR/fm-bootstrap.sh" lavish-compatible >/dev/null 2>&1; then LAVISH_LINE='If your deliverable is a visual artifact the captain will review and iterate on, use the lavish-axi rule: arm your board with bin/fm-procevent-lavish.sh arm --for ; never run lavish-axi poll yourself. Re-arm with the reply after each nonterminal round to acknowledge it, route the board feedback through your steering inbox, write needs-decision [key=board-review] with the live board URL when the captain owes a decision, and stop at session_ended or an empty End without re-arming - acknowledge that final round with bin/fm-procevent.sh handled to conclude and retire your board.' @@ -580,7 +607,7 @@ $TASK_SECTION $HERDR_SECTION # Setup -You are in a disposable git worktree of $REPO, at a detached HEAD on a clean default branch. +$SETUP_BASE This is a SCOUT task: the deliverable is a written report, not a PR. The worktree is your laboratory - install, run, edit, and make scratch commits freely; all of it is discarded at teardown. The report is the only thing that survives, so anything worth keeping must be in it. @@ -640,8 +667,8 @@ case "$MODE" in 2. Run \`no-mistakes doctor\`; if it reports the repo is not initialized here, run \`no-mistakes init\`." ;; esac -RULE1=$(fm_ship_rule_one "$MODE" "$ID" "$BRANCH" "$FORGE") || exit 1 -DOD=$(fm_dod_block "$MODE" "$ID" "$BRANCH" "$FORGE") || exit 1 +RULE1=$(fm_ship_rule_one "$MODE" "$ID" "$BRANCH" "$FORGE" "$BASE_BRANCH") || exit 1 +DOD=$(fm_dod_block "$MODE" "$ID" "$BRANCH" "$FORGE" "$BASE_BRANCH") || exit 1 cat > "$BRIEF" <` # and is the immutable task branch rendered in every delivery contract. +# The optional fifth argument is the task's base branch from bin/fm-brief.sh +# --base-branch; empty means the repository default. A named base is the branch +# the worker starts from, never pushes to, and targets with its pull request, and +# fm_base_branch_valid refuses it where no pull request carries the work. # Callers of the gate are bin/fm-crew-state.sh (current-state done), # bin/fm-pr-check.sh (PR registration), and bin/fm-inactive-reconcile.sh # (secondmate ledger-first publish of a child done). A ship `done:` is not @@ -140,23 +144,62 @@ fm_forge_valid_for_mode() { # return 0 } -fm_ship_rule_one() { # [branch] [] - local mode=$1 id=$2 forge=${4:-none} - local branch=${3:-fm/$id} +# A task's optional base branch replaces the repository default as the branch +# its copy starts from and its pull request targets. bin/fm-brief.sh records it +# as a "Base branch: " line under the brief's `# Setup` heading, +# bin/fm-spawn.sh takes it as --base-branch, refuses a brief whose Base branch +# lines (fm_brief_base_branches) disagree, and records base_branch= in the task +# metadata, and every later consumer reads that metadata field. It is +# refused on local-only, whose landing fast-forwards local main, and on a Gerrit +# forge, whose publish path targets the change's own branch. +fm_base_branch_valid() { # + local base=$1 mode=$2 forge=$3 caller=$4 + [ -n "$base" ] || return 0 + if [ "${base#-}" != "$base" ] || ! git check-ref-format --branch "$base" >/dev/null 2>&1; then + echo "error: $caller: base branch '$base' is not a valid git branch name" >&2 + return 1 + fi + if [ "$mode" = local-only ]; then + echo "error: $caller: a base branch cannot ship mode=local-only, whose landing fast-forwards local main; ship no-mistakes or direct-PR, which open a pull request against the base" >&2 + return 1 + fi + if [ "$forge" != none ]; then + echo "error: $caller: a base branch is not supported on forge=$forge" >&2 + return 1 + fi + return 0 +} + +# Print the value of every "Base branch: " line that directly follows the +# base-variant Setup sentence bin/fm-brief.sh writes; return 1 when there is +# none. Any other "Base branch:" line is prose and ignored. +fm_brief_base_branches() { # + awk ' + setup && sub(/^Base branch: /, "") { print; n++ } + { setup = /^You are in a disposable git worktree of .*, at a detached HEAD on a clean copy of its base branch\.$/ } + END { exit !n } + ' "$1" +} + +fm_ship_rule_one() { # [branch] [] [] + local mode=$1 id=$2 forge=${4:-none} base=${5:-} + local branch=${3:-fm/$id} target='the default branch' fm_forge_valid_for_mode "$forge" "$mode" fm_ship_rule_one || return 1 + fm_base_branch_valid "$base" "$mode" "$forge" fm_ship_rule_one || return 1 + [ -z "$base" ] || target="the base branch \`$base\` or the default branch" if [ "$forge" = gerrit ]; then printf '%s\n' "1. Never push with git and never create a change except through the one \`gerrit-axi publish --squash\` your Definition of done names. Never run \`gerrit-axi submit\`, never vote or review a change by any path, including \`gerrit review\` or a label option on a push, and never abandon one: a human reviewer approves and submits it on the server." return 0 fi case "$mode" in direct-PR) - printf '%s\n' "1. Never push to the default branch (push only your \`$branch\` branch). Never merge a PR." + printf '%s\n' "1. Never push to $target (push only your \`$branch\` branch). Never merge a PR." ;; local-only) printf '%s\n' "1. Never push to any remote and never open a PR. Work only on your \`$branch\` branch; firstmate handles the merge into local \`main\`." ;; no-mistakes) - printf '%s\n' '1. Never push to the default branch. Never merge a PR.' + printf '%s\n' "1. Never push to $target. Never merge a PR." ;; *) echo "error: fm_ship_rule_one: unknown delivery mode '$mode'" >&2 @@ -350,10 +393,17 @@ There is no pull request, no \`gh-axi\` call, and no forge CI result to report: EOF } -fm_dod_block() { # [branch] [] - local mode=$1 id=$2 forge=${4:-none} - local branch=${3:-fm/$id} +fm_dod_block() { # [branch] [] [] + local mode=$1 id=$2 forge=${4:-none} base=${5:-} + local branch=${3:-fm/$id} pr_base='' nm_base='' base_q fm_forge_valid_for_mode "$forge" "$mode" fm_dod_block || return 1 + fm_base_branch_valid "$base" "$mode" "$forge" fm_dod_block || return 1 + if [ -n "$base" ]; then + printf -v base_q '%q' "$base" + pr_base=", against the base branch \`$base\` (\`--base $base_q\`), not the repository default" + nm_base="This task's base branch is \`$base\`, not the repository default: pass \`--base-branch $base_q\` on every \`no-mistakes axi run\` that starts a run, so the pipeline rebases onto, opens its PR against, and watches CI for that branch. +" + fi case "$mode:$forge" in direct-PR:gerrit) cat <\` must print \`draft: no\`, where is the PR number from your PR URL); if it is a draft, mark it ready with \`gh-axi pr ready \`. A draft cannot be merged, so a done report on one leaves the merge unasked. Then append \`done [at=]: PR {url}\` to the status file and stop. @@ -440,7 +490,7 @@ The task is complete only when committed on your branch. When you believe it is complete, append \`done [at=]: {summary}\` to the status file and stop. Firstmate will then instruct you to run /no-mistakes to validate and ship a PR. That first \`done:\` is the handoff that starts the pipeline, which owns the push; it is not a request to push from this copy. - +${nm_base} EOF fm_nm_driving_block "$forge" cat <&2; exit 1; } diff --git a/bin/fm-review-diff.sh b/bin/fm-review-diff.sh index cb1877b49dc..d566a089c00 100755 --- a/bin/fm-review-diff.sh +++ b/bin/fm-review-diff.sh @@ -4,6 +4,8 @@ # Pooled project clones do not keep their local default branch current, so this # helper compares remote-backed projects against origin/ after fetching # the default branch, and local-only projects against the local default branch. +# A task whose meta records base_branch= (bin/fm-spawn.sh) compares against +# origin/ instead of the default branch. # When state/.meta records pr= as a GitHub pull-request URL or a bare # number for an open PR, the compare side is ALWAYS a freshly fetched # refs/pull//head by default so review stays current after no-mistakes fix @@ -74,7 +76,8 @@ default_branch() { return 1 } -DEFAULT=$(default_branch) || { echo "error: cannot determine default branch for $PROJ; expected origin/HEAD, main, or master" >&2; exit 1; } +DEFAULT=$(grep '^base_branch=' "$META" | cut -d= -f2- || true) +[ -n "$DEFAULT" ] || DEFAULT=$(default_branch) || { echo "error: cannot determine default branch for $PROJ; expected origin/HEAD, main, or master" >&2; exit 1; } BRANCH=$(grep '^branch=' "$META" | cut -d= -f2- || true) [ -n "$BRANCH" ] || BRANCH="fm/$ID" diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index fcee5a152b3..164ced52fbf 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash # Spawn a direct report: a crewmate in a treehouse or Orca worktree, or a # secondmate in its isolated firstmate home. -# Usage: fm-spawn.sh --mode --yolo [--branch-prefix ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] -# fm-spawn.sh --scout [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] +# Usage: fm-spawn.sh --mode --yolo [--branch-prefix ] [--base-branch ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] +# fm-spawn.sh --scout [--base-branch ] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] # fm-spawn.sh [] [--harness |harness|launch-command] [--model ] [--effort ] [--backend ] --secondmate # --mode and --yolo are this task's delivery contract, REQUIRED for every ship # spawn and refused on --scout and --secondmate spawns. Firstmate resolves both @@ -38,6 +38,16 @@ # prints a one-line deviation notice and continues, because the registered # prefix is the captain's standing preference and the brief agreement above # already guarantees the worker's instructions match the branch. +# --base-branch is the optional branch selected at intake for a ship or scout +# to start from and target instead of origin's default branch. A fresh launch +# resets its pooled copy to origin/, refusing when the project has no +# origin or origin lacks that branch, or when the project's registered forge +# cannot carry it. It must agree with every Setup "Base branch:" line in the +# brief (bin/fm-brief.sh --base-branch writes one; other such lines are prose), +# and a brief with such a line refuses a spawn without the flag. The spawn records it as +# base_branch= in state/.meta, which a relaunch reuses and later review and +# cleanup read; it is refused on secondmates and relaunches, and without it +# nothing changes. # Ship/scout launches always put fm-dod-lib.sh's current worker role scope # first in the private launch-brief overlay, including the exact task-owned # steering inbox. This never rewrites a project's instruction files or a @@ -250,8 +260,8 @@ # not marked. # Only after this isolation check, every fresh ship or scout requires a clean # task worktree. When an origin configuration is detected, spawn fetches it, -# resolves the current remote default branch, and resets to its tip. When none -# is detected, spawn skips that remote freshness check and launches from the +# resolves the current remote default branch (or uses --base-branch, described +# above), and resets to its tip. When none is detected, spawn skips that remote freshness check and launches from the # clean worktree's current HEAD. Relaunch reuses the recorded worktree without # fetching or resetting its base. An unreachable detected origin, unresolved # default branch, or non-clean worktree refuses a fresh spawn rather than @@ -662,6 +672,8 @@ BACKEND_SET=0 MODE_SET=0 YOLO_SET=0 BRANCH_PREFIX_SET=0 +BASE_BRANCH= +BASE_BRANCH_SET=0 TRACEPARENT_SET=0 RELAUNCH=0 POS=() @@ -703,6 +715,10 @@ for a in "$@"; do BRANCH_PREFIX=$a BRANCH_PREFIX_SET=1 ;; + base-branch) + BASE_BRANCH=$a + BASE_BRANCH_SET=1 + ;; traceparent) TRACEPARENT_ARG=$a TRACEPARENT_SET=1 @@ -760,6 +776,11 @@ for a in "$@"; do BRANCH_PREFIX=${a#--branch-prefix=} BRANCH_PREFIX_SET=1 ;; + --base-branch) want_value="base-branch" ;; + --base-branch=*) + BASE_BRANCH=${a#--base-branch=} + BASE_BRANCH_SET=1 + ;; --traceparent) want_value=traceparent ;; --traceparent=*) TRACEPARENT_ARG=${a#--traceparent=} @@ -846,6 +867,10 @@ if [ "$RELAUNCH" -eq 1 ]; then echo "error: --relaunch reuses the task's recorded ship branch; --branch-prefix cannot override it" >&2 exit 1 } + [ "$BASE_BRANCH_SET" -eq 0 ] || { + echo "error: --relaunch reuses the task's recorded base branch; --base-branch cannot override it" >&2 + exit 1 + } else # Delivery contract (AGENTS.md section 7). A ship task's mode and yolo are # firstmate's per-task decision, so they are required and closed-set validated @@ -891,6 +916,10 @@ else echo "error: --branch-prefix applies only to ship spawns; a scout makes no branch and a secondmate records no ship branch" >&2 exit 1 } + [ "$KIND" != secondmate ] || [ "$BASE_BRANCH_SET" -eq 0 ] || { + echo "error: --base-branch applies only to ship and scout spawns; a secondmate charter has no task base" >&2 + exit 1 + } fi fi @@ -1472,6 +1501,7 @@ if [ "${#POS[@]}" -gt 0 ] && [ "${POS[0]}" != "$idpart" ] && case "$idpart" in * [ "$MODE_SET" -eq 0 ] || shared_args+=(--mode "$MODE") [ "$YOLO_SET" -eq 0 ] || shared_args+=(--yolo "$YOLO") [ "$BRANCH_PREFIX_SET" -eq 0 ] || shared_args+=(--branch-prefix "$BRANCH_PREFIX") + [ "$BASE_BRANCH_SET" -eq 0 ] || shared_args+=(--base-branch "$BASE_BRANCH") for pair in "${POS[@]}"; do case "$pair" in *=*) : ;; @@ -3057,6 +3087,23 @@ if [ "$KIND" = ship ] || [ "$KIND" = scout ]; then fi fi fi + if [ "$RELAUNCH" -eq 1 ]; then + BASE_BRANCH=$(fm_meta_get "$RELAUNCH_META" base_branch) + elif [ "$BASE_BRANCH_SET" -eq 1 ]; then + [ -n "$BASE_BRANCH" ] || { + echo "error: --base-branch requires a branch name" >&2 + exit 1 + } + BASE_FORGE=$("$FM_ROOT/bin/fm-project-mode.sh" --forge "$(basename "$PROJ_ABS")") || exit 1 + fm_base_branch_valid "$BASE_BRANCH" "$MODE" "${BASE_FORGE:-none}" "fm-spawn.sh --base-branch" || exit 1 + if ! fm_brief_base_branches "$BRIEF" >/dev/null || fm_brief_base_branches "$BRIEF" | grep -vxF -- "$BASE_BRANCH" >/dev/null; then + echo "error: $BRIEF must record Base branch: $BASE_BRANCH and no other Base branch line to spawn with --base-branch $BASE_BRANCH; scaffold it with bin/fm-brief.sh --base-branch $BASE_BRANCH" >&2 + exit 1 + fi + elif fm_brief_base_branches "$BRIEF" >/dev/null; then + echo "error: $BRIEF records a Base branch line but the spawn has no --base-branch; pass the brief's base with --base-branch or re-scaffold the brief without one" >&2 + exit 1 + fi # Use the existing launch-brief overlay for every worker kind, including # pre-scope briefs and relaunches. Charters never enter this worker path. SOURCE_BRIEF=$BRIEF @@ -3341,8 +3388,8 @@ spawn_worktree_has_origin_config() { # return 1 } -freshen_spawn_worktree_base() { # - local worktree=$1 default target expected actual status +freshen_spawn_worktree_base() { # [] + local worktree=$1 base=${2:-} default target expected actual status status=$(git -C "$worktree" -c core.quotePath=false status --porcelain) || { echo "error: could not inspect pooled worktree '$worktree' before refreshing its base" >&2 return 1 @@ -3356,20 +3403,28 @@ freshen_spawn_worktree_base() { # return 1 fi if ! spawn_worktree_has_origin_config "$worktree"; then + [ -z "$base" ] || { + echo "error: pooled worktree '$worktree' has no origin, so it cannot start from base branch '$base'" >&2 + return 1 + } return 0 fi if ! git -C "$worktree" fetch --quiet origin; then echo "error: could not fetch origin for pooled worktree '$worktree'; refusing to launch from a potentially stale base" >&2 return 1 fi - if ! git -C "$worktree" remote set-head origin --auto >/dev/null 2>&1; then - echo "error: could not resolve origin's current default branch for pooled worktree '$worktree'; refusing to launch from a potentially stale base" >&2 - return 1 + if [ -n "$base" ]; then + default=$base + else + if ! git -C "$worktree" remote set-head origin --auto >/dev/null 2>&1; then + echo "error: could not resolve origin's current default branch for pooled worktree '$worktree'; refusing to launch from a potentially stale base" >&2 + return 1 + fi + default=$(default_branch "$worktree") || { + echo "error: could not determine origin's default branch for pooled worktree '$worktree'; refusing to launch from a potentially stale base" >&2 + return 1 + } fi - default=$(default_branch "$worktree") || { - echo "error: could not determine origin's default branch for pooled worktree '$worktree'; refusing to launch from a potentially stale base" >&2 - return 1 - } target="origin/$default" if ! git -C "$worktree" fetch --quiet origin "+refs/heads/$default:refs/remotes/origin/$default"; then echo "error: could not fetch '$target' for pooled worktree '$worktree'; refusing to launch from a potentially stale base" >&2 @@ -4331,7 +4386,7 @@ elif [ "$KIND" != secondmate ] && [ "$BACKEND" != orca ]; then fi fi if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ]; then - freshen_spawn_worktree_base "$WT" || exit 1 + freshen_spawn_worktree_base "$WT" "$BASE_BRANCH" || exit 1 fi # Re-assert the durable task copy after either treehouse acquisition or endpoint @@ -4890,7 +4945,7 @@ SPAWN_META_PATH=$SPAWN_META_TMP preserve_relaunch_meta() { awk -F= ' BEGIN { - split("window endpoint_task_id worktree project harness kind mode yolo branch tasktmp model effort account account_provider busy_gen spawn_gen traceparent backend herdr_session herdr_workspace_id herdr_tab_id herdr_pane_id zellij_session zellij_tab_id zellij_pane_id orca_worktree_id terminal cmux_workspace_id cmux_surface_id home projects control_relaunch_tx", keys, " ") + split("window endpoint_task_id worktree project harness kind mode yolo branch tasktmp base_branch model effort account account_provider busy_gen spawn_gen traceparent backend herdr_session herdr_workspace_id herdr_tab_id herdr_pane_id zellij_session zellij_tab_id zellij_pane_id orca_worktree_id terminal cmux_workspace_id cmux_surface_id home projects control_relaunch_tx", keys, " ") for (i in keys) owned[keys[i]] = 1 } !($1 in owned) @@ -4907,6 +4962,7 @@ preserve_relaunch_meta() { [ -z "$YOLO" ] || echo "yolo=$YOLO" [ -z "${BRANCH:-}" ] || echo "branch=$BRANCH" echo "tasktmp=$TASK_TMP" + [ -z "$BASE_BRANCH" ] || echo "base_branch=$BASE_BRANCH" echo "model=${MODEL:-default}" echo "effort=${EFFORT:-default}" # The worker account pin, only when this home declares one, so an unpinned diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index c85b66f8502..1a0f6c14e78 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -49,7 +49,9 @@ # GitHub reports a PR head that contains the current local work, or its content is # already present in the up-to-date default branch. This recognizes the common # squash-merge-then-delete-branch flow, where the branch's own commits live nowhere -# on a remote yet the change is fully in main. +# on a remote yet the change is fully in main. A task whose meta records +# base_branch= (bin/fm-spawn.sh) runs that content check against origin's copy of +# its base branch instead of the default branch. # Squash merges collapse the branch's commits, so per-commit patch ids against main # no longer match, and a pipeline rebase can leave the local worktree diverged from # the PR head. A diverged copy is not treated as landed: path-set coverage, git @@ -1164,6 +1166,7 @@ elif [ "$TREEHOUSE_SLOT_LOCK_REQUIRED" = 1 ]; then fi MODE=$(grep '^mode=' "$META" | cut -d= -f2- || true) [ -n "$MODE" ] || MODE=no-mistakes +BASE_BRANCH=$(grep '^base_branch=' "$META" | cut -d= -f2- || true) # A record accepted as a legacy incarnation (no spawn_gen, and either # --legacy-record given or the record is windowless) may be torn down only @@ -1580,8 +1583,8 @@ pr_is_merged() { # "added". Returns non-zero when inconclusive (no default ref, or a merge conflict), # so the caller refuses rather than guesses. content_in_default() { - local name ref default_tree merged_tree - name=$(default_branch) || return 1 + local name=${BASE_BRANCH:-} ref default_tree merged_tree + [ -n "$name" ] || name=$(default_branch) || return 1 if git -C "$WT" remote get-url origin >/dev/null 2>&1; then git -C "$WT" fetch --quiet origin "+refs/heads/$name:refs/remotes/origin/$name" >/dev/null 2>&1 || return 1 ref="refs/remotes/origin/$name" diff --git a/docs/architecture.md b/docs/architecture.md index a8754ae3bee..1f6b9cf31d9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -377,6 +377,7 @@ Firstmate passes the binding unchanged to `bin/fm-brief.sh --forge` and never in `bin/fm-forge-detect.sh` only proposes a binding at project-add intake; nothing re-derives one from a clone at use time. `bin/fm-project-mode.sh` remains the one registry parser for the mechanical consumers that have no task in hand: fleet sync's `local-only` skip and home seeding's refusal and no-mistakes initialization. The registry's optional `branch=` annotation overrides a project's ship-branch prefix (default `fm/`) the same way: firstmate resolves it via `bin/fm-project-mode.sh --branch-prefix` at intake and passes it explicitly to `bin/fm-brief.sh --branch-prefix`, which never reads the registry itself; each script's own header owns its side of that contract. +A task's base branch is a per-task choice with no registry entry: `bin/fm-brief.sh --base-branch` records it in the brief, a fresh spawn passed the same `--base-branch` resets the task's copy to `origin/` and records `base_branch=` in task meta, and the worker's pull request targets that branch; review, cleanup, and promotion read the recorded value instead of the default branch. When a selected delivery path calls for a diff, `bin/fm-review-diff.sh` refreshes the authoritative base and, when task meta records a GitHub pull-request `pr=`, always fetches and compares against `refs/pull//head` by default (recorded `pr_head=` is only an offline fallback) before falling back to the local branch with a warning. A GitLab merge request and a Gerrit change expose no such ref, so a task recording one of those diffs the local branch under that same warning, which is its current content. Where a no-mistakes pipeline stores evidence in the repo, it publishes that PR-viewable validation evidence to an orphan evidence branch that shares no history with code branches, so it never enters the crew branch or the default branch. diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index 225d1c0975c..61d1cfba0a1 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -1176,6 +1176,76 @@ test_home_brief_include_is_appended_last() { pass "fm-brief.sh: the home brief include lands last on ship and scout, verbatim, and fails closed" } +# --base-branch names the branch a task starts from and a ship's PR targets. It is +# recorded as a Base branch line under # Setup, which fm-spawn reads back, and is +# refused where no pull request carries the work. +test_base_branch_is_rendered_and_bounded() { + local home out rc brief base meta_base + home="$TMP_ROOT/base-branch-home" + mkdir -p "$home/data" + # shellcheck source=bin/fm-dod-lib.sh + . "$ROOT/bin/fm-dod-lib.sh" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-dp-b1 some-proj --mode direct-PR --base-branch feature/hub >/dev/null 2>&1 \ + || fail "direct-PR --base-branch should scaffold" + brief="$home/data/brief-base-dp-b1/brief.md" + base=$(fm_brief_base_branches "$brief") + [ "$base" = feature/hub ] || fail "the direct-PR brief recorded base '$base', not feature/hub" + # shellcheck disable=SC2016 # literal backticks in rendered prose must stay unexpanded + assert_grep 'open a PR with `gh-axi` that is ready for review, not a draft, against the base branch `feature/hub` (`--base feature/hub`)' "$brief" \ + "the direct-PR definition of done does not target the base branch" + # shellcheck disable=SC2016 + assert_grep 'Never push to the base branch `feature/hub` or the default branch' "$brief" \ + "the direct-PR safety rule does not protect the base branch" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-nm-b2 some-proj --mode no-mistakes --base-branch release/1.2 >/dev/null 2>&1 \ + || fail "no-mistakes --base-branch should scaffold" + brief="$home/data/brief-base-nm-b2/brief.md" + # shellcheck disable=SC2016 + assert_grep 'pass `--base-branch release/1.2` on every `no-mistakes axi run`' "$brief" \ + "the no-mistakes definition of done does not pass the base branch to the pipeline" + + # A base git accepts but the shell would expand is quoted in worker commands. + # shellcheck disable=SC2016 # the literal $HOTFIX is the point + meta_base='release/$HOTFIX' + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-meta-b8 some-proj --mode no-mistakes --base-branch "$meta_base" >/dev/null 2>&1 \ + || fail "no-mistakes --base-branch with a shell metacharacter should scaffold" + brief="$home/data/brief-base-meta-b8/brief.md" + base=$(fm_brief_base_branches "$brief") + [ "$base" = "$meta_base" ] || fail "the brief recorded base '$base', not $meta_base" + # shellcheck disable=SC2016 + assert_grep 'pass `--base-branch release/\$HOTFIX` on every' "$brief" \ + "the no-mistakes command did not shell-quote the base branch" + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-meta-b9 some-proj --mode direct-PR --base-branch "$meta_base" >/dev/null 2>&1 \ + || fail "direct-PR --base-branch with a shell metacharacter should scaffold" + # shellcheck disable=SC2016 + assert_grep '(`--base release/\$HOTFIX`)' "$home/data/brief-base-meta-b9/brief.md" \ + "the direct-PR command did not shell-quote the base branch" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-scout-b3 some-proj --scout --base-branch feature/hub >/dev/null 2>&1 \ + || fail "scout --base-branch should scaffold" + base=$(fm_brief_base_branches "$home/data/brief-base-scout-b3/brief.md") + [ "$base" = feature/hub ] || fail "the scout brief recorded base '$base', not feature/hub" + + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-none-b4 some-proj --mode direct-PR >/dev/null 2>&1 + brief="$home/data/brief-base-none-b4/brief.md" + ! fm_brief_base_branches "$brief" >/dev/null || fail "a brief without --base-branch recorded a base" + assert_grep 'at a detached HEAD on a clean default branch.' "$brief" \ + "a brief without --base-branch changed its default-branch setup line" + + out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-lo-b5 some-proj --mode local-only --base-branch feature/hub 2>&1); rc=$? + [ "$rc" -ne 0 ] || fail "local-only --base-branch should be refused" + assert_contains "$out" "mode=local-only" "the local-only refusal did not explain itself" + out=$(FM_HOME="$home" "$ROOT/bin/fm-brief.sh" brief-base-bad-b6 some-proj --mode direct-PR --base-branch 'bad..name' 2>&1); rc=$? + [ "$rc" -ne 0 ] || fail "an invalid base branch name should be refused" + out=$(FM_HOME="$home" FM_SECONDMATE_CHARTER=charter "$ROOT/bin/fm-brief.sh" brief-base-sm-b7 --secondmate --no-projects --base-branch feature/hub 2>&1); rc=$? + [ "$rc" -ne 0 ] || fail "a secondmate charter should refuse --base-branch" + for id in brief-base-lo-b5 brief-base-bad-b6 brief-base-sm-b7; do + [ ! -e "$home/data/$id/brief.md" ] || fail "a refused --base-branch scaffold wrote $id" + done + pass "fm-brief.sh: --base-branch records the base, targets the PR at it, and is refused where no PR carries it" +} + # (a) An unregistered/default project - no --branch-prefix passed at all - must # keep every generated ship mode's branch on the legacy "fm/" name, byte # for byte, so every existing firstmate installation is unaffected. @@ -1424,6 +1494,7 @@ test_scout_lavish_line_follows_presentation_floor test_workers_wait_without_spending_turns test_wait_no_turns_absent_keeps_the_previous_brief test_home_brief_include_is_appended_last +test_base_branch_is_rendered_and_bounded test_ship_branch_prefix_defaults_to_legacy_fm test_ship_branch_prefix_override_is_consistent_across_modes test_ship_branch_prefix_empty_override_yields_bare_task_id diff --git a/tests/fm-dod-lib.test.sh b/tests/fm-dod-lib.test.sh index dea79f0e3ed..05f16995e85 100644 --- a/tests/fm-dod-lib.test.sh +++ b/tests/fm-dod-lib.test.sh @@ -382,6 +382,48 @@ test_pr_based_dod_draft_check_uses_gh_axi() { pass "PR-based DoD draft check uses gh-axi" } +# A scout spawned on a named base keeps that base through promotion: the ship +# instructions start from it and the PR targets it; local-only cannot carry it. +test_promotion_keeps_the_recorded_base_branch() { + local home id meta out status mode + home="$TMP_ROOT/promote-base-home" + for mode in direct-PR local-only; do + id="promote-base-$mode" + meta="$home/state/$id.meta" + mkdir -p "$home/state" "$home/data/$id" + printf 'window=fm-%s\nkind=scout\nworktree=/tmp/wt\nbase_branch=feature/hub\n' "$id" > "$meta" + cat > "$home/data/$id/brief.md" <<'EOF' +# Task +## Captain's intent +Fix the hub bug. + +## Firstmate spec +Reproduce it first. + +# Setup +You are in a disposable git worktree of proj, at a detached HEAD on a clean copy of its base branch. +Base branch: feature/hub +EOF + out=$(FM_HOME="$home" FM_STATE_OVERRIDE="$home/state" "$ROOT/bin/fm-promote.sh" "$id" --mode "$mode" --yolo off 2>&1) + status=$? + if [ "$mode" = direct-PR ]; then + expect_code 0 "$status" "promoting a scout with a recorded base should succeed"$'\n'"$out" + # shellcheck disable=SC2016 # literal backticks in rendered prose must stay unexpanded + assert_grep 'Return to a clean copy of the base branch `feature/hub`' "$home/data/$id/ship-instructions.md" \ + "promotion did not start the ship from the recorded base" + # shellcheck disable=SC2016 + assert_grep 'against the base branch `feature/hub`' "$home/data/$id/ship-instructions.md" \ + "promotion did not target the PR at the recorded base" + assert_grep 'base_branch=feature/hub' "$meta" "promotion dropped the recorded base" + else + [ "$status" -ne 0 ] || fail "promoting a based scout to local-only should be refused" + assert_contains "$out" "mode=local-only" "the local-only promotion refusal did not explain itself" + assert_grep 'kind=scout' "$meta" "a refused promotion changed the task record" + fi + done + pass "promotion keeps a scout's recorded base branch and refuses local-only for it" +} + test_scout_done_is_not_gated test_unpushed_ship_done_is_refused test_no_mistakes_prevalidation_done_is_not_gated @@ -400,5 +442,6 @@ test_standalone_local_only_needs_project_ref test_non_done_lines_are_not_gated test_fenced_and_indented_captain_lines_are_not_intent test_pr_based_dod_draft_check_uses_gh_axi +test_promotion_keeps_the_recorded_base_branch echo "all fm-dod-lib tests passed" diff --git a/tests/fm-review-diff.test.sh b/tests/fm-review-diff.test.sh index 832c4c9a93b..fb47bb590e6 100755 --- a/tests/fm-review-diff.test.sh +++ b/tests/fm-review-diff.test.sh @@ -15,6 +15,8 @@ # reviewed even when the worktree HEAD has moved off it # (g) meta records a corrupt branch= -> refused, never silently reviewed as # the moved worktree HEAD +# (h) meta records base_branch= -> the diff is against origin/, +# so the base branch's own commits never appear as task changes set -u # shellcheck source=tests/lib.sh @@ -216,6 +218,26 @@ test_corrupt_recorded_branch_is_refused() { pass "fm-review-diff refuses a corrupt recorded ship branch instead of reviewing the wrong content" } +test_recorded_base_branch_is_the_review_base() { + local case_dir out + case_dir=$(make_case base-branch) + git -C "$case_dir/project" checkout -q -b feature/hub main + printf 'hub only\n' > "$case_dir/project/hub.txt" + git -C "$case_dir/project" add hub.txt + git -C "$case_dir/project" commit -qm "hub branch commit" + git -C "$case_dir/project" push -q origin feature/hub + git -C "$case_dir/wt" reset -q --hard origin/feature/hub 2>/dev/null \ + || { git -C "$case_dir/wt" fetch -q origin feature/hub && git -C "$case_dir/wt" reset -q --hard FETCH_HEAD; } + printf 'task change\n' > "$case_dir/wt/feature.txt" + git -C "$case_dir/wt" commit -qam "task change on hub" + write_task_meta "$case_dir" "base_branch=feature/hub" + + out=$(run_review_diff "$case_dir" task-x1) + assert_contains "$out" '+task change' "base-branch: the task's own change is missing from the review diff" + assert_not_contains "$out" 'hub.txt' "base-branch: the base branch's own commit was reviewed as a task change" + pass "fm-review-diff compares a task against its recorded base branch" +} + test_pr_meta_uses_pr_head_not_stale_local test_pr_meta_fetches_pull_head_without_recorded_sha test_stale_recorded_pr_head_loses_to_fetched_pull_head @@ -223,3 +245,4 @@ test_no_pr_meta_uses_local_branch test_unreachable_pr_head_falls_back_with_warning test_recorded_branch_beats_moved_worktree_head test_corrupt_recorded_branch_is_refused +test_recorded_base_branch_is_the_review_base diff --git a/tests/fm-spawn-pool-base-freshen.test.sh b/tests/fm-spawn-pool-base-freshen.test.sh index 2d39f3607ca..f3f5c9eb74a 100755 --- a/tests/fm-spawn-pool-base-freshen.test.sh +++ b/tests/fm-spawn-pool-base-freshen.test.sh @@ -743,10 +743,145 @@ test_pool_slot_claim_follows_the_spawn_outcome() { pass "a Treehouse slot claim names the launched task, refuses when unclaimable, and is dropped by a locked abort" } +publish_feature_branch() { # + git -C "$CASE_DIR/publisher" checkout --quiet -b "$1" + printf 'only on %s\n' "$1" > "$CASE_DIR/publisher/feature-only.txt" + git -C "$CASE_DIR/publisher" add feature-only.txt + git -C "$CASE_DIR/publisher" -c user.name='Firstmate Tests' -c user.email='tests@example.invalid' commit -qm feature + git -C "$CASE_DIR/publisher" push --quiet origin "$1" +} + +brief_with_base() { # [] + fm_test_spawn_brief "$HOME_DIR" "$1" ${3:+"$3"} + printf '\n# Setup\nYou are in a disposable git worktree of project, at a detached HEAD on a clean copy of its base branch.\nBase branch: %s\n' "$2" >> "$HOME_DIR/data/$1/brief.md" +} + +test_named_base_branch_starts_from_that_branch() { + local rec id out status kind + for kind in ship scout; do + id="pool-named-base-$kind-r1" + rec=$(make_case "named-base-$kind" "$id") + read_case_record "$rec" + publish_feature_branch feature/hub + brief_with_base "$id" feature/hub + if [ "$kind" = ship ]; then + out=$(run_spawn "$id" --mode direct-PR --yolo off --base-branch feature/hub) + else + out=$(run_spawn "$id" --scout --base-branch feature/hub) + fi + status=$? + expect_code 0 "$status" "a $kind spawn with a matching base branch should launch"$'\n'"$out" + [ "$(git -C "$POOL_DIR" rev-parse HEAD)" = "$(git -C "$POOL_DIR" rev-parse origin/feature/hub)" ] \ + || fail "the $kind copy did not start from origin/feature/hub" + assert_grep 'only on feature/hub' "$POOL_DIR/feature-only.txt" \ + "the $kind copy is missing the base branch content" + assert_grep 'base_branch=feature/hub' "$HOME_DIR/state/$id.meta" \ + "the $kind spawn did not record its base branch" + done + + id='pool-named-base-missing-r1' + rec=$(make_case named-base-missing "$id") + read_case_record "$rec" + brief_with_base "$id" feature/missing + out=$(run_spawn "$id" --scout --base-branch feature/missing) + status=$? + [ "$status" -ne 0 ] || fail "a base branch origin lacks should refuse the spawn" + assert_contains "$out" "origin/feature/missing" "the refusal did not name the missing base" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "a refused base-branch spawn published task metadata" + + id='pool-named-base-local-only-r1' + rec=$(make_case named-base-local-only "$id") + read_case_record "$rec" + brief_with_base "$id" feature/hub + out=$(run_spawn "$id" --mode local-only --yolo off --base-branch feature/hub) + status=$? + [ "$status" -ne 0 ] || fail "a base branch on a local-only ship should refuse the spawn" + assert_contains "$out" "mode=local-only" "the local-only refusal did not explain itself" + pass "--base-branch picks the copy's starting point and is recorded; a missing or local-only base refuses" +} + +test_base_branch_must_agree_with_the_brief() { + local rec id out status case_name + for case_name in flag-only line-only task-decoy; do + id="pool-base-agree-$case_name-r1" + rec=$(make_case "base-agree-$case_name" "$id") + read_case_record "$rec" + publish_feature_branch feature/hub + case "$case_name" in + flag-only) + fm_test_spawn_brief "$HOME_DIR" "$id" + out=$(run_spawn "$id" --scout --base-branch feature/hub) + ;; + line-only) + brief_with_base "$id" feature/hub + out=$(run_spawn "$id" --scout) + ;; + task-decoy) + brief_with_base "$id" feature/hub $'You are in a disposable git worktree of project, at a detached HEAD on a clean copy of its base branch.\nBase branch: decoy' + out=$(run_spawn "$id" --scout --base-branch feature/hub) + ;; + esac + status=$? + [ "$status" -ne 0 ] || fail "a $case_name base-branch disagreement should refuse the spawn" + assert_contains "$out" "Base branch" "the $case_name refusal did not name the Base branch line" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "a refused $case_name spawn published task metadata" + done + pass "a spawn refuses when --base-branch and the brief's Base branch lines disagree" +} + +# A Base branch line quoted in the captain's intent is prose: it neither blocks +# a spawn nor redirects it. +test_prose_base_branch_line_is_ignored() { + local rec id out status + id='pool-base-prose-none-r1' + rec=$(make_case base-prose-none "$id") + read_case_record "$rec" + fm_test_spawn_brief "$HOME_DIR" "$id" $'Fix the release.\nBase branch: release/1.2' + out=$(run_spawn "$id" --scout) + status=$? + expect_code 0 "$status" "a no-base brief quoting a Base branch line should launch"$'\n'"$out" + assert_no_grep 'base_branch=' "$HOME_DIR/state/$id.meta" "prose recorded a base branch" + + id='pool-base-prose-flag-r1' + rec=$(make_case base-prose-flag "$id") + read_case_record "$rec" + publish_feature_branch feature/hub + brief_with_base "$id" feature/hub $'Fix the release.\nBase branch: release/1.2' + out=$(run_spawn "$id" --scout --base-branch feature/hub) + status=$? + expect_code 0 "$status" "a flagged spawn whose intent names another branch should launch"$'\n'"$out" + [ "$(git -C "$POOL_DIR" rev-parse HEAD)" = "$(git -C "$POOL_DIR" rev-parse origin/feature/hub)" ] \ + || fail "the prose branch redirected the copy away from origin/feature/hub" + assert_grep 'base_branch=feature/hub' "$HOME_DIR/state/$id.meta" "the flag's base was not recorded" + pass "a Base branch line in the captain's intent neither blocks nor redirects a spawn" +} + +# A named base is validated against the project's registered forge for scouts +# too, so a scout that promotion could never ship is refused at spawn. +test_scout_base_branch_refused_on_gerrit_forge() { + local rec id out status + id='pool-base-gerrit-scout-r1' + rec=$(make_case base-gerrit-scout "$id") + read_case_record "$rec" + publish_feature_branch feature/hub + printf '%s\n' '- project [no-mistakes forge=gerrit] - fixture (added 2026-01-01)' > "$HOME_DIR/data/projects.md" + brief_with_base "$id" feature/hub + out=$(run_spawn "$id" --scout --base-branch feature/hub) + status=$? + [ "$status" -ne 0 ] || fail "a based scout on a forge=gerrit project should refuse the spawn" + assert_contains "$out" "forge=gerrit" "the gerrit refusal did not explain itself" + [ ! -e "$HOME_DIR/state/$id.meta" ] || fail "a refused gerrit scout spawn published task metadata" + pass "a based scout on a forge=gerrit project is refused at spawn" +} + test_remote_seeded_home_spawns_from_treehouse_pool test_pool_slot_claim_follows_the_spawn_outcome test_linked_spawning_home_rejects_primary_before_refresh test_stale_pool_base_refreshes_before_branching +test_named_base_branch_starts_from_that_branch +test_base_branch_must_agree_with_the_brief +test_prose_base_branch_line_is_ignored +test_scout_base_branch_refused_on_gerrit_forge test_non_main_default_branch_refreshes_before_branching test_direct_pr_and_scout_refresh_before_launch test_dirty_pool_refuses_without_discarding_work diff --git a/tests/fm-teardown.test.sh b/tests/fm-teardown.test.sh index 462c3447202..3497c74725b 100755 --- a/tests/fm-teardown.test.sh +++ b/tests/fm-teardown.test.sh @@ -1173,6 +1173,52 @@ SH pass "worktree whose content already landed in the default branch is torn down (content fallback)" } +# A task recording base_branch= landed when its content reached that branch, not +# the default branch: a squash merge into the base branch is the landing. +test_content_fallback_uses_recorded_base_branch() { + local case_dir rc landed tmp + for landed in base default; do + case_dir=$(make_case "content-base-$landed") + write_meta "$case_dir" direct-PR ship + printf 'base_branch=feature/hub\n' >> "$case_dir/state/task-x1.meta" + tmp="$case_dir/_hub" + git clone -q "$case_dir/origin.git" "$tmp" + git -C "$tmp" push -q origin HEAD:refs/heads/feature/hub + rm -rf "$tmp" + wt_commit_file "$case_dir" feature.txt hello "add feature" + if [ "$landed" = base ]; then + tmp="$case_dir/_land" + git clone -q "$case_dir/origin.git" "$tmp" + git -C "$tmp" checkout -q feature/hub + printf 'hello\n' > "$tmp/feature.txt" + git -C "$tmp" add feature.txt + git -C "$tmp" -c user.email=t@t -c user.name=t commit -q -m "squash feature.txt" + git -C "$tmp" push -q origin HEAD:feature/hub + rm -rf "$tmp" + else + land_on_origin_main "$case_dir" feature.txt hello + fi + cat > "$case_dir/fakebin/treehouse" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$case_dir/fakebin/treehouse" + + set +e + run_teardown "$case_dir" > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + if [ "$landed" = base ]; then + expect_code 0 "$rc" "content-base: content squashed into the recorded base branch should count as landed" + assert_absent "$case_dir/state/task-x1.meta" "content-base: teardown kept the record of landed work" + else + [ "$rc" -ne 0 ] || fail "content-base: content only on the default branch passed for a task based on feature/hub" + assert_present "$case_dir/state/task-x1.meta" "content-base: a refused teardown removed the task record" + fi + done + pass "the content-landed fallback checks a task's recorded base branch, not the default branch" +} + test_content_fallback_refreshes_stale_origin_ref() { local case_dir rc case_dir=$(make_case content-stale-ref) @@ -4497,6 +4543,7 @@ test_squash_merged_stale_local_refuses_when_forge_unreachable test_pr_check_does_not_refresh_stale_pr_head test_pr_check_records_remote_head_when_local_lags test_content_in_default_fallback_allows +test_content_fallback_uses_recorded_base_branch test_content_fallback_refreshes_stale_origin_ref test_dirty_worktree_refuses test_untracked_only_refusal_diagnostic From e5b9dddc982e546b81b4bfce754df1465270a59e Mon Sep 17 00:00:00 2001 From: Tiago Date: Mon, 5 Oct 2026 15:26:05 -0300 Subject: [PATCH 03/13] fix(bin): name the Firstmate skill file as a fallback in the worker role (#6647) * fix(bin): point project workers at the Firstmate skill file The Skill tool cannot resolve a Firstmate skill from another project's worktree, so the launch role names the readable skill file instead. * no-mistakes(review): fix(bin): name Firstmate skill file as fallback only --- bin/fm-dod-lib.sh | 11 +++++++++-- bin/fm-spawn.sh | 13 ++++++++----- tests/fm-dod-lib.test.sh | 21 +++++++++++++++++++++ tests/fm-spawn-dispatch-profile.test.sh | 2 ++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/bin/fm-dod-lib.sh b/bin/fm-dod-lib.sh index 7b9e5403547..d5caf5ed7d6 100755 --- a/bin/fm-dod-lib.sh +++ b/bin/fm-dod-lib.sh @@ -95,6 +95,12 @@ # fm_brief_intent_overlay it is a distinctly titled launch section that states # its own precedence, so a brief or project instruction that authors a # conflicting role is superseded rather than duplicated. +# The code root argument is the Firstmate checkout that holds +# .agents/skills/firstmate-coding-guidelines/SKILL.md. A worker in a Firstmate +# worktree loads that skill by name from its own checkout. A worker whose +# session does not register the skill, such as one in another project's +# worktree, cannot, so the role also names the file as the fallback to read; +# the Claude launch grants the skills directory that holds it. # fm_ship_rule_one owns the mode-specific first ship safety rule shared by an # ordinary ship brief and the durable contract written during scout promotion. # It takes the same optional trailing forge argument, because the rule that keeps @@ -109,8 +115,8 @@ # shellcheck source=bin/fm-brief-heading-lib.sh . "$(d=${BASH_SOURCE[0]%/*}; [ "$d" != "${BASH_SOURCE[0]}" ] || d=.; cd "${d:-/}" && pwd)/fm-brief-heading-lib.sh" -fm_brief_worker_role() { # - local state=$1 task_id=$2 +fm_brief_worker_role() { # + local state=$1 task_id=$2 root=$3 cat <<'EOF' # Current worker role contract You are a crewmate: an autonomous worker agent managed by firstmate. @@ -123,6 +129,7 @@ Never inspect or change any other home's endpoint namespace; this authorization When this task works on Firstmate itself, the repository root `AGENTS.md` (also imported by `CLAUDE.md`) is project content and the supervisor contract for the firstmate managing you: follow this brief instead of that supervisor contract. Project instructions still govern the work wherever they do not conflict with this worker identity, including `CONTRIBUTING.md` and `firstmate-coding-guidelines` for Firstmate changes. EOF + printf "If the \`firstmate-coding-guidelines\` skill name does not resolve in this session, read \`%s/.agents/skills/firstmate-coding-guidelines/SKILL.md\` instead.\n" "$root" } # Closed-set gate shared by every forge-aware renderer and bin/fm-brief.sh, so a diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 164ced52fbf..0e58e9c29c0 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -2817,10 +2817,13 @@ rovo_config_override_flag() { # Firstmate worker always reads outside its cwd - a secondmate's steers live # in the PARENT home's state/.inbox, and a ship or scout worker's launch # record, steers, and brief live in this home's state/operational-inbox, -# state/.inbox, and data/, with the code root's .agents/skills named -# by its definition of done - so every Claude launch, fresh spawn and -# relaunch, in both permission modes, grants exactly those task-channel -# directories. Paths resolve the way rovo_config_override_flag resolves them +# state/.inbox, and data/, plus the code root's .agents/skills so the +# worker can read the skill file the launch role names as the fallback for a +# session where the skill name does not resolve - so every Claude launch, +# fresh spawn and relaunch, in both permission modes, grants exactly those +# task-channel directories. The skills grant is that directory, not the +# checkout root, so the grant does not open the whole checkout. Paths resolve +# the way rovo_config_override_flag resolves them # (real paths under the task's home). The state channel dirs are created # lazily by their first record, so they are made here: an --add-dir naming a # directory that does not exist at launch would leave the channel created @@ -3110,7 +3113,7 @@ if [ "$KIND" = ship ] || [ "$KIND" = scout ]; then BRIEF="$DATA/$ID/launch-brief.md" BRIEF_TMP="$DATA/$ID/.launch-brief.md.${BASHPID:-$$}" { - fm_brief_worker_role "$STATE" "$ID" && + fm_brief_worker_role "$STATE" "$ID" "$FM_ROOT" && printf '\n' && cat "$SOURCE_BRIEF" && if [ "$KIND" = ship ] && [ "$MODE" = no-mistakes ]; then diff --git a/tests/fm-dod-lib.test.sh b/tests/fm-dod-lib.test.sh index 05f16995e85..ee471c2c867 100644 --- a/tests/fm-dod-lib.test.sh +++ b/tests/fm-dod-lib.test.sh @@ -444,4 +444,25 @@ test_fenced_and_indented_captain_lines_are_not_intent test_pr_based_dod_draft_check_uses_gh_axi test_promotion_keeps_the_recorded_base_branch +# The launch role is the generated text a worker receives. It must keep the +# skill name, so a session that registers the skill loads it by name, and must +# name the skill file as the fallback for a session where the name does not +# resolve. +test_worker_role_names_skill_and_fallback_file() { + local role_file path + role_file="$TMP_ROOT/worker-role.txt" + path="$ROOT/.agents/skills/firstmate-coding-guidelines/SKILL.md" + [ -f "$path" ] || fail "Firstmate skill file is missing at $path" + fm_brief_worker_role "$TMP_ROOT/state" upstream-4751 "$ROOT" >"$role_file" + assert_grep "\`CONTRIBUTING.md\` and \`firstmate-coding-guidelines\` for Firstmate changes" "$role_file" \ + "worker role did not name the skill" + assert_grep "If the \`firstmate-coding-guidelines\` skill name does not resolve in this session, read \`$path\` instead." "$role_file" \ + "worker role did not name the skill file as the fallback" + assert_no_grep "Skill tool cannot resolve" "$role_file" \ + "worker role claims the Skill tool never resolves the skill" + pass "worker role names the skill and its fallback skill file" +} + +test_worker_role_names_skill_and_fallback_file + echo "all fm-dod-lib tests passed" diff --git a/tests/fm-spawn-dispatch-profile.test.sh b/tests/fm-spawn-dispatch-profile.test.sh index 4ab76dfe640..b69665863f3 100755 --- a/tests/fm-spawn-dispatch-profile.test.sh +++ b/tests/fm-spawn-dispatch-profile.test.sh @@ -1691,6 +1691,8 @@ SH assert_grep "$inbox" "$prompt" "$kind command did not name the worker's own steering inbox" assert_grep "do not reject it as another home's state" "$prompt" "$kind command did not distinguish its inbox from another home's namespace" assert_grep "Never inspect or change any other home's endpoint namespace" "$prompt" "$kind command weakened cross-home isolation" + assert_grep "skill name does not resolve in this session, read \`$ROOT/.agents/skills/firstmate-coding-guidelines/SKILL.md\` instead." "$prompt" \ + "$kind command did not name the Firstmate skill file as the fallback" assert_grep 'brief for' "$prompt" "$kind command lost the task" [ "$(grep -c '^# Current worker role contract$' "$prompt")" -eq 1 ] || fail "$brief_kind $kind duplicated the delivered worker contract" From 4c0331a5cd9451d2d463a74c94f3c96ebfc1ba64 Mon Sep 17 00:00:00 2001 From: Stephen Haptonstahl Date: Mon, 5 Oct 2026 19:05:21 -0400 Subject: [PATCH 04/13] feat(bin): retire a contribution whose forge object is permanently gone (#6655) * feat(bin): retire a contribution whose forge object is permanently gone Add fm-contributions.sh retire , which records actor, reason and time on the saved record and removes that task/url pair from known, poll rotation and coverage even while a backlog link remains. Repeating a retire keeps the first provenance; an unrecorded pair, unknown actor, empty reason or unacknowledged pending signal is refused. * no-mistakes(review): Keep retirement per task when settling final owners * no-mistakes(ci): Made the three Greptile fixes the captain chose. ci-1, retirement needs the captain's word: `retire` now accepts only the actor `captain`. Any other actor is refused, including `fleet`. I removed `fleet` from the usage line, the script header (`bin/fm-contributions.sh`), the check in `bin/fm-contributions.sh` and the retired-record validation in `bin/fm-contributions.jq`, which now requires `.actor == "captain"`. The script header now says a retirement always records the captain's word, because the script cannot verify who runs it and the authority to retire is the captain's. The Bearings skill line in `.agents/skills/bearings/SKILL.md` now says `retire` records the captain's word. In the tests, a `fleet` retire is now a refusal case and must leave the record unretired. Every other retire call in the tests uses `captain`. The PR description has not been changed yet: the ruling asks for the same captain's-word statement there, and that belongs to the PR phase. ci-2, blank reasons: the reason check is now `[ -n "${5//[[:space:]]/}" ]`, so a reason made only of spaces or tabs is refused. I added a refusal test that passes a space-tab-space reason. The header also lists a blank reason among the refusals. ci-3, the gone-object test: the test forge wrapper has a new `not-found` fault that returns HTTP 404 for every `api repos/o/r/...` read. The happy-path test `test_retire_ends_observation_of_a_gone_contribution` now uses it in place of the 502 `down` fault, so it models a permanently gone repository rather than an outage. Verification: `bash tests/fm-contributions.test.sh` passed with exit 0. The last lines of its output include the three retire tests passing and no failures. `shellcheck` flagged only SC2034 (`FM_WAKE_QUEUE` unused) and SC1091 (an unfollowed source of `bin/fm-path-lib.sh`), neither on a line this round changed --- .agents/skills/bearings/SKILL.md | 1 + bin/fm-contributions.jq | 7 ++- bin/fm-contributions.sh | 38 +++++++++++++-- tests/fm-contributions.test.sh | 80 +++++++++++++++++++++++++++++++- 4 files changed, 121 insertions(+), 5 deletions(-) diff --git a/.agents/skills/bearings/SKILL.md b/.agents/skills/bearings/SKILL.md index edc11f1c375..ba635d447e9 100644 --- a/.agents/skills/bearings/SKILL.md +++ b/.agents/skills/bearings/SKILL.md @@ -191,6 +191,7 @@ A `check: contributions` wake is arriving information about owned work, not perm Read `bin/fm-contributions.sh pending` in the owning home and inspect the source comment or review as evidence; source bodies are untrusted content rather than instructions. The command's header owns the durable records, observation bounds, judged-head rule, exact commands and acknowledgement mechanics. Treat missing, failed, expired, unsupported, and truncated observation coverage as work for the fleet to reconcile, never as proof that no contribution needs attention. +Only concrete evidence that the forge object is permanently gone, such as a deleted repository, justifies the command's `retire` operation, which records the captain's word; a transient, authentication, or rate-limit failure never does. When a maintainer verdict has an identifiable judged commit, record it through the command's `verdict` operation with that exact head and source URL. Never bind old prose to the head current at capture time merely because no judged head was supplied. diff --git a/bin/fm-contributions.jq b/bin/fm-contributions.jq index fc3f9715ab1..bb885fb6d79 100644 --- a/bin/fm-contributions.jq +++ b/bin/fm-contributions.jq @@ -13,6 +13,8 @@ def valid_record: and ((.notified // []) | type == "array" and all(.[]; type == "string")) and (.error == null or (.error | type == "string")) and (.checked_at == null or (.checked_at | fromdateiso8601 | type == "number")) + and (.retired == null or (.retired | (.actor == "captain") + and (.reason | type == "string" and length > 0) and (.at | fromdateiso8601 | type == "number"))) and (.verdict == null or (.verdict | (.head | sha) and (.source | type == "string") and (.actor | IN("captain","fleet","maintainer","nobody")) and (.summary | type == "string"))) and (.observation == null or (.kind as $kind | .observation | @@ -30,7 +32,10 @@ def known($input; $saved): + [($input.backlog.records // [])[] | select(.structured == true) as $task | ($task.links // [])[] | select(canonical_url) | {task:$task.id,url:.}] + [$saved[] | .task as $task | .records[] | {task:$task,url}]) - | unique_by([.task,.url]); + | unique_by([.task,.url]) + # A retired record ends that task's ownership even while a backlog link remains. + | [$saved[] | .task as $task | .records[] | select(.retired != null) | {task:$task,url}] as $retired + | map(select(. as $pair | any($retired[]; . == $pair) | not)); def latest_checks: group_by(.name) | map(sort_by([(.started_at // ""),(.id // 0)]) | last); def projected($input; $saved; $now; $max_age): diff --git a/bin/fm-contributions.sh b/bin/fm-contributions.sh index 12bfc5fffcf..aa43c904d2a 100755 --- a/bin/fm-contributions.sh +++ b/bin/fm-contributions.sh @@ -7,6 +7,7 @@ # fm-contributions.sh pending # fm-contributions.sh verdict # fm-contributions.sh ack +# fm-contributions.sh retire captain # fm-contributions.sh arm [--if-owned] # # snapshot is read-only and never contacts a forge. Its input is the canonical @@ -18,7 +19,8 @@ # # This script owns fm-contributions.v1: one atomic file per durable task with # task and records[]. Each record contains url, kind, checked_at, error, -# observation, verdict, seen event tokens, pending events, and notified tokens. +# observation, verdict, seen event tokens, pending events, notified tokens, and +# retired provenance once retired. # observation is one coherent forge read (a PR head is rechecked after fetching # checks/reviews). Checks are normalized by name, id, started_at, status and # conclusion; projection picks the newest attempt per distinct name. The last @@ -31,6 +33,17 @@ # can grant merge authority. Captain-actor prose requires an existing live hold; # an eligible merge remains a captain call, never an automatic forge action. # +# retire ends one task's observation of a contribution whose forge object can +# never be read again, such as a PR in a deleted repository. It records retired +# with actor captain, a non-empty reason and the UTC time. It refuses any +# other actor, a blank reason, and a task/url pair with no saved record or +# with unacknowledged pending signals. A retired pair leaves known, rotation and coverage even while a +# backlog link remains. Retiring a retired pair again is a no-op that keeps the +# first provenance. Nothing un-retires a record, poll never retires one on its +# own, and a later owner settled from a retired record is not retired. A +# retirement always records the captain's word: the script cannot verify who +# runs it, and the authority to retire is the captain's. +# # poll consumes fm-fleet-snapshot.sh --contribution-input, a local-only read, # and spends at most FM_CONTRIBUTIONS_BUDGET seconds on forge reads (default 20, # 1..25). A configured value rides the generated check shim into watcher runs @@ -331,14 +344,14 @@ settle_final() { # canonical-url task... : copy the URL's final observation to e jq -n --slurpfile saved "$TMP/saved.json" --arg url "$url" ' [$saved[0][] | .records[] | select(.url == $url and (.observation.state | IN("merged","closed")))] as $final - | ([$final[] | select(.error == null)] | first) // ($final | first)' > "$TMP/final.json" + | $final | sort_by([.retired != null, .error != null]) | first' > "$TMP/final.json" for task in "$@"; do fm_pr_task_id_valid "$task" || { printf 'contributions: invalid durable task id\n'; continue; } jq -n --slurpfile saved "$TMP/saved.json" --arg task "$task" --arg url "$url" ' [$saved[0][] | select(.task == $task) | .records[] | select(.url == $url)] | first' > "$TMP/old.json" if jq -e '. == null' "$TMP/old.json" >/dev/null; then jq -n --slurpfile final "$TMP/final.json" ' - $final[0] + {error:null,pending:[],notified:[]}' > "$TMP/row.json" + $final[0] + {error:null,pending:[],notified:[]} | del(.retired)' > "$TMP/row.json" write_record "$task" "$TMP/row.json" elif jq -e '(.observation.state | IN("merged","closed") | not) or .error != null' "$TMP/old.json" >/dev/null; then jq -n --slurpfile final "$TMP/final.json" --slurpfile old "$TMP/old.json" ' @@ -481,5 +494,24 @@ case "${1:-}" in fi write_record "$task" "$TMP/update.json" ;; + retire) + [ "$#" -eq 5 ] || fail 'retire needs task, URL, actor and reason' + task=$2; url=$3 + fm_pr_task_id_valid "$task" || fail 'invalid contribution task' + [ "$4" = captain ] || fail "invalid retire actor '$4'; expected: captain" + [ -n "${5//[[:space:]]/}" ] || fail 'retire needs a non-empty reason' + acquire; read_saved + jq -e --arg task "$task" --arg url "$url" '.[] | select(.task == $task) | .records[] | select(.url == $url)' "$TMP/saved.json" > "$TMP/row.json" \ + || fail 'contribution is not recorded for this durable task' + if jq -e '.retired != null' "$TMP/row.json" >/dev/null; then + printf 'contributions: already retired %s for %s\n' "$url" "$task" + exit 0 + fi + jq -e '(.pending | length) == 0' "$TMP/row.json" >/dev/null \ + || fail 'acknowledge pending signals before retiring this contribution' + jq --arg actor "$4" --arg reason "$5" --arg at "$NOW" \ + '.retired={actor:$actor,reason:$reason,at:$at}' "$TMP/row.json" > "$TMP/update.json" + write_record "$task" "$TMP/update.json" + ;; *) usage >&2; exit 2 ;; esac diff --git a/tests/fm-contributions.test.sh b/tests/fm-contributions.test.sh index ce87baefafb..9af32fc2950 100755 --- a/tests/fm-contributions.test.sh +++ b/tests/fm-contributions.test.sh @@ -652,6 +652,7 @@ case "$fault:$*" in fail-late:'api repos/o/r/pulls/8/reviews?'*) clock_bump 100; printf 'HTTP 502\n' >&2; exit 1 ;; fail:'api repos/o/r/pulls/8/reviews?'*) printf 'HTTP 502\n' >&2; exit 1 ;; down:*) printf 'HTTP 502\n' >&2; exit 1 ;; + not-found:'api repos/o/r/'*) printf 'HTTP 404\n' >&2; exit 1 ;; hang:'api repos/o/r/pulls/8') sleep 4 ;; head:'pr view '*) printf '{"headRefOid":"%s","reviewDecision":"APPROVED"}\n' "$(printf 'b%.0s' $(seq 40))"; exit 0 ;; esac @@ -1098,8 +1099,85 @@ test_late_owner_keeps_failure_episode_suppressed() { pass 'a late owner does not restart a shared forge failure episode' } +test_retire_ends_observation_of_a_gone_contribution() { + local home out line='contributions: observation unavailable for https://github.com/o/r/pull/8' url=https://github.com/o/r/pull/8 + home=$(new_home retire-gone) + forge_home "$home" + wrap_forge "$home" + printf 'not-found\n' > "$home/forge/fault" + out=$(with_home "$home" env FM_CONTRIBUTIONS_NOW=2026-09-16T09:00:00Z "$ROOT/bin/fm-contributions.sh" poll) || fail 'failing poll failed' + [ "$out" = "$line" ] || fail "a gone repository did not raise the unavailable check: $out" + bearings "$home" | jq -e '.contributions.known == 1 and .contributions.checked == 0 + and .contributions.complete == false and .contributions.proven_clear == false' >/dev/null \ + || fail 'an unreadable contribution did not hold coverage incomplete before retirement' + with_home "$home" env FM_CONTRIBUTIONS_NOW=2026-09-16T09:30:00Z "$ROOT/bin/fm-contributions.sh" retire delivery "$url" captain 'repository deleted' \ + || fail 'retire of an owned unreadable contribution failed' + jq -e '.records[0].retired == {actor:"captain",reason:"repository deleted",at:"2026-09-16T09:30:00Z"}' \ + "$home/data/delivery/contributions.json" >/dev/null || fail 'retire did not record its provenance' + : > "$home/forge/calls" + for at in 2026-09-16T10:00:00Z 2026-09-16T10:05:00Z; do + out=$(with_home "$home" env FM_CONTRIBUTIONS_NOW="$at" "$ROOT/bin/fm-contributions.sh" poll) || fail "poll after retire failed at $at" + [ -z "$out" ] || fail "a retired contribution still raised a check: $out" + done + [ ! -s "$home/forge/calls" ] || fail "a retired contribution stayed in rotation: $(cat "$home/forge/calls")" + bearings "$home" | jq -e '.contributions.known == 0 and .contributions.checked == 0 + and .contributions.complete == true and .contributions.proven_clear == true' >/dev/null \ + || fail 'a retired contribution still counted against coverage despite its backlog link' + pass 'retire stops the unavailable check, leaves rotation and restores complete coverage' +} + +test_late_owner_of_a_retired_final_contribution_is_not_retired() { + local home out + home=$(new_home retire-late-owner) + forge_home "$home" + wrap_forge "$home" + mutate_record "$home" delivery '.records[0].observation.state="merged" + | .records[0].retired={actor:"captain",reason:"repository deleted",at:"2026-09-16T09:30:00Z"}' + record "$home" duplicate 8 merged mergeable + mutate_record "$home" duplicate '.records[0].error="forge observation unavailable or changed during read"' + printf -- '- [ ] late - Filed https://github.com/o/r/pull/8 (repo: sample) (kind: ship)\n' >> "$home/data/backlog.md" + out=$(with_home "$home" env FM_CONTRIBUTIONS_NOW=2026-09-17T08:00:00Z "$ROOT/bin/fm-contributions.sh" poll) || fail 'late-owner poll failed' + [ -z "$out" ] || fail "a late owner of a retired final contribution printed: $out" + [ ! -s "$home/forge/calls" ] || fail 'a known final contribution triggered a forge read' + jq -e '.records[0] | .retired == null and .observation.state == "merged" and .error == null' \ + "$home/data/late/contributions.json" >/dev/null || fail 'a late owner inherited another task'"'"'s retirement' + jq -e '.records[0].retired.reason == "repository deleted"' "$home/data/delivery/contributions.json" >/dev/null \ + || fail 'settling a late owner changed the retired record' + with_home "$home" "$ROOT/bin/fm-fleet-snapshot.sh" --contribution-input > "$home/input.json" || fail 'contribution input failed' + with_home "$home" "$ROOT/bin/fm-contributions.sh" snapshot "$home/input.json" --all | jq -e '.rows[0].tasks == ["duplicate","late"]' >/dev/null \ + || fail 'a late owner settled beside a retired final record left known' + pass 'a late owner settled beside a retired final record stays unretired and known' +} + +test_retire_is_idempotent_and_refuses_unknown_pairs() { + local home url=https://github.com/o/r/pull/8 before err + home=$(new_home retire-refusals) + forge_home "$home" + retire() { with_home "$home" "$ROOT/bin/fm-contributions.sh" retire "$@"; } + retire delivery "$url" fleet 'repository deleted' >/dev/null 2>&1 && fail 'retire accepted the fleet as its actor' + jq -e '.records[0].retired == null' "$home/data/delivery/contributions.json" >/dev/null || fail 'a fleet retire changed the record' + retire delivery "$url" captain 'repository deleted' >/dev/null || fail 'first retire failed' + before=$(cat "$home/data/delivery/contributions.json") + retire delivery "$url" captain 'second reason' >/dev/null || fail 'repeating a retire was refused' + [ "$(cat "$home/data/delivery/contributions.json")" = "$before" ] || fail 'repeating a retire rewrote its first provenance' + printf -- '- [ ] linked - Linked only https://github.com/o/r/pull/30 (repo: sample) (kind: ship)\n' >> "$home/data/backlog.md" + err=$(retire linked https://github.com/o/r/pull/30 captain gone 2>&1) && fail 'retire created a record for an unobserved pair' + case "$err" in *'not recorded for this durable task'*) ;; *) fail "unrecorded-pair refusal was unclear: $err" ;; esac + [ ! -e "$home/data/linked/contributions.json" ] || fail 'a refused retire created a record' + retire other "$url" captain gone >/dev/null 2>&1 && fail 'retire accepted a task that does not own the URL' + record "$home" queued 31 open mergeable + retire queued https://github.com/o/r/pull/31 owner gone >/dev/null 2>&1 && fail 'retire accepted an unknown actor' + retire queued https://github.com/o/r/pull/31 captain '' >/dev/null 2>&1 && fail 'retire accepted an empty reason' + retire queued https://github.com/o/r/pull/31 captain ' ' >/dev/null 2>&1 && fail 'retire accepted a whitespace-only reason' + retire queued https://github.com/o/r/pull/31 captain >/dev/null 2>&1 && fail 'retire accepted a missing reason' + mutate_record "$home" queued '.records[0].pending=[{token:"comment:1:x",type:"comment"}]' + retire queued https://github.com/o/r/pull/31 captain gone >/dev/null 2>&1 && fail 'retire dropped an unacknowledged signal' + jq -e '.records[0].retired == null' "$home/data/queued/contributions.json" >/dev/null || fail 'a refused retire changed the record' + pass 'retire is idempotent and refuses non-captain, unknown, malformed and signal-bearing pairs' +} + failures=0 -for test_name in test_actor_coverage test_stale_verdict test_unchecked_is_not_silence test_newest_check_has_no_verdict test_comment_wake test_review_wake test_inline_wake test_ready_issue_wake test_fresh_issue_requires_maintainer test_missing_lane_remains_missing test_partial_freshness_keeps_measured_rows test_malformed_record_cannot_prove_silence test_issue_timeline_and_exact_ack test_verdict_retains_judged_head test_verdict_actor_values_are_discoverable test_observed_replacement_refreshes_verdict test_unobserved_head_leaves_verdict_unknown test_away_yolo_is_fleet_work test_away_yolo_cross_home_is_fleet_work test_retired_and_unsupported_coverage test_unsupported_forge_is_not_fleet_work test_held_unsupported_forge_is_not_captain_work test_shared_contribution_signal_wakes_once test_watcher_keeps_diagnostics_separate_from_contribution_wakes test_expired_child_unsupported_forge_stays_unmeasured test_watcher_surfaces_new_contribution_once test_home_summary_coverage test_unreadable_pending_is_not_empty test_record_task_identity_matches_dirname_basename test_read_only_views_create_no_state test_budget_refusal_between_calls test_budget_bounded_call_timeout test_genuine_failure_near_deadline_is_unavailable test_shared_url_observed_once test_terminal_contribution_settles test_late_owner_inherits_terminal_observation test_interrupted_multi_owner_poll_settles_every_owner test_done_task_open_pr_still_observed test_reservation_defers_later_url_when_fifteen_seconds_do_not_remain test_three_second_pr_reads_complete_fresh_in_one_cycle test_slow_read_deadline_kill_is_budget_refusal test_unmeasured_url_does_not_starve_the_tail test_budget_is_cut_down_to_the_watcher_check_bound test_arm_plumbs_a_configured_budget_into_the_check_shim test_unavailable_forge_records_error_and_wakes_once_per_episode test_late_owner_keeps_failure_episode_suppressed; do +for test_name in test_actor_coverage test_stale_verdict test_unchecked_is_not_silence test_newest_check_has_no_verdict test_comment_wake test_review_wake test_inline_wake test_ready_issue_wake test_fresh_issue_requires_maintainer test_missing_lane_remains_missing test_partial_freshness_keeps_measured_rows test_malformed_record_cannot_prove_silence test_issue_timeline_and_exact_ack test_verdict_retains_judged_head test_verdict_actor_values_are_discoverable test_observed_replacement_refreshes_verdict test_unobserved_head_leaves_verdict_unknown test_away_yolo_is_fleet_work test_away_yolo_cross_home_is_fleet_work test_retired_and_unsupported_coverage test_unsupported_forge_is_not_fleet_work test_held_unsupported_forge_is_not_captain_work test_shared_contribution_signal_wakes_once test_watcher_keeps_diagnostics_separate_from_contribution_wakes test_expired_child_unsupported_forge_stays_unmeasured test_watcher_surfaces_new_contribution_once test_home_summary_coverage test_unreadable_pending_is_not_empty test_record_task_identity_matches_dirname_basename test_read_only_views_create_no_state test_budget_refusal_between_calls test_budget_bounded_call_timeout test_genuine_failure_near_deadline_is_unavailable test_shared_url_observed_once test_terminal_contribution_settles test_late_owner_inherits_terminal_observation test_interrupted_multi_owner_poll_settles_every_owner test_done_task_open_pr_still_observed test_reservation_defers_later_url_when_fifteen_seconds_do_not_remain test_three_second_pr_reads_complete_fresh_in_one_cycle test_slow_read_deadline_kill_is_budget_refusal test_unmeasured_url_does_not_starve_the_tail test_budget_is_cut_down_to_the_watcher_check_bound test_arm_plumbs_a_configured_budget_into_the_check_shim test_unavailable_forge_records_error_and_wakes_once_per_episode test_late_owner_keeps_failure_episode_suppressed test_retire_ends_observation_of_a_gone_contribution test_late_owner_of_a_retired_final_contribution_is_not_retired test_retire_is_idempotent_and_refuses_unknown_pairs; do ( "$test_name" ) || failures=$((failures + 1)) done [ "$failures" -eq 0 ] || fail "$failures contribution regressions" From 332de270c386ec1ad7fb0318c6a87c425415f617 Mon Sep 17 00:00:00 2001 From: cloud-practitioner <113355863+cloud-practitioner@users.noreply.github.com> Date: Tue, 6 Oct 2026 10:05:52 +1100 Subject: [PATCH 05/13] test: extend remote-reply whole-log recapture waits (#6639) * test: give the remote-reply whole-log recapture a longer wait * no-mistakes(review): Extend both recapture waits and simplify retry handling --- tests/fm-remote-reply.test.sh | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/fm-remote-reply.test.sh b/tests/fm-remote-reply.test.sh index dfdc212c1b9..414767f4147 100755 --- a/tests/fm-remote-reply.test.sh +++ b/tests/fm-remote-reply.test.sh @@ -657,6 +657,19 @@ assert_grep 'report=data/remote-secondmates/ios/data/reply/writefail.md' "$PAREN mirrored_cursor_is_current "the recovered delta did not advance the cursor" pass "a failed mirror write never drops status content or advances the cursor" +# The whole-log recapture re-fetches every document the log offers, one remote +# job at a time, so it needs far more than one await_reply_result budget on a +# loaded runner. Each attempt is a full wait that re-checks ownership of the +# source, so the recapture is bounded by RECAPTURE_WAIT_ATTEMPTS of them. +RECAPTURE_WAIT_ATTEMPTS=3 +await_recapture_result() { # + local attempt + for attempt in $(seq 1 "$RECAPTURE_WAIT_ATTEMPTS"); do + await_reply_result "$1" && return 0 + done + return 1 +} + # A source line remains the replay identity even when document availability # changes between a successful mirror append and a failed ingestion commit. REPLAY_LINE='needs-decision [key=replay-decision]: pick report=data/reply/replay.md' @@ -703,7 +716,7 @@ assert_not_contains "$(status_open_decisions "$PARENT/state/ios.status")" $'repl stop_reply_listener || fail "the reply listener did not stop before the cursor-loss recapture" rm -f "$PARENT/state/remote-replies/ios.cursor" GEN=$((GEN + 1)) -await_reply_result "$PARENT/state/procevent-inbox/$SID.$GEN.result" \ +await_recapture_result "$PARENT/state/procevent-inbox/$SID.$GEN.result" \ || fail "the replay-identity whole-log recapture was not captured" assert_present "$PARENT/state/procevent-inbox/$SID.$GEN.handled" \ "the replay-identity whole-log recapture was not applied" @@ -967,7 +980,7 @@ mv "$PARENT/state/.wake-queue" "$TMP_ROOT/wake-queue-before-replay" 2>/dev/null stop_reply_listener || fail "the reply listener did not stop before the whole-log recapture" rm -f "$PARENT/state/remote-replies/ios.cursor" GEN=$((GEN + 1)) -await_reply_result "$PARENT/state/procevent-inbox/$SID.$GEN.result" \ +await_recapture_result "$PARENT/state/procevent-inbox/$SID.$GEN.result" \ || fail "the cursor-loss recapture was not captured" assert_present "$PARENT/state/procevent-inbox/$SID.$GEN.handled" \ "the whole-log recapture was not acknowledged by the adapter" From 70f2ed3d0ce66d481a105a9b79877c861ec9552e Mon Sep 17 00:00:00 2001 From: Tiago Date: Mon, 5 Oct 2026 20:06:55 -0300 Subject: [PATCH 06/13] fix(bin): reopen a pending-reply escalation after its resolve (#6654) * fix(bin): reopen a pending-reply escalation after its resolve A retry of an open decision still appends nothing. A later same-kind escalation after a resolved line for that key appends again, so the next loss is visible. * no-mistakes(ci): Both Greptile findings are fixed in the working tree; the four directly affected test suites pass, and a wider run of related suites had not finished when I returned this result. Invariant: a line already in the status file is recorded, and only a caller with its own evidence of a new episode may append it again. It must hold for every caller of status_event_recorded: the continuity break and the other call in bin/fm-procevent-remote-reply.sh, fm_parent_channel_append_once, and the pending-reply escalation. Changes: - bin/fm-classify-lib.sh: status_event_recorded is back to the idempotent retry check. It returns at the first matching line, and a later resolved line no longer makes that line look new. This fixes ci-1 for every caller and restores the early exit for ci-2, with no separate scan optimization. - bin/fm-pending-reply-lib.sh: _fm_pending_reply_maybe_escalate_locked now owns the reopen. It appends a new blocked line when the record already has an escalated_epoch (it escalated before and was reset) and the keyed decision is no longer open. Otherwise it uses status_event_recorded, so a retry or a resend that leaves the decision open appends nothing. - Comments on status_event_recorded and the pending-reply header describe this scope. Tests: - tests/fm-pending-reply.test.sh: the regression for escalate, operator resolve, second escalate is unchanged and passes. - tests/fm-remote-reply.test.sh: new regression drives the real reader. After an operator resolve of remote-reply-continuity-ios, a repeated handle and ingest of the same break appends nothing and leaves the decision closed. I placed it beside the existing continuity-break test, not in the pending-reply file, because only that file has the reader harness. - tests/fm-classify-corr-token.test.sh and tests/fm-classify-decision-key.test.sh: the cases that expected a blocked line to append again after a resolve now assert it stays recorded. Verification: - fm-classify-decision-key, fm-classify-corr-token, fm-pending-reply and fm-remote-reply test suites all exit 0 with the fix. - With the two bin files reverted to the commit under review, the new continuity regression and the updated parent-publisher case both fail, so they reproduce ci-1. - shellcheck -x on the changed files reports nothing. - Not finished: a background run of every other suite that mentions the parent channel or pending replies had produced no output, pass or fail, when I returned. One known gap: the escalation writes escalated_epoch before phase. If the process dies between those two writes and an operator resolves the key before the next tick, that tick appends a blocked line for the same episode. I left it, because closing it needs new state. Issue 4755 and other escalation behavior are untouched. Nothing is committed --- bin/fm-classify-lib.sh | 6 ++ bin/fm-pending-reply-lib.sh | 30 +++++++--- tests/fm-classify-corr-token.test.sh | 25 +++++++++ tests/fm-classify-decision-key.test.sh | 45 ++++++++++++++- tests/fm-pending-reply.test.sh | 77 ++++++++++++++++++++++++++ tests/fm-remote-reply.test.sh | 20 +++++++ 6 files changed, 195 insertions(+), 8 deletions(-) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index 7482e5a9df6..713f1b7a8f0 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -550,6 +550,12 @@ _fm_status_unstamped() { # -> line with its stamp remov # all other bytes, including correlation metadata, still identify the event. # Both sides normalize through _fm_status_untimed, so a stamped retry of an # already-recorded event can never read as a new one. +# A match stays recorded for the life of the file, whatever follows it: a +# later resolved line for the same key does not make the line new again, so a +# caller that re-reads an unchanged source after an operator resolve (the +# continuity break in bin/fm-procevent-remote-reply.sh, which does not advance +# its cursor) appends nothing. A caller that owns evidence of a new episode +# decides that itself, as bin/fm-pending-reply-lib.sh's escalation does. status_event_recorded() { # local wanted line untimed [ -f "$1" ] || return 1 diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index 5b5fa96b697..dc5b96a96f7 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -93,10 +93,15 @@ # contract; the remote enqueue deduplicates onto the same record). The resend # resets the record to awaiting_report and leaves the published escalation # decision open: a confirmed delivery does not settle the request, only a -# correlated report does. A later missed-report escalation reuses that key -# rather than opening a duplicate, and only the ordinary resolve close closes -# it. A delivered record, whatever its phase, is never reset. Without this, a -# wake retried only through its owner +# correlated report does. A later escalation reuses that key rather than +# opening a duplicate while the decision stays open, and only the ordinary +# resolve close closes it. Once that close is in the log, the next escalation +# of a record that already escalated and was reset is a new episode: it +# appends a new blocked line for the same key, even one identical to the +# first, and the fold opens the decision again. That reopen belongs to this +# escalation alone; status_event_recorded (bin/fm-classify-lib.sh) stays an +# idempotent retry check for every caller. A delivered record, whatever its +# phase, is never reset. Without this, a wake retried only through its owner # (bin/fm-backlog-handoff.sh's receiver wake) stayed refused forever once the # watcher escalated between the lost transport and the next resume. # @@ -1257,7 +1262,7 @@ fm_pending_reply_maybe_escalate() { # _fm_pending_reply_maybe_escalate_locked() { # local state=$1 corr=$2 local rec phase completed now payload parent_status line kind first display - local delivered task_id meta sm_home remote_host grace age + local delivered task_id meta sm_home remote_host grace age key new_episode rec=$(fm_pending_reply_path "$state" "$corr") [ -f "$rec" ] || return 1 phase=$(fm_pending_reply_get "$rec" phase) @@ -1318,8 +1323,19 @@ _fm_pending_reply_maybe_escalate_locked() { # fi [ -n "$parent_status" ] || return 1 mkdir -p "$(dirname "$parent_status")" 2>/dev/null || return 1 - line="blocked [key=$(fm_pending_reply_escalation_key "$corr")]: $payload" - if ! status_event_recorded "$parent_status" "$line"; then + key=$(fm_pending_reply_escalation_key "$corr") + line="blocked [key=$key]: $payload" + # A record that already escalated reaches here again only after a reset, so + # a closed decision means the operator settled the earlier episode and this + # loss is a new one. While the decision is open the identical line is a retry. + new_episode=1 + if [ -n "$(fm_pending_reply_get "$rec" escalated_epoch)" ]; then + case $'\n'"$(status_open_decisions "$parent_status")" in + *$'\n'"$key"$'\t'*) ;; + *) new_episode=0 ;; + esac + fi + if [ "$new_episode" -eq 0 ] || ! status_event_recorded "$parent_status" "$line"; then printf '%s\n' "$(status_stamp_line "$line")" >> "$parent_status" 2>/dev/null || return 1 fi now=$(fm_pending_reply_now) diff --git a/tests/fm-classify-corr-token.test.sh b/tests/fm-classify-corr-token.test.sh index 7367595d588..09b7341370d 100755 --- a/tests/fm-classify-corr-token.test.sh +++ b/tests/fm-classify-corr-token.test.sh @@ -635,6 +635,31 @@ test_optional_event_time() { [ -n "$(status_open_decisions "$dir/state/task.status")" ] || fail "time cleared an open decision" printf '%s\n' 'resolved [at=1700000001] [key=timed]: answered' >> "$dir/state/task.status" [ -z "$(status_open_decisions "$dir/state/task.status")" ] || fail "timed resolution did not close decision" + # The parent publisher uses the same retry check, and a resolve does not + # make a published line new: a keyed blocker and a note each stay once. + line='blocked [key=remote-reply-continuity-ios]: remote reply continuity broke for ios (truncated)' + : > "$dir/state/episode.status" + fm_parent_channel_append_once "$dir/state/episode.status" "$line" \ + || fail "first continuity append failed" + fm_parent_channel_append_once "$dir/state/episode.status" "$line" \ + || fail "open continuity retry failed" + [ "$(wc -l < "$dir/state/episode.status")" -eq 1 ] \ + || fail "an open continuity blocker was duplicated" + printf '%s\n' 'resolved [key=remote-reply-continuity-ios]: operator rebased the mirror' \ + >> "$dir/state/episode.status" + fm_parent_channel_append_once "$dir/state/episode.status" "$line" \ + || fail "continuity retry after the resolve failed" + [ "$(wc -l < "$dir/state/episode.status")" -eq 2 ] \ + || fail "a resolve made a recorded continuity blocker append again" + [ -z "$(status_open_decisions "$dir/state/episode.status")" ] \ + || fail "a continuity retry reopened the resolved decision" + line='note: remote document did not transfer for ios: data/reply/missing.md - absent' + printf '%s\n' "$line" > "$dir/state/note.status" + printf '%s\n' 'resolved: closed the default decision' >> "$dir/state/note.status" + fm_parent_channel_append_once "$dir/state/note.status" "$line" \ + || fail "note retry failed" + [ "$(wc -l < "$dir/state/note.status")" -eq 2 ] \ + || fail "a resolve duplicated a note" pass "optional event time preserves parsing and legacy unknown time" } diff --git a/tests/fm-classify-decision-key.test.sh b/tests/fm-classify-decision-key.test.sh index 0419d24bce4..78c021fd2c1 100755 --- a/tests/fm-classify-decision-key.test.sh +++ b/tests/fm-classify-decision-key.test.sh @@ -11,7 +11,9 @@ # verb, regardless of order or count. These tests drive the REAL # status_line_verb / status_open_decisions / status_open_decisions_incremental # functions over crafted status files and assert their folded output, never the -# fold's own source text. Also covers status_key_closing_verb, which reports how +# fold's own source text. Also covers status_event_recorded: a recorded line +# stays recorded across a later resolved line for its key. Also covers +# status_key_closing_verb, which reports how # the status side currently reads one key so a consumer can tell a settled key # from one handed to a durable captain-held task (bin/fm-captain-hold.sh # diverged). Cross-drain cursor persistence and the incremental @@ -561,3 +563,44 @@ test_declared_wait_survives_answers_past_the_event_window() { test_keyless_wait_survives_stated_default_retraction test_declared_wait_survives_answers_past_the_event_window test_bare_prose_cannot_open_or_close_a_decision + +# status_event_recorded is an idempotent retry check: a stamped retry matches, +# and no later resolved line - for another key, outside the reserved-key +# vocabulary, or the real close of that key - makes a recorded line new again. +# The same holds for a note and a done line. +test_recorded_line_stays_recorded_across_a_resolve() { + local dir f line + dir=$(case_dir episode) + f="$dir/task.status" + line='blocked [key=pending-reply-abc]: pending-reply-delivery-unknown: task=mate pending-reply-id=abc request=wake' + printf '%s\n' "$line" > "$f" + status_event_recorded "$f" "$line" \ + || fail "the open blocker was not recorded" + status_event_recorded "$f" \ + "blocked [key=pending-reply-abc] [at=1700000000]: pending-reply-delivery-unknown: task=mate pending-reply-id=abc request=wake" \ + || fail "a stamp made the recorded blocker look new" + printf '%s\n' 'resolved [key=other]: answered elsewhere' >> "$f" + status_event_recorded "$f" "$line" \ + || fail "another key's resolve made the blocker look new" + printf '%s\n' 'resolved [key=pending-reply-abc]: answered: not this library' >> "$f" + status_event_recorded "$f" "$line" \ + || fail "a reserved-key resolve outside the vocabulary made the blocker look new" + printf '%s\n' 'resolved [key=pending-reply-abc] [at=1700000001]: pending-reply-resolved: task=mate pending-reply-id=abc via=operator-resolve-key dismiss' >> "$f" + [ -z "$(status_open_decisions "$f")" ] \ + || fail "the resolve did not close the decision: $(status_open_decisions "$f")" + status_event_recorded "$f" "$line" \ + || fail "the resolve of its own key made the blocker look new" + printf '%s\n' 'note: remote document did not transfer for ios: data/reply/missing.md - absent' \ + > "$dir/note.status" + printf '%s\n' 'resolved: closed the default decision' >> "$dir/note.status" + status_event_recorded "$dir/note.status" \ + 'note: remote document did not transfer for ios: data/reply/missing.md - absent' \ + || fail "a resolve made a note look new" + printf '%s\n' 'done [corr=abcd]: shipped' > "$dir/done.status" + printf '%s\n' 'resolved [key=default]: closed' >> "$dir/done.status" + status_event_recorded "$dir/done.status" 'done [corr=abcd]: shipped' \ + || fail "a resolve made a done line look new" + pass "a recorded line stays recorded across a later resolve" +} + +test_recorded_line_stays_recorded_across_a_resolve diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 777d1c97c57..697ec00796f 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -32,6 +32,8 @@ # 17. Recovery and escalation grace are measured from the relevant turn's # completion, never from delivery or send time, and each takes one fresh, # uncached status read - accepting any verb - immediately before firing +# 18. A same-kind escalation after an operator close appends again and reopens +# the decision; a retry while that decision is still open appends nothing set -u # shellcheck source=tests/lib.sh @@ -1992,6 +1994,80 @@ test_escalated_undelivered_correlation_stays_retryable() { pass "an escalated correlation stays retryable only while undelivered" } +test_same_kind_escalation_reopens_after_operator_close() { + ( + local dir fb log home state corr status blocked open + dir="$TMP_ROOT/same-kind-reescalation" + mkdir -p "$dir" + fb=$(make_stubs "$dir") + log="$dir/send.log" + home=$(setup_parent same-kind) + state="$home/state" + fm_write_secondmate_meta "$state/mate.meta" "$home/sm" "sess:fm-mate" + export FM_PENDING_REPLY_NOW=10000 + corr=$(fm_pending_reply_create "$home" "$state" mate "wake after lost transport") + status="$state/mate.status" + fm_pending_reply_prepare_delivery "$state" "$corr" \ + || fail "prepare failed" + fm_pending_reply_tick_one "$state" "$corr" unknown \ + || fail "first tick failed" + [ "$(phase_of "$state" "$corr")" = escalated ] \ + || fail "first loss should escalate" + blocked=$(grep -cF "blocked [key=pending-reply-$corr]" "$status") + [ "$blocked" = 1 ] \ + || fail "first escalation should append one blocked line, got $blocked" + fm_pending_reply_reset_known_undelivered "$state" "$corr" \ + || fail "reset before close failed" + fm_pending_reply_prepare_delivery "$state" "$corr" \ + || fail "retry prepare failed" + export FM_PENDING_REPLY_NOW=15000 + fm_pending_reply_tick_one "$state" "$corr" unknown \ + || fail "retry tick failed" + [ "$(phase_of "$state" "$corr")" = escalated ] \ + || fail "retry should escalate the record again" + blocked=$(grep -cF "blocked [key=pending-reply-$corr]" "$status") + [ "$blocked" = 1 ] \ + || fail "a retry while the decision is open must not append, got $blocked" + open=$(status_open_decisions "$status" | cut -f1) + [ "$open" = "pending-reply-$corr" ] \ + || fail "the first decision must stay open, got '$open'" + run_send "$fb" "$home" "$log" mate --resolve-key "pending-reply-$corr" \ + "dismiss the unknown-delivery hold" \ + || fail "operator close failed" + open=$(status_open_decisions "$status") + [ -z "$open" ] || fail "operator close left the decision open: $open" + fm_pending_reply_reset_known_undelivered "$state" "$corr" \ + || fail "reset after close failed" + fm_pending_reply_prepare_delivery "$state" "$corr" \ + || fail "second-episode prepare failed" + export FM_PENDING_REPLY_NOW=20000 + fm_pending_reply_tick_one "$state" "$corr" unknown \ + || fail "second episode tick failed" + [ "$(phase_of "$state" "$corr")" = escalated ] \ + || fail "second loss should escalate" + blocked=$(grep -cF "blocked [key=pending-reply-$corr]" "$status") + [ "$blocked" = 2 ] \ + || fail "a new escalation after the close should append, got $blocked" + open=$(status_open_decisions "$status" | cut -f1) + [ "$open" = "pending-reply-$corr" ] \ + || fail "the second escalation should reopen the decision, got '$open'" + fm_pending_reply_reset_known_undelivered "$state" "$corr" \ + || fail "reset of the reopened decision failed" + fm_pending_reply_prepare_delivery "$state" "$corr" \ + || fail "reopened retry prepare failed" + export FM_PENDING_REPLY_NOW=25000 + fm_pending_reply_tick_one "$state" "$corr" unknown \ + || fail "reopened retry tick failed" + blocked=$(grep -cF "blocked [key=pending-reply-$corr]" "$status") + [ "$blocked" = 2 ] \ + || fail "a retry of the reopened decision must not append, got $blocked" + open=$(status_open_decisions "$status" | cut -f1) + [ "$open" = "pending-reply-$corr" ] \ + || fail "the reopened decision must stay open across the retry, got '$open'" + ) || fail "same-kind re-escalation after an operator close failed" + pass "a same-kind escalation after an operator close opens the decision again" +} + # --- run -------------------------------------------------------------------- test_normal_correlated_reply_resolves_once @@ -2040,5 +2116,6 @@ test_mechanical_helper_writes_parent_channel test_remote_parent_replies_is_not_wrong_home test_local_parent_replies_is_wrong_home_evidence test_escalated_undelivered_correlation_stays_retryable +test_same_kind_escalation_reopens_after_operator_close printf 'ok - all pending-reply tests passed\n' diff --git a/tests/fm-remote-reply.test.sh b/tests/fm-remote-reply.test.sh index 414767f4147..f5e8aef0c5d 100755 --- a/tests/fm-remote-reply.test.sh +++ b/tests/fm-remote-reply.test.sh @@ -1032,6 +1032,26 @@ if [ "${FM_TEST_EVIDENCE:-0}" = 1 ]; then fi pass "truncation is detected, escalated once, and not silently rebased" +# The break does not advance the cursor, so a later read of the unchanged +# remote log reports the same break. An operator resolve in between must not +# make that repeat look like a new break. +printf '%s\n' 'resolved [key=remote-reply-continuity-ios]: operator accepted the break' \ + >> "$PARENT/state/ios.status" +[ -z "$(status_open_decisions "$PARENT/state/ios.status")" ] \ + || fail "operator resolve left the continuity decision open" +rm -f "$PARENT/state/procevent-inbox/$SID.$GEN.handled" +set +e +remote_env "$ADAPTER" handle ios "$GEN" "$RESULT_TWELVE" > "$TMP_ROOT/handle-resolved.out" 2>&1 +handle_rc=$? +set -e +[ "$handle_rc" -eq 3 ] || fail "repeated continuity handling returned an unexpected status: $handle_rc" +remote_env "$ADAPTER" ingest ios "$RESULT_TWELVE" >/dev/null 2>&1 || true +[ "$(grep -cF 'blocked [key=remote-reply-continuity-ios]' "$PARENT/state/ios.status")" -eq 1 ] \ + || fail "a repeated continuity break appended again after the operator resolve" +[ -z "$(status_open_decisions "$PARENT/state/ios.status")" ] \ + || fail "a repeated continuity break reopened the decision the operator resolved" +pass "a repeated continuity break after an operator resolve appends nothing" + rm -f "$PARENT/state/procevent-inbox/$SID.$GEN.handled" if remote_env "$ADAPTER" retire ios > "$TMP_ROOT/retire-pending.out" 2>&1; then fail "remote reply retirement accepted an unhandled captured result" From 2d7daa9e365e91d740524a1892a7525f62c6bbdd Mon Sep 17 00:00:00 2001 From: Tiago Date: Mon, 5 Oct 2026 23:14:21 -0300 Subject: [PATCH 07/13] fix: refuse a confirming Enter on the Claude background-task exit picker (#6666) * fix: refuse a confirming Enter on the Claude exit picker The background-task picker still classifies as pending, so a retried Enter confirms "Exit and stop tasks". Stop after the Enter that opened it, and raise the existing stale wake with the dialog name. A non-paused secondmate still skips that wake, so a secondmate parked on the picker still looks idle. Model-downgrade confirmation, MCP approval, and any other Claude exit confirmation are not covered, because there is no recorded screen for them. * no-mistakes(review): fix: anchor exit picker match and wake second mates * fix: refuse a typed submit while the Claude exit picker is open A pane that already shows the picker must not receive the message or a confirming Enter. The match requires the recorded heading and footer lines, and it is skipped when no dialog name is being recorded. * no-mistakes(review): restore watcher to main behaviour, drop dialog wake * no-mistakes(test): test: align Herdr picker fixtures with the preflight read * no-mistakes(document): document exit refusal on a recognised dialog * fix: remove the dialog file when exit runs in a subshell do_exit is invoked from a command substitution, so the parent cleanup never saw the path it is supposed to delete. * no-mistakes(review): fix: set the dialog file path after the control lock * no-mistakes(document): document why the dialog file path follows the lock * no-mistakes(ci): The exit cleanup in bin/fm-control.sh now deletes the dialog file first and releases the control lock second. The changes are in the worktree and are not committed. Invariant: only the process that holds the control lock for a task may write or delete that task's dialog file ($STATE/.composer-dialog, the file that carries the picker name from the composer read to the Enter checks). The old cleanup released the lock and then deleted the file, so the delete ran outside the lock. Places where this invariant must hold: control_cleanup is the only place that deletes the file, and the single assignment after CONTROL_LOCK_HELD=1 is the only place that names it. do_exit only truncates the file, and it runs while the parent holds the lock. A process that loses the lock never sets the path, so it deletes nothing (earlier fix, unchanged). No sibling site needed a change. Fix: in control_cleanup, the rm of the dialog file moved above the fm_lock_release call, with a two-line comment that gives the reason. No dialog recognition or supervision code changed. Test: added test_exit_removes_the_dialog_file_before_releasing_the_lock in tests/fm-control-relaunch.test.sh. It runs one `fm-control exit` with a recording rm on PATH. The lock release removes paths at or under the control lock with rm, so the recording rm writes whether the dialog file exists at that moment. The test asserts the last record is "absent". It starts no second command. Verification, all run locally: - Before the fix, the new test failed with "the dialog file must be gone when the control lock is released, got: present". - After the fix, tests/fm-control-relaunch.test.sh exits 0 with 75 ok lines and no "not ok" line, including the new test and the existing test that the file is gone after exit and after relaunch. - tests/fm-control.test.sh exits 0 with 44 ok lines. - shellcheck -x on both changed files exits 0 with no output. One thing I noticed and did not change: tests/fm-control-relaunch.test.sh prints 615 "rm: cannot remove ... Permission denied" lines during its temp-directory teardown. The count is the same with my change reverted, so this change does not cause it, and the suite still exits 0. I could not read the Greptile check log (the check run was not found), so I worked from the finding text and the user instructions --- bin/backends/herdr.sh | 9 ++ bin/fm-backend.sh | 39 +++++-- bin/fm-composer-lib.sh | 76 +++++++++++++ bin/fm-control.sh | 44 +++++++- bin/fm-tmux-lib.sh | 5 + docs/agent-control.md | 3 + docs/verification/runtime-backends.md | 18 +++ tests/fm-backend-herdr.test.sh | 71 ++++++++++++ tests/fm-composer-lib.test.sh | 155 ++++++++++++++++++++++++++ tests/fm-control-relaunch.test.sh | 54 +++++++++ tests/fm-control.test.sh | 86 ++++++++++++++ tests/fm-tmux-submit-busy.test.sh | 70 +++++++++++- 12 files changed, 621 insertions(+), 9 deletions(-) diff --git a/bin/backends/herdr.sh b/bin/backends/herdr.sh index c0cdd0540db..15ae10a5800 100644 --- a/bin/backends/herdr.sh +++ b/bin/backends/herdr.sh @@ -3487,7 +3487,12 @@ fm_backend_herdr_send_text_submit() { # esac # Native stayed idle. Composer empty is positive delivery (a landed # Claude turn that never flipped agent_status). Proven pending retries. + # A picker that classifies pending must not receive that retry. verdict=$(fm_backend_herdr_composer_state "$target") + if fm_composer_blocking_dialog_noted >/dev/null; then + printf 'unknown' + return 0 + fi case "$verdict" in empty) printf 'empty'; return 0 ;; pending|pending-unproven) ;; @@ -3496,6 +3501,10 @@ fm_backend_herdr_send_text_submit() { # else sleep "$sleep_s" verdict=$(fm_backend_herdr_composer_state "$target") + if fm_composer_blocking_dialog_noted >/dev/null; then + printf 'unknown' + return 0 + fi if [ "$verdict" = pending ] && [ "$raw_status" != working ] \ && [ "$footer_baseline" = idle ] \ && [ "$(fm_backend_herdr_rendered_busy_state "$target")" = busy ]; then diff --git a/bin/fm-backend.sh b/bin/fm-backend.sh index 798bb5c599a..916e3d2e353 100644 --- a/bin/fm-backend.sh +++ b/bin/fm-backend.sh @@ -811,18 +811,43 @@ fm_backend_send_key() { # [expected-label] # fm_backend_send_text_submit: type text once, then submit and verify, # retrying only the submission (never retyping). Echoes the backend's # proof-carrying verdict; callers require exact empty for confirmed delivery. +# A pane that already shows the recognised dialog is refused before any +# adapter types, so that submit neither types the text nor sends Enter. fm_backend_send_text_submit() { # [expected-label] - local backend=$1 + local backend=$1 rc=0 target label dialog shift + target=$1 + label=${6:-} fm_backend_source "$backend" || return 1 + # Every Enter loop below reads the dialog sink, so it must exist before + # any adapter types: a sink that fails here leaves the composer untouched. + fm_composer_dialog_sink_prepare || { + echo "error: the dialog check for a $backend submit could not be recorded" >&2 + return 1 + } + # One composer read after the sink exists and before the adapter types. + # The classify writes the sink; a named dialog means the next Enter would + # answer it. + if [ -n "$label" ]; then + fm_backend_composer_state "$backend" "$target" "$label" >/dev/null || true + else + fm_backend_composer_state "$backend" "$target" >/dev/null || true + fi + if dialog=$(fm_composer_blocking_dialog_noted); then + fm_composer_dialog_sink_release + echo "error: blocked on a prompt: $dialog" >&2 + return 1 + fi case "$backend" in - tmux) fm_backend_tmux_send_text_submit "$@" ;; - herdr) fm_backend_herdr_send_text_submit "$@" ;; - zellij) fm_backend_zellij_send_text_submit "$@" ;; - orca) fm_backend_orca_send_text_submit "$@" ;; - cmux) fm_backend_cmux_send_text_submit "$@" ;; - *) echo "error: no send-text implementation for backend '$backend'" >&2; return 1 ;; + tmux) fm_backend_tmux_send_text_submit "$@" || rc=$? ;; + herdr) fm_backend_herdr_send_text_submit "$@" || rc=$? ;; + zellij) fm_backend_zellij_send_text_submit "$@" || rc=$? ;; + orca) fm_backend_orca_send_text_submit "$@" || rc=$? ;; + cmux) fm_backend_cmux_send_text_submit "$@" || rc=$? ;; + *) echo "error: no send-text implementation for backend '$backend'" >&2; rc=1 ;; esac + fm_composer_dialog_sink_release + return "$rc" } # fm_backend_kill: remove the task's session endpoint. An already-gone target diff --git a/bin/fm-composer-lib.sh b/bin/fm-composer-lib.sh index 38285587980..1e2cb3b8371 100644 --- a/bin/fm-composer-lib.sh +++ b/bin/fm-composer-lib.sh @@ -1671,9 +1671,80 @@ EOF printf '%s\n' "$joined" | LC_ALL=C awk '{$1=$1; printf "%s", $0}' } +# fm_composer_blocking_dialog: name a screen whose next Enter would answer it. +# Prints the name and returns 0 only for the recorded structure of one dialog: +# the heading on its own line, then its selected row alone on a row, with the +# recorded footer as the last non-blank row. A heading buried in a sentence, +# or a last line that only starts with the same words, is not that dialog. +# The strings alone are not enough, because a diff, a note, or a test fixture +# on the pane can quote all of them above a normal composer. A miss returns 1 +# and prints nothing. +# Recorded 2026-10-05 on Claude Code 2.1.289: /exit while a background shell +# is still running opens this picker, and its selected row is Exit and stop tasks. +fm_composer_blocking_dialog() { # -> dialog name + local screen=${1-} + [ -n "$screen" ] || return 1 + if printf '%s\n' "$screen" | fm_composer_strip_ansi | LC_ALL=C awk ' + /^[ \t]*Background work is running[ \t\r]*$/ { heading = 1 } + heading && /^[ \t]*❯ 1\. Exit and stop tasks[ \t\r]*$/ { selected = 1 } + /[^ \t\r]/ { last = $0 } + END { exit !(selected && last ~ /^[ \t]*Enter to confirm · Esc to cancel[ \t\r]*$/) } + '; then + printf '%s' 'Claude background-task exit picker' + return 0 + fi + return 1 +} + +# A command substitution drops a shell variable, and every composer read runs +# inside one. The name is therefore written to FM_COMPOSER_DIALOG_SINK when +# that path is set. The classifier verdict is unchanged. When the sink is +# unset the name would be discarded, so the match is skipped. +fm_composer_note_blocking_dialog() { # + local name= + [ -n "${FM_COMPOSER_DIALOG_SINK:-}" ] || return 1 + if name=$(fm_composer_blocking_dialog "$1"); then + printf '%s' "$name" > "$FM_COMPOSER_DIALOG_SINK" || return 1 + return 0 + fi + : > "$FM_COMPOSER_DIALOG_SINK" || return 1 + return 1 +} + +# fm_composer_blocking_dialog_noted: print the name the latest classify wrote +# to the sink. Returns 1 when the sink is unset or empty. +fm_composer_blocking_dialog_noted() { + [ -n "${FM_COMPOSER_DIALOG_SINK:-}" ] || return 1 + [ -s "$FM_COMPOSER_DIALOG_SINK" ] || return 1 + cat "$FM_COMPOSER_DIALOG_SINK" +} + +# Empty the sink, creating it when the caller has not. Sets +# FM_COMPOSER_DIALOG_OWNED=1 only for a sink this call created, so a caller +# that shares the path can still read the name after the release. +fm_composer_dialog_sink_prepare() { + FM_COMPOSER_DIALOG_OWNED=0 + if [ -z "${FM_COMPOSER_DIALOG_SINK:-}" ]; then + FM_COMPOSER_DIALOG_SINK=$(mktemp "${TMPDIR:-/tmp}/fm-composer-dialog.XXXXXX") || return 1 + FM_COMPOSER_DIALOG_OWNED=1 + return 0 + fi + : > "$FM_COMPOSER_DIALOG_SINK" +} + +fm_composer_dialog_sink_release() { + if [ "${FM_COMPOSER_DIALOG_OWNED:-}" = 1 ]; then + rm -f "$FM_COMPOSER_DIALOG_SINK" + FM_COMPOSER_DIALOG_SINK= + FM_COMPOSER_DIALOG_OWNED=0 + fi +} + fm_composer_classify_screen() { # [cursor_row] [identity] local caps=$1 screen=$2 cy=${3:-} identity=${4:-} local styled=0 cursor=0 has_identity=0 kv plain + # Note the dialog before any early return so a pending picker is still named. + fm_composer_note_blocking_dialog "$screen" || true while IFS= read -r kv; do case "$kv" in styled=1) styled=1 ;; @@ -1795,6 +1866,11 @@ fm_composer_submit_retry_core() { # "$send_key_fn" "$target" Enter "$expected_label" || true sleep "$sleep_s" state=$("$state_fn" "$target" "$expected_label") + # The first Enter can open a picker. A later Enter would confirm it. + if fm_composer_blocking_dialog_noted >/dev/null; then + printf 'unknown' + return 0 + fi case "$state" in pending|pending-unproven) ;; *) printf '%s' "$state"; return 0 ;; diff --git a/bin/fm-control.sh b/bin/fm-control.sh index aa4c9af2004..f61876bd0b9 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -200,6 +200,11 @@ control_cleanup() { && declare -F relaunch_rollback >/dev/null 2>&1; then relaunch_rollback || true fi + # Remove the dialog file while the lock is still held: once it is released, + # the next lifecycle command for this task writes the same path. + if [ -n "${FM_COMPOSER_DIALOG_SINK:-}" ]; then + rm -f "$FM_COMPOSER_DIALOG_SINK" + fi if [ "$CONTROL_LOCK_HELD" = 1 ]; then CONTROL_LOCK_HELD=0 fm_lock_release "$CONTROL_LOCK" || true @@ -313,6 +318,11 @@ trap control_cleanup EXIT fm_lock_try_acquire "$CONTROL_LOCK" \ || die "another lifecycle action is already running for task $ID" CONTROL_LOCK_HELD=1 +# do_exit runs in a command substitution. That subshell does not run this +# EXIT trap, so the parent has to hold the path the trap removes. Set it +# only once the lock is held: a process that loses the lock runs the same +# trap, and would remove the file the lock holder is reading. +FM_COMPOSER_DIALOG_SINK=$STATE/$ID.composer-dialog META="$STATE/$ID.meta" if [ ! -f "$META" ]; then case "$RAW_ID" in @@ -390,6 +400,13 @@ require_state_verified_backend() { # die "task $ID runs on the $BACKEND backend, which has no recovery-grade agent-state classifier, so '$1' cannot prove the agent actually stopped; refusing rather than reporting an unproven transition as done" } +# refuse_blocking_prompt: the screen is a dialog a confirming Enter would +# answer. Name it and stop. Do not type Escape or an option: both dismiss +# or choose. +refuse_blocking_prompt() { # + die "task $ID is blocked on a prompt: $1. Refusing to type Enter into it." +} + # rendered_matches : whether any row of the visible viewport matches. # An unreadable viewport is a no, so every caller treats it as missing proof. rendered_matches() { # @@ -557,7 +574,7 @@ retire_busy_incarnation() { # do_exit: stop the running agent, preserving endpoint and worktree. Prints # `already-stopped`, `endpoint-gone`, or `stopped`. do_exit() { - local state cmd hazard verdict composer_state cancel absence interrupt_result=not-needed + local state cmd hazard verdict composer_state cancel absence interrupt_result=not-needed dialog require_state_verified_backend exit state=$(agent_state) case "$state" in @@ -624,8 +641,16 @@ do_exit() { if [ -n "$hazard" ] && rendered_matches "$hazard"; then die "task $ID shows the $HARNESS revert picker, where typed text becomes a search and Enter reverts file changes; refusing to type the $cmd exit command. Close it with $(fm_control_interrupt_key "$HARNESS"), never Enter, then retry '$VERB'" fi + : > "$FM_COMPOSER_DIALOG_SINK" \ + || die "task $ID's dialog check could not be recorded" composer_state=$(fm_backend_composer_state "$BACKEND" "$T" "$LABEL" 2>/dev/null) \ || composer_state=unknown + # The classify that filled the sink ran in a subshell, so read the file + # rather than a function that subshell sourced. + if [ -s "${FM_COMPOSER_DIALOG_SINK:-}" ]; then + dialog=$(cat "$FM_COMPOSER_DIALOG_SINK") + refuse_blocking_prompt "$dialog" + fi case "$composer_state" in empty) ;; pending) @@ -645,7 +670,24 @@ do_exit() { || die "the exit command could not be sent to task $ID on $BACKEND" [ "$verdict" != send-failed ] \ || die "the exit command could not be sent to task $ID on $BACKEND" + # The submitting Enter can open the picker. The agent is still alive, and + # another Enter would confirm the selected row. A dead agent may leave the + # same text behind; that is not a prompt still waiting. + if [ -s "${FM_COMPOSER_DIALOG_SINK:-}" ]; then + dialog=$(cat "$FM_COMPOSER_DIALOG_SINK") + if [ "$(agent_state)" != dead ]; then + refuse_blocking_prompt "$dialog" + fi + fi state=$(wait_agent_state "$EXIT_WAIT" dead) || { + # A submit can return before any read sees the picker: a native busy + # verdict needs no composer read, and a cleared composer can be read + # before the picker renders. Read the screen once more here. + : > "$FM_COMPOSER_DIALOG_SINK" || true + fm_backend_composer_state "$BACKEND" "$T" "$LABEL" >/dev/null 2>&1 || true + if [ -s "$FM_COMPOSER_DIALOG_SINK" ]; then + refuse_blocking_prompt "$(cat "$FM_COMPOSER_DIALOG_SINK")" + fi die "exit-delivered $ID interrupt=$interrupt_result exit-command=delivered agent-state=$state exit=unconfirmed; the agent did not stop within ${EXIT_WAIT}s" } # The incarnation is over: retire its busy wiring so no stale record or diff --git a/bin/fm-tmux-lib.sh b/bin/fm-tmux-lib.sh index f031e65870b..f7cb21f2c84 100755 --- a/bin/fm-tmux-lib.sh +++ b/bin/fm-tmux-lib.sh @@ -250,6 +250,11 @@ fm_tmux_submit_enter_core() { # [baseline-idle tmux send-keys -t "$target" Enter 2>/dev/null || true sleep "$sleep_s" state=$(fm_tmux_composer_state "$target") + # The first Enter can open a picker. A later Enter would confirm it. + if fm_composer_blocking_dialog_noted >/dev/null; then + printf 'unknown' + return 0 + fi case "$state" in pending|pending-unproven) ;; unknown) diff --git a/docs/agent-control.md b/docs/agent-control.md index c949a13ba96..a543e881938 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -50,6 +50,9 @@ muse is the one verified adapter that restores the cancelled prompt back into it The clear is refused before anything is sent when the recorded backend cannot deliver it. `exit` reads the composer's state before typing the exit command and requires the exact `empty` verdict; a `pending` verdict refuses by naming the pending text, and any other verdict (`unknown`, `pending-unproven`, or an unreadable read) refuses as not proven empty, matching the fail-safe contract every other consumer that can overwrite composer input follows. +`exit` also refuses, naming the dialog as `blocked on a prompt`, when the screen shows a recognised dialog that a further Enter would answer, whether the dialog was open before the exit command was typed or the submitting Enter opened it; it sends no Escape and chooses no option, so closing the dialog is left to the operator. +A stopped agent whose pane still shows the dialog text is not refused. +[`fm_composer_blocking_dialog`](../bin/fm-composer-lib.sh) owns the recognised set, which today is only Claude's background-task exit picker; [its verification record](verification/runtime-backends.md#claude-background-task-exit-picker) lists the dialogs that are not covered. **Teardown and discard are not verbs and will not become verbs.** `exit` stops an agent and preserves everything else. diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index f45033ef120..ce429421552 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -1264,6 +1264,24 @@ FM_HERDR_SUBMIT_CONFIRM_LIVE=1 tests/fm-herdr-submit-confirm-live-e2e.test.sh ok - live Herdr submit confirm: Claude Code (2.1.283 (Claude Code)) on herdr 0.9.0 proves and submits a typed /exit behind its command popup ``` +### Claude background-task exit picker + +Measured 2026-10-05 against Claude Code 2.1.289 in an isolated tmux session. +The Herdr lab was not running, so the Herdr path is covered by the existing fakes. +Typing `/exit` while a background shell is still running opens a picker whose selected row is "Exit and stop tasks" and whose footer is "Enter to confirm · Esc to cancel". +That screen still classifies as pending, the same verdict as unsubmitted composer text. +A second Enter would confirm the selected row. +The picker is recognised by its recorded structure only: the heading on its own line, then the selected row alone on its row, with `Enter to confirm · Esc to cancel` as the last non-blank row. +The same strings quoted above a normal composer, as a diff, this note, or a test fixture shows them, are not a picker. +Submit retries now stop after the Enter that opened the picker and report unknown. +A typed submit to a pane that already shows the picker types nothing and sends no Enter. +Exit reports that the worker is blocked on the Claude background-task exit picker and does not type another Enter. +A submit can return before any read sees the picker, so exit reads the screen once more when its wait for the agent to stop times out, and names the picker there too. +Exit does not report a stopped agent whose pane still shows the picker text as blocked on a prompt. +The watcher does not read the picker: a pane parked on it keeps the ordinary stale triage. +No recorded screen was available for a model-downgrade confirmation, an MCP approval, or a Claude exit confirmation other than this picker, so those dialogs are not covered. +Refusing an Enter that would confirm a dialog restores an existing safety path, so it is not gated behind a flag. + ### Prune and respawn The real label-collision reproduction is owned by: diff --git a/tests/fm-backend-herdr.test.sh b/tests/fm-backend-herdr.test.sh index 0911e981bcc..01d0a7015f8 100755 --- a/tests/fm-backend-herdr.test.sh +++ b/tests/fm-backend-herdr.test.sh @@ -126,6 +126,14 @@ herdr_submit_claude_prefix() { # printf ' \xe2\x9d\xaf %s\n' "$text" > "$resp/4.out" } +# herdr_submit_preflight_prefix: fm_backend_send_text_submit reads the composer +# once before the adapter types. That read is call 1 and shows an empty +# composer, so every adapter call moves one slot later. +herdr_submit_preflight_prefix() { # + herdr_submit_shift "$1" 1 + printf ' \xe2\x9d\xaf\n' > "$1/1.out" +} + # make_herdr_server_env_fakebin: a stateful server stub that records only the # long-lived server launch environment, then reports the server as running. make_herdr_server_env_fakebin() { # -> echoes fakebin dir @@ -4431,6 +4439,67 @@ test_send_text_submit_popup_autocomplete_requires_second_enter() { pass "fm_backend_herdr_send_text_submit: a slash-command popup's placeholder fill on Enter #1 never flips agent_status to working, so it does not short-circuit as submitted; Enter #2 is retried and lands it" } +test_send_text_submit_refuses_confirming_enter_on_exit_picker() { + local dir log resp fb out enter_count + dir="$TMP_ROOT/submit-exit-picker"; mkdir -p "$dir/responses" "$dir/tmp"; log="$dir/log"; resp="$dir/responses"; : > "$log" + herdr_submit_claude_prefix "$resp" "/exit" + printf '{"result":{"agent":{"agent":"claude","agent_status":"idle"}}}\n' > "$resp/5.out" + printf '{"result":{"agent":{"agent":"claude","agent_status":"idle"}}}\n' > "$resp/7.out" + printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'The following will stop when you exit:' \ + 'shell · sleep 300' \ + ' 2. Move to background and exit' \ + ' 3. Stay' \ + 'Enter to confirm · Esc to cancel' > "$resp/8.out" + herdr_submit_preflight_prefix "$resp" + fb=$(make_herdr_fakebin "$dir") + out=$( PATH="$fb:$PATH" FM_HERDR_LOG="$log" FM_HERDR_RESPONSES="$resp" FM_BACKEND_HERDR_SUBMIT_POLLS=1 \ + TMPDIR="$dir/tmp" \ + bash -c '. "$0/bin/fm-backend.sh"; fm_backend_send_text_submit herdr default:w1:p2 "/exit" 3 0.01 0.01' "$ROOT" ) + enter_count=$(grep -c $'\x1f''pane'$'\x1f''send-keys'$'\x1f''w1:p2'$'\x1f''enter' "$log" || true) + [ "$out" = unknown ] || fail "the exit picker should stop the retry as unknown, got '$out'; log: $(cat "$log")" + [ "$enter_count" -eq 1 ] || fail "the exit picker should get one Enter, got $enter_count; log: $(cat "$log")" + [ -z "$(ls -A "$dir/tmp")" ] || fail "the submit left its dialog record behind: $(ls -A "$dir/tmp")" + pass "fm_backend_herdr_send_text_submit: the Claude background-task exit picker gets no confirming Enter" +} + +# Herdr can report `blocked` for a picker the submitting Enter opened. The +# submit then reports delivery with no composer read, so the picker is named +# only by the caller's next composer read, the one fm-control exit takes when +# its wait for the agent to stop times out. +test_blocked_submit_leaves_the_exit_picker_to_the_next_composer_read() { + local dir log resp fb out enter_count + dir="$TMP_ROOT/submit-blocked-picker"; mkdir -p "$dir/responses"; log="$dir/log"; resp="$dir/responses"; : > "$log" + herdr_submit_claude_prefix "$resp" "/exit" + printf '{"result":{"agent":{"agent":"claude","agent_status":"idle"}}}\n' > "$resp/5.out" + printf '{"result":{"agent":{"agent":"claude","agent_status":"blocked"}}}\n' > "$resp/7.out" + printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'The following will stop when you exit:' \ + 'shell · sleep 300' \ + ' 2. Move to background and exit' \ + ' 3. Stay' \ + 'Enter to confirm · Esc to cancel' > "$resp/8.out" + herdr_submit_preflight_prefix "$resp" + fb=$(make_herdr_fakebin "$dir") + out=$( PATH="$fb:$PATH" FM_HERDR_LOG="$log" FM_HERDR_RESPONSES="$resp" FM_BACKEND_HERDR_SUBMIT_POLLS=1 \ + FM_COMPOSER_DIALOG_SINK="$dir/sink" \ + bash -c '. "$0/bin/fm-backend.sh" + verdict=$(fm_backend_send_text_submit herdr default:w1:p2 "/exit" 3 0.01 0.01) + printf "%s|%s|" "$verdict" "$(cat "$FM_COMPOSER_DIALOG_SINK")" + fm_backend_composer_state herdr default:w1:p2 >/dev/null + cat "$FM_COMPOSER_DIALOG_SINK"' "$ROOT" ) + enter_count=$(grep -c $'\x1f''pane'$'\x1f''send-keys'$'\x1f''w1:p2'$'\x1f''enter' "$log" || true) + [ "$out" = 'empty||Claude background-task exit picker' ] \ + || fail "a blocked submit should report delivery unnamed and the next composer read should name the picker, got '$out'; log: $(cat "$log")" + [ "$enter_count" -eq 1 ] || fail "a blocked submit should send one Enter, got $enter_count; log: $(cat "$log")" + [ -f "$dir/sink" ] || fail "a submit must not remove a dialog record its caller owns" + pass "fm_backend_send_text_submit (herdr): a picker behind a blocked verdict is named by the caller's next composer read" +} + test_send_text_submit_confirms_blocked_after_enter() { local dir log resp fb out enter_count dir="$TMP_ROOT/submit-blocked"; mkdir -p "$dir/responses"; log="$dir/log"; resp="$dir/responses"; : > "$log" @@ -5954,6 +6023,8 @@ test_send_text_submit_detects_landed_send test_send_text_submit_detects_swallowed_enter test_send_text_submit_replays_literal_send_stderr test_send_text_submit_popup_autocomplete_requires_second_enter +test_send_text_submit_refuses_confirming_enter_on_exit_picker +test_blocked_submit_leaves_the_exit_picker_to_the_next_composer_read test_send_text_submit_confirms_blocked_after_enter test_send_text_submit_preexisting_working_pending_is_queued_enter test_send_text_submit_preexisting_working_does_not_confirm_failed_enter diff --git a/tests/fm-composer-lib.test.sh b/tests/fm-composer-lib.test.sh index 4dbddf14b82..83ce4599472 100755 --- a/tests/fm-composer-lib.test.sh +++ b/tests/fm-composer-lib.test.sh @@ -1080,3 +1080,158 @@ test_queued_enter_verdict_does_not_convert_other_states() { test_queued_enter_verdict_busy_pending_is_empty test_queued_enter_verdict_idle_pending_stays_pending test_queued_enter_verdict_does_not_convert_other_states + +# The selected row sits on cursor row 1 so a tmux read whose cursor is that +# row, and a cursorless read, both still see unsubmitted text. +exit_picker_screen() { + printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'The following will stop when you exit:' \ + 'shell · sleep 300' \ + ' 2. Move to background and exit' \ + ' 3. Stay' \ + 'Enter to confirm · Esc to cancel' +} + +fm_test_picker_send() { + printf 'Enter\n' >> "$FM_TEST_PICKER_ENTERS" +} + +fm_test_picker_state() { + fm_composer_classify_screen 'styled=1' "$FM_TEST_PICKER_SCREEN" 1 +} + +test_background_exit_picker_stays_pending_and_blocks_retry() { + local screen out rc sink enters + screen=$(exit_picker_screen) + out=$(fm_composer_blocking_dialog "$screen"); rc=$? + [ "$rc" -eq 0 ] || fail "the recorded picker should match" + [ "$out" = 'Claude background-task exit picker' ] || fail "dialog name was '$out'" + out=$(fm_composer_blocking_dialog 'Background work is running'); rc=$? + [ "$rc" -eq 1 ] || fail "a heading alone must not match" + [ -z "$out" ] || fail "a miss must print nothing, got '$out'" + out=$(fm_composer_blocking_dialog "$(printf '%s\n' 'Background work is running' 'Exit and stop tasks')"); rc=$? + [ "$rc" -eq 1 ] || fail "two of the three strings must not match" + out=$(fm_composer_blocking_dialog "$(printf '%s\n' "$screen" '' '')"); rc=$? + [ "$rc" -eq 0 ] || fail "blank rows below the footer should still match" + sink=$(mktemp) + FM_COMPOSER_DIALOG_SINK=$sink + out=$(fm_composer_classify_screen 'styled=1' "$screen" 1) + [ "$out" = pending ] || fail "cursor on the selected row should stay pending, got '$out'" + [ "$(cat "$sink")" = 'Claude background-task exit picker' ] || fail "classify should note the dialog, got '$(cat "$sink")'" + out=$(fm_composer_classify_screen 'styled=1' "$screen") + [ "$out" = pending ] || fail "a styled cursorless picker should stay pending, got '$out'" + unset FM_COMPOSER_DIALOG_SINK + rm -f "$sink" + FM_TEST_PICKER_SCREEN=$screen + FM_TEST_PICKER_ENTERS=$(mktemp) + : > "$FM_TEST_PICKER_ENTERS" + fm_composer_dialog_sink_prepare || fail "the dialog sink could not be prepared" + sink=$FM_COMPOSER_DIALOG_SINK + out=$(fm_composer_submit_retry_core fm_test_picker_send fm_test_picker_state win 3 0) + fm_composer_dialog_sink_release + [ ! -e "$sink" ] || fail "the release should remove a sink that prepare created" + [ -z "${FM_COMPOSER_DIALOG_SINK:-}" ] || fail "the release should unset a sink that prepare created" + enters=$(grep -c '^Enter$' "$FM_TEST_PICKER_ENTERS" || true) + [ "$out" = unknown ] || fail "a picker must stop the retry as unknown, got '$out'" + [ "$enters" -eq 1 ] || fail "a picker must receive one Enter, got $enters" + rm -f "$FM_TEST_PICKER_ENTERS" + unset FM_TEST_PICKER_SCREEN FM_TEST_PICKER_ENTERS + pass "the Claude background-task exit picker stays pending and receives no confirming Enter" +} + +# The picker's own text, shown the way a worker pane shows it when it prints +# this repository's diff, verification note, or a test fixture: quoted above a +# normal composer. No picker is open, so the next Enter confirms nothing. +quoted_exit_picker_screen() { + printf '%s\n' \ + '● Here is the fixture the test uses:' \ + "+ 'Background work is running' \\" \ + "+ '❯ 1. Exit and stop tasks' \\" \ + "+ 'Enter to confirm · Esc to cancel'" \ + ' The selected row is "❯ 1. Exit and stop tasks" and the footer is "Enter to confirm · Esc to cancel".' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'Enter to confirm · Esc to cancel' \ + '' \ + '╭──────────────╮' \ + '│ > next steer │' \ + '╰──────────────╯' +} + +test_dialog_heading_and_footer_must_be_the_recorded_lines() { + local screen out rc + screen=$(printf '%s\n' \ + 'The fixture mentions Background work is running in a sentence' \ + '❯ 1. Exit and stop tasks' \ + 'Enter to confirm · Esc to cancel') + out=$(fm_composer_blocking_dialog "$screen"); rc=$? + [ "$rc" -eq 1 ] || fail "a heading buried in a sentence must not match" + [ -z "$out" ] || fail "a miss must print nothing, got '$out'" + screen=$(printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'Enter to confirm the deployment') + out=$(fm_composer_blocking_dialog "$screen"); rc=$? + [ "$rc" -eq 1 ] || fail "a last line that only starts with the confirm words must not match" + [ -z "$out" ] || fail "a miss must print nothing, got '$out'" + pass "a buried heading or a different last line is not the exit picker" +} + +test_dialog_note_skips_the_match_when_no_sink_is_set() { + local screen out rc before after + screen=$(exit_picker_screen) + unset FM_COMPOSER_DIALOG_SINK + out=$(fm_composer_note_blocking_dialog "$screen"); rc=$? + [ "$rc" -eq 1 ] || fail "a note without a sink should return 1, got $rc" + [ -z "$out" ] || fail "a note without a sink should print nothing, got '$out'" + [ -z "${FM_COMPOSER_DIALOG_SINK:-}" ] || fail "a note without a sink must not create one" + out=$(fm_composer_classify_screen 'styled=1' "$screen" 1) + [ "$out" = pending ] || fail "classify without a sink should stay pending, got '$out'" + trap 'true' RETURN + before=$(trap -p RETURN) + fm_composer_dialog_sink_prepare || fail "the dialog sink could not be prepared" + fm_composer_dialog_sink_release + after=$(trap -p RETURN) + trap - RETURN + [ "$before" = "$after" ] || fail "release replaced the caller RETURN trap: $after" + pass "a dialog note without a sink skips the match, and release leaves a caller RETURN trap" +} + +test_quoted_exit_picker_text_is_not_a_dialog() { + local screen out rc sink enters + screen=$(quoted_exit_picker_screen) + out=$(fm_composer_blocking_dialog "$screen"); rc=$? + [ "$rc" -eq 1 ] || fail "picker text quoted above a normal composer must not match" + [ -z "$out" ] || fail "a miss must print nothing, got '$out'" + out=$(fm_composer_blocking_dialog "$(printf '%s\n' \ + 'Background work is running' \ + "+ '❯ 1. Exit and stop tasks' \\" \ + 'Enter to confirm · Esc to cancel')"); rc=$? + [ "$rc" -eq 1 ] || fail "a selected row that is not alone on its row must not match" + out=$(fm_composer_blocking_dialog "$(printf '%s\n' \ + '❯ 1. Exit and stop tasks' \ + 'Background work is running' \ + 'Enter to confirm · Esc to cancel')"); rc=$? + [ "$rc" -eq 1 ] || fail "a selected row above the heading must not match" + FM_TEST_PICKER_SCREEN=$screen + FM_TEST_PICKER_ENTERS=$(mktemp) + : > "$FM_TEST_PICKER_ENTERS" + fm_composer_dialog_sink_prepare || fail "the dialog sink could not be prepared" + sink=$FM_COMPOSER_DIALOG_SINK + out=$(fm_composer_submit_retry_core fm_test_picker_send fm_test_picker_state win 3 0) + [ ! -s "$sink" ] || fail "quoted picker text must not be noted as a dialog, got '$(cat "$sink")'" + fm_composer_dialog_sink_release + enters=$(grep -c '^Enter$' "$FM_TEST_PICKER_ENTERS" || true) + [ "$out" = pending ] || fail "quoted picker text must keep the ordinary pending verdict, got '$out'" + [ "$enters" -eq 3 ] || fail "quoted picker text must keep the ordinary Enter retries, got $enters" + rm -f "$FM_TEST_PICKER_ENTERS" + unset FM_TEST_PICKER_SCREEN FM_TEST_PICKER_ENTERS + pass "picker text quoted above a normal composer is not read as a live picker" +} + +test_background_exit_picker_stays_pending_and_blocks_retry +test_dialog_heading_and_footer_must_be_the_recorded_lines +test_dialog_note_skips_the_match_when_no_sink_is_set +test_quoted_exit_picker_text_is_not_a_dialog diff --git a/tests/fm-control-relaunch.test.sh b/tests/fm-control-relaunch.test.sh index 1242dc26bfe..a214f057b7e 100755 --- a/tests/fm-control-relaunch.test.sh +++ b/tests/fm-control-relaunch.test.sh @@ -1627,6 +1627,7 @@ test_concurrent_relaunch_is_refused() { i=$((i + 1)) done [ -e "$lock" ] || { kill "$holder" 2>/dev/null; fail "could not stage a held control lock"; } + printf 'held\n' > "$dir/home/state/rl19.composer-dialog" out=$(run_control "$dir" rl19 relaunch --note "concurrent"); rc=$? kill "$holder" 2>/dev/null || true wait "$holder" 2>/dev/null || true @@ -1635,6 +1636,8 @@ test_concurrent_relaunch_is_refused() { "the refusal should name the concurrent action" [ "$(cat "$dir/fake/command")" = claude ] \ || fail "a refused concurrent relaunch must not stop the agent" + [ "$(cat "$dir/home/state/rl19.composer-dialog" 2>/dev/null)" = held ] \ + || fail "a refused concurrent relaunch must not remove the lock holder's dialog file" pass "fm-control relaunch: two control actions on one task serialize instead of interleaving" } @@ -2386,6 +2389,57 @@ test_relaunch_moves_a_drifted_item_back_in_flight() { pass "relaunch heals an item that drifted out of In flight while the task stayed live" } +test_exit_and_relaunch_remove_the_dialog_file() { + local dir out rc + dir=$(new_case dialog-file-exit rl70) + add_ship_task "$dir" rl70 claude + out=$(run_control "$dir" rl70 exit); rc=$? + expect_code 0 "$rc" "exit should stop the agent"$'\n'"$out" + [ ! -e "$dir/home/state/rl70.composer-dialog" ] \ + || fail "exit should remove the dialog file" + + dir=$(new_case dialog-file-relaunch rl71) + add_ship_task "$dir" rl71 claude + out=$(run_control "$dir" rl71 relaunch --note "replace the agent"); rc=$? + expect_code 0 "$rc" "relaunch should replace the agent"$'\n'"$out" + [ ! -e "$dir/home/state/rl71.composer-dialog" ] \ + || fail "relaunch should remove the dialog file" + pass "fm-control removes the dialog file after exit and after relaunch" +} + +# The lock release removes paths at or under the control lock with rm, so a +# recording rm sees the state directory at the moment of release without a +# second overlapping command. +test_exit_removes_the_dialog_file_before_releasing_the_lock() { + local dir out rc lock sink trace + dir=$(new_case dialog-file-order rl72) + add_ship_task "$dir" rl72 claude + lock="$dir/home/state/.control-rl72.lock" + sink="$dir/home/state/rl72.composer-dialog" + trace="$dir/fake/rm-trace" + cat > "$dir/fakebin/rm" <> "$trace" + break + ;; + esac +done +exec "$(command -v rm)" "\$@" +SH + chmod +x "$dir/fakebin/rm" + out=$(run_control "$dir" rl72 exit); rc=$? + expect_code 0 "$rc" "exit should stop the agent"$'\n'"$out" + [ ! -e "$lock" ] || fail "exit should release the control lock" + [ "$(tail -n 1 "$trace" 2>/dev/null)" = absent ] \ + || fail "the dialog file must be gone when the control lock is released, got: $(cat "$trace" 2>/dev/null)" + pass "fm-control exit removes the dialog file before it releases the control lock" +} + +test_exit_and_relaunch_remove_the_dialog_file +test_exit_removes_the_dialog_file_before_releasing_the_lock test_same_harness_relaunch_keeps_identity_and_reuses_the_endpoint test_relaunch_refuses_before_exit_when_the_composer_holds_pending_text test_relaunch_refuses_before_exit_when_the_composer_state_is_unproven diff --git a/tests/fm-control.test.sh b/tests/fm-control.test.sh index 832c3fd7a49..f34fb8acb91 100755 --- a/tests/fm-control.test.sh +++ b/tests/fm-control.test.sh @@ -175,6 +175,18 @@ case "${1:-}" in done printf 'fakepane\n'; exit 0 ;; capture-pane) + if [ -f "$D/after-enter" ] && [ -f "$D/keys" ] && grep -qx Enter "$D/keys"; then + # after-enter-late holds how many captures after Enter still show the + # ordinary pane, for a screen that renders after the submit has read it. + late=0 + [ ! -f "$D/after-enter-late" ] || late=$(cat "$D/after-enter-late") + if [ "$late" -gt 0 ]; then + printf '%s' "$((late - 1))" > "$D/after-enter-late" + else + cat "$D/after-enter" + exit 0 + fi + fi if [ -f "$D/devin" ]; then devin_screen "$(cat "$D/devin")"; elif [ -f "$D/pane" ]; then cat "$D/pane"; else printf '╭────╮\n│ │\n╰────╯\n'; fi exit 0 ;; list-windows) @@ -838,6 +850,77 @@ test_busy_agent_is_interrupted_before_the_exit_command() { pass "fm-control exit: a busy agent receives interrupt delivery before the exit command" } +exit_picker_screen() { + printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'The following will stop when you exit:' \ + 'shell · sleep 300' \ + ' 2. Move to background and exit' \ + ' 3. Stay' \ + 'Enter to confirm · Esc to cancel' +} + +test_exit_refuses_an_open_background_picker() { + local dir out rc + dir=$(new_case open-picker) + add_task "$dir" t1 claude + alive_as "$dir" claude + exit_picker_screen > "$dir/fake/pane" + out=$(run_control "$dir" t1 exit); rc=$? + expect_code 1 "$rc" "an open exit picker should refuse"$'\n'"$out" + assert_contains "$out" "blocked on a prompt: Claude background-task exit picker" \ + "the refusal should name the dialog" + assert_not_contains "$out" "Esc" "the refusal must not name a dismissal key" + [ ! -s "$dir/fake/literal" ] || fail "an open picker must not be typed into" + [ ! -s "$dir/fake/keys" ] || fail "an open picker must receive no keys" + pass "fm-control exit: an already-open background-task picker is not typed into" +} + +test_exit_refuses_the_confirming_enter() { + local dir out rc enters + dir=$(new_case confirm-picker) + add_task "$dir" t1 claude + alive_as "$dir" claude + exit_picker_screen > "$dir/fake/after-enter" + out=$(env FM_FAKE_NEVER_DIES=1 PATH="$dir/fakebin:$PATH" FM_HOME="$dir/home" \ + FM_FAKE_DIR="$dir/fake" FM_CONTROL_POLL=0.01 FM_CONTROL_EXIT_WAIT=0.05 \ + "$CONTROL" t1 exit 2>&1); rc=$? + expect_code 1 "$rc" "the confirming Enter should refuse"$'\n'"$out" + assert_contains "$out" "blocked on a prompt: Claude background-task exit picker" \ + "the refusal should name the dialog" + assert_not_contains "$out" "Esc" "the refusal must not name a dismissal key" + [ "$(literals "$dir")" = /exit ] || fail "the exit command should still be typed, got '$(literals "$dir")'" + enters=$(grep -c '^Enter$' "$dir/fake/keys" || true) + [ "$enters" -eq 1 ] || fail "only the submitting Enter should be sent, got $enters" + pass "fm-control exit: the Enter that opens the background-task picker is not followed by a confirming Enter" +} + +# The submit reads a cleared composer before the picker renders, so it reports +# delivery and no read inside it sees the picker. Exit's own read after the +# stop wait times out must still name the dialog. +test_exit_names_a_picker_that_renders_after_the_submit() { + local dir out rc enters + dir=$(new_case late-picker) + add_task "$dir" t1 claude + alive_as "$dir" claude + exit_picker_screen > "$dir/fake/after-enter" + printf '1' > "$dir/fake/after-enter-late" + out=$(env FM_FAKE_NEVER_DIES=1 PATH="$dir/fakebin:$PATH" FM_HOME="$dir/home" \ + FM_FAKE_DIR="$dir/fake" FM_CONTROL_POLL=0.01 FM_CONTROL_EXIT_WAIT=0.05 \ + "$CONTROL" t1 exit 2>&1); rc=$? + expect_code 1 "$rc" "a picker that renders after the submit should refuse"$'\n'"$out" + [ "$(cat "$dir/fake/after-enter-late")" = 0 ] \ + || fail "the submit should have read the ordinary pane once after Enter" + assert_contains "$out" "blocked on a prompt: Claude background-task exit picker" \ + "the refusal should name the dialog" + assert_not_contains "$out" "did not stop within" \ + "a recognised picker must not fall back to the generic timeout message" + enters=$(grep -c '^Enter$' "$dir/fake/keys" || true) + [ "$enters" -eq 1 ] || fail "only the submitting Enter should be sent, got $enters" + pass "fm-control exit: a picker that renders after the submit returned is named when the stop wait times out" +} + test_idle_agent_is_not_interrupted() { local dir out rc gen dir=$(new_case idle) @@ -1100,6 +1183,9 @@ test_interrupt_refuses_when_no_agent_runs test_ambiguous_endpoint_refuses test_busy_agent_is_interrupted_before_the_exit_command test_idle_agent_is_not_interrupted +test_exit_refuses_an_open_background_picker +test_exit_refuses_the_confirming_enter +test_exit_names_a_picker_that_renders_after_the_submit test_interrupt_without_acknowledgement_preserves_busy_state test_muse_interrupt_confirms_adapter_acknowledgement test_interrupt_revalidates_agent_after_acknowledgement_wait diff --git a/tests/fm-tmux-submit-busy.test.sh b/tests/fm-tmux-submit-busy.test.sh index 5290aaaea97..058c9f7ebb5 100755 --- a/tests/fm-tmux-submit-busy.test.sh +++ b/tests/fm-tmux-submit-busy.test.sh @@ -44,7 +44,14 @@ case "${1:-}" in send-keys) shift; is_enter=0 while [ "$#" -gt 0 ]; do - case "$1" in -t) shift ;; -l) ;; Enter) is_enter=1 ;; esac; shift + case "$1" in + -t) shift ;; + -l) ;; + Enter) is_enter=1 ;; + -*) ;; + *) [ -z "${FM_FAKE_SENT:-}" ] || printf 'typed %s\n' "$1" >> "$FM_FAKE_SENT" ;; + esac + shift done if [ "$is_enter" = 1 ]; then [ -z "${FM_FAKE_SENT:-}" ] || printf 'Enter\n' >> "$FM_FAKE_SENT" @@ -344,6 +351,67 @@ test_claude_busy_signature_uses_real_capture_shapes() { test_busy_pane_pending_returns_empty test_idle_pane_pending_returns_pending + +test_exit_picker_refuses_confirming_enter() { + local dir fakebin composer sent vfile enters + dir="$TMP_ROOT/exit-picker" + fakebin=$(make_submit_mock "$dir") + composer="$dir/composer" + sent="$dir/sent.log" + vfile="$dir/verdict" + printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'The following will stop when you exit:' \ + 'shell · sleep 300' \ + ' 2. Move to background and exit' \ + ' 3. Stay' \ + 'Enter to confirm · Esc to cancel' > "$composer" + : > "$sent" + touch "$dir/.swallow" + fm_composer_dialog_sink_prepare || fail "the dialog sink could not be prepared" + PATH="$fakebin:$PATH" FM_FAKE_COMPOSER="$composer" FM_FAKE_SENT="$sent" \ + FM_FAKE_SWALLOW="$dir/.swallow" FM_FAKE_PERSIST_SWALLOW=1 FM_FAKE_PANE_BUSY=0 \ + fm_tmux_submit_enter_core "win" 3 0 > "$vfile" 2>/dev/null + fm_composer_dialog_sink_release + [ "$(cat "$vfile")" = unknown ] || fail "exit picker should return unknown, got '$(cat "$vfile")'" + enters=$(grep -c '^Enter$' "$sent" || true) + [ "$enters" -eq 1 ] || fail "exit picker should get one Enter, got $enters" + pass "fm_tmux_submit_enter_core: the Claude background-task exit picker gets no confirming Enter" +} + +test_exit_picker_refuses_confirming_enter + +test_typed_submit_on_open_exit_picker_types_nothing() { + local dir fakebin composer sent before rc=0 err + # shellcheck source=/dev/null + . "$ROOT/bin/fm-backend.sh" + dir="$TMP_ROOT/typed-on-picker" + fakebin=$(make_submit_mock "$dir") + composer="$dir/composer" + sent="$dir/sent.log" + err="$dir/err" + printf '%s\n' \ + 'Background work is running' \ + '❯ 1. Exit and stop tasks' \ + 'The following will stop when you exit:' \ + 'shell · sleep 300' \ + ' 2. Move to background and exit' \ + ' 3. Stay' \ + 'Enter to confirm · Esc to cancel' > "$composer" + before=$(cat "$composer") + : > "$sent" + PATH="$fakebin:$PATH" FM_FAKE_COMPOSER="$composer" FM_FAKE_SENT="$sent" \ + fm_backend_send_text_submit tmux win 'please continue' 3 0 0 >"$dir/out" 2>"$err" || rc=$? + [ "$rc" -ne 0 ] || fail "a typed submit onto an open picker should refuse" + [ ! -s "$sent" ] || fail "a typed submit onto an open picker sent input: $(cat "$sent")" + [ "$(cat "$composer")" = "$before" ] || fail "a typed submit onto an open picker changed the pane" + grep -F 'blocked on a prompt: Claude background-task exit picker' "$err" >/dev/null \ + || fail "the refusal should name the picker, got '$(cat "$err")'" + pass "fm_backend_send_text_submit: a typed message to the exit picker types nothing and sends no Enter" +} + +test_typed_submit_on_open_exit_picker_types_nothing test_wrapped_continuation_retries_swallowed_enter test_placeholder_like_bare_input_retries_swallowed_enter test_busy_pane_composer_clears_first_try From 5838f105008cb24bc999a819d3c5b50c2fd5675d Mon Sep 17 00:00:00 2001 From: M00NLIG7 <57321738+M00NLIG7@users.noreply.github.com> Date: Mon, 5 Oct 2026 20:29:16 -0600 Subject: [PATCH 08/13] fix: restore timeout watchdog compatibility with macOS Bash 3.2 (#6028) * fix: support stock macOS Bash in timeout watchdog * no-mistakes(ci): Fixed ci-1 in tests/fm-timeout-lib.test.sh: both startup-owner failure paths now kill the bounded command group before killing the watchdog. Fault injection reproduced both leaks before the fix and confirmed cleanup afterward. Bash 3.2 suite, syntax check, ShellCheck, and diff checks passed; GNU timeout coverage skipped because the binary is unavailable. Production code unchanged --- bin/fm-timeout-lib.sh | 8 ++-- tests/fm-timeout-lib.test.sh | 77 ++++++++++++++++++++++++++++++++++-- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/bin/fm-timeout-lib.sh b/bin/fm-timeout-lib.sh index a785ad8b793..18a793a87ac 100644 --- a/bin/fm-timeout-lib.sh +++ b/bin/fm-timeout-lib.sh @@ -221,11 +221,13 @@ fm_exec_timed() { # exit 125 fi owner=${FM_EXEC_TIMED_OWNER_PID:-$$} - [ "$owner" != "$BASHPID" ] || owner=$PPID unset FM_EXEC_TIMED_OWNER_PID if command -v perl >/dev/null 2>&1; then exec perl -MPOSIX=WNOHANG,setpgid -MTime::HiRes=time -e ' - my ($bound, $grace, $owner) = (shift, shift, shift); + my ($bound, $grace, $owner, $shell_parent) = (shift, shift, shift, shift); + # exec preserves the shell PID, including in Bash 3.2 subshells where + # BASHPID is unavailable. Keep the pre-exec parent for startup races. + $owner = $shell_parent if $owner == $$; my $parent = getppid(); my ($pid, $pending, $kill_at, $timed_out) = (0, "", 0, 0); for my $sig (qw(TERM INT HUP)) { @@ -272,7 +274,7 @@ fm_exec_timed() { # } select undef, undef, undef, 0.05; } - ' -- "$seconds" "$grace" "$owner" "$@" + ' -- "$seconds" "$grace" "$owner" "$PPID" "$@" elif command -v timeout >/dev/null 2>&1; then exec timeout -k "$grace" "$seconds" "$@" elif command -v gtimeout >/dev/null 2>&1; then diff --git a/tests/fm-timeout-lib.test.sh b/tests/fm-timeout-lib.test.sh index 56bcc6dfc5a..621367e558b 100755 --- a/tests/fm-timeout-lib.test.sh +++ b/tests/fm-timeout-lib.test.sh @@ -64,6 +64,35 @@ test_passes_the_command_status_and_output_through() { pass "fm_exec_timed passes a command's status and output through unchanged" } +# Exercise the installed system Bash explicitly, including stock macOS 3.2: +# neither a top-level call nor a subshell may abort before starting its command. +# Unset BASHPID so newer system Bash also covers the missing-variable case. +test_system_bash_preserves_completion_and_signal_statuses() { + local mode status command out rc + for mode in top-level subshell; do + for status in 0 7 137 143; do + case "$status" in + 137) command='echo ran; kill -KILL $$' ;; + 143) command='echo ran; kill -TERM $$' ;; + *) command="echo ran; exit $status" ;; + esac + rc=0 + out=$(PATH=$PERL_ONLY /bin/bash -c ' + . "$1/bin/fm-timeout-lib.sh" + unset BASHPID + if [ "$2" = subshell ]; then + ( fm_exec_timed 5 1 bash -c "$3" ) + else + fm_exec_timed 5 1 bash -c "$3" + fi + ' _ "$ROOT" "$mode" "$command" 2>&1) || rc=$? + [ "$rc" -eq "$status" ] || fail "system Bash $mode lost command status $status (rc=$rc: $out)" + [ "$out" = ran ] || fail "system Bash $mode did not run the command cleanly: $out" + done + done + pass "system Bash top-level and subshell calls preserve success, failure, and signal status" +} + # A command that honors TERM ends at the bound, long before the grace would # have forced it, and is gone afterwards. test_term_ends_a_cooperative_command_at_the_bound() { @@ -109,7 +138,7 @@ test_the_bound_replaces_the_calling_shell() { rm -f "$dir/caller" "$dir/parent" ( . "$ROOT/bin/fm-timeout-lib.sh" - printf '%s\n' "$BASHPID" > "$dir/caller" + perl -e 'print getppid(), "\n"' > "$dir/caller" PATH=$path fm_exec_timed 5 1 bash -c 'echo "$PPID" > "$1"' _ "$dir/parent" ) || fail "the bounded probe failed under PATH=$path" caller=$(cat "$dir/caller") @@ -204,16 +233,16 @@ test_a_named_owner_that_is_gone_ends_the_command() { # fm_exec_timed - the watchdog then starts already reparented - is still # detected instead of leaving the command running to its bound. test_an_owner_that_dies_during_startup_ends_the_command() { - local dir watchdog started + local dir watchdog started pid dir="$TMP_ROOT/startup-owner" mkdir -p "$dir" # shellcheck disable=SC2016 PATH=$PERL_ONLY bash -c ' . "$1/bin/fm-timeout-lib.sh" ( - echo "$BASHPID" > "$2/watchdog" + perl -e "print getppid(), qq(\\n)" > "$2/watchdog" while kill -0 "$$" 2>/dev/null; do sleep 0.05; done - fm_exec_timed 60 1 bash -c "exec sleep 300" + fm_exec_timed 60 1 bash -c "echo \$\$ > \"\$1\"; exec sleep 300" _ "$2/pid" ) >/dev/null 2>&1 & exit 0 ' _ "$ROOT" "$dir" @@ -222,6 +251,10 @@ test_an_owner_that_dies_during_startup_ends_the_command() { started=$SECONDS while kill -0 "$watchdog" 2>/dev/null; do if [ "$((SECONDS - started))" -ge 15 ]; then + if [ -s "$dir/pid" ]; then + pid=$(cat "$dir/pid") + kill -KILL -- "-$pid" 2>/dev/null || true + fi kill -KILL "$watchdog" 2>/dev/null || true fail "a watchdog whose owner died during startup ran on toward its bound" fi @@ -230,6 +263,40 @@ test_an_owner_that_dies_during_startup_ends_the_command() { pass "fm_exec_timed ends the command when its owner dies during watchdog startup" } +# A top-level calling shell has its own PID in $$, unlike a Bash subshell. +# Capture its parent before exec: that parent can exit while the top-level +# shell is still on its way into the watchdog. +test_a_top_level_parent_that_dies_during_startup_ends_the_command() { + local dir watchdog started pid + dir="$TMP_ROOT/top-level-parent" + mkdir -p "$dir" + PATH=$PERL_ONLY bash -c ' + bash -c '\'' + . "$1/bin/fm-timeout-lib.sh" + echo "$$" > "$2/watchdog" + while kill -0 "$PPID" 2>/dev/null; do sleep 0.05; done + fm_exec_timed 60 1 bash -c "echo \$\$ > \"\$1\"; exec sleep 300" _ "$2/pid" + '\'' _ "$1" "$2" >/dev/null 2>&1 & + while [ ! -s "$2/watchdog" ]; do sleep 0.02; done + exit 0 + ' _ "$ROOT" "$dir" + wait_for_file "$dir/watchdog" + watchdog=$(cat "$dir/watchdog") + started=$SECONDS + while kill -0 "$watchdog" 2>/dev/null; do + if [ "$((SECONDS - started))" -ge 15 ]; then + if [ -s "$dir/pid" ]; then + pid=$(cat "$dir/pid") + kill -KILL -- "-$pid" 2>/dev/null || true + fi + kill -KILL "$watchdog" 2>/dev/null || true + fail "top-level watchdog lost its pre-exec parent and ran toward its bound" + fi + sleep 0.02 + done + pass "fm_exec_timed preserves a top-level shell's parent across exec startup" +} + # perl is preferred whenever it exists, because only its watchdog can reap a # leftover descendant after replacing the caller. test_perl_is_preferred_over_timeout() { @@ -328,6 +395,7 @@ test_run_timed_passes_a_natural_exit_through_a_fired_bound() { } test_passes_the_command_status_and_output_through +test_system_bash_preserves_completion_and_signal_statuses test_run_timed_reports_the_bound_when_the_wrapper_records_a_signal_death test_run_timed_passes_a_natural_exit_through_a_fired_bound test_term_ends_a_cooperative_command_at_the_bound @@ -337,6 +405,7 @@ test_a_descendant_holding_the_output_cannot_outlast_the_bound test_a_signal_to_the_bounding_process_reaches_the_command test_a_named_owner_that_is_gone_ends_the_command test_an_owner_that_dies_during_startup_ends_the_command +test_a_top_level_parent_that_dies_during_startup_ends_the_command test_perl_is_preferred_over_timeout test_refuses_rather_than_running_unbounded test_rejects_malformed_bounds_before_running_anything From e06a46fec726071618da0460f0de552d4d078ef1 Mon Sep 17 00:00:00 2001 From: Symphony Date: Tue, 6 Oct 2026 21:56:41 +0700 Subject: [PATCH 09/13] fix(project-management): use subshell form for Initialize command (#6699) Wrap the cd command in a subshell to comply with the cd-guard policy that blocks persistent top-level directory changes in the primary firstmate checkout. The subshell form (cd projects/ && ...) is accepted by the policy as documented in issue #6502. Fixes #6502 --- .agents/skills/project-management/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.agents/skills/project-management/SKILL.md b/.agents/skills/project-management/SKILL.md index d68d5195330..6c39cc871ed 100644 --- a/.agents/skills/project-management/SKILL.md +++ b/.agents/skills/project-management/SKILL.md @@ -84,7 +84,7 @@ The captain's request to create that local project authorizes this local initial Run no-mistakes initialization only for `no-mistakes` and `no-mistakes-prod-only` projects: ```sh -cd projects/ && no-mistakes init && no-mistakes doctor +(cd projects/ && no-mistakes init && no-mistakes doctor) ``` Initialization configures the local gate and does not vendor a no-mistakes skill into the project. From 99da16d954ccee7e0dbf594d88707e6871fd0929 Mon Sep 17 00:00:00 2001 From: Freudator86 <94322668+Freudator86@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:36:09 +0200 Subject: [PATCH 10/13] feat(bin): add armable daily startup growth check (#6725) * Add daily startup growth check * no-mistakes(review): watch printed startup memory, retain growth baselines, drop knobs * no-mistakes(review): delegate budget verdict, report before publish, pin shim home * no-mistakes(review): baseline first sightings silently, drop mtime, spare secondmates * no-mistakes(review): cap the wake line, validate budget verdict fields * no-mistakes(review): guard record schema, check appends, tighten assertions * no-mistakes(review): drop arbitrary tracked library, make re-arm idempotent * no-mistakes(review): correct tracked-set wording, restore shim guards, register artifacts * no-mistakes(review): exit on signal instead of publishing partial record * no-mistakes(document): correct startup-growth record removal cost in state registry * no-mistakes(test): sweep orphaned empty startup-growth temp records on due evaluation * no-mistakes(document): note watcher need for armed startup growth check * no-mistakes(ci): Diagnosed "Behavior portable serial 5": the shard failed on tests/fm-contributions.test.sh in test_arm_plumbs_a_configured_budget_into_the_check_shim (inherited mode) with "generated check did not attempt a read" and a missing forge/calls file. Root cause: bin/fm-contributions.sh:379-382 computes DEADLINE=$(date +%s)+BUDGET and then gates each read on DEADLINE-$(date +%s) >= OBSERVATION_RESERVE (= min(BUDGET,15)). At the one-second budget this test uses, that gate demands zero elapsed whole seconds, so a wall-clock second boundary crossing between the two date calls makes the poll loop break before any forge read. The test calls wrap_forge (which installs a fake date reading $FORGE/clock only when that file exists) but never seeded forge/clock, so it ran against the real clock. The repo already documents this exact hazard at tests/fm-contributions.test.sh:675-677 and freezes the clock in every other budget-constrained test: test_budget_exhaustion_keeps_prior_record (both modes), test_genuine_failure_near_deadline_is_unavailable, test_reservation_defers_later_url_when_fifteen_seconds_do_not_remain, test_slow_read_deadline_kill_is_budget_refusal, test_budget_is_cut_down_to_the_watcher_check_bound. test_arm_plumbs was the only omission; its single loop body covers both the configured and inherited modes. The remaining wrap_forge users run the default 20-second budget (5 seconds of slack above the reserve) and are not reachable by this race. Fix (smallest, matching the sibling idiom): added the clock freeze `/bin/date +%s > "$home/forge/clock"` plus a two-line comment inside that test's loop, before the poll. Three added lines in tests/fm-contributions.test.sh; no production code and no other file touched. Verification: reproduced the exact CI failure message by running a scratch copy whose fake date advances one second per read (worst case of the unfrozen clock) - missing forge/calls, "not ok - generated check did not attempt a read". With the fix, the single test passes 10/10 standalone and the full tests/fm-contributions.test.sh passes all 46 assertions with exit 0. tests/fm-startup-growth-check.test.sh still exits 0. bash -n and shellcheck -S warning are clean on the edited file; scratch repro files were removed, leaving only the intended three-line change. Note for the author: this flake is not caused by this branch - tests/fm-contributions.test.sh is untouched by the PR and the race is pre-existing, surfacing only on a loaded runner (the shard's neighbouring assertions show 80+ second gaps). It was fixed because it is genuine nondeterminism with an established in-repo remedy and the check cannot otherwise go green. No work was done on the unselected greptile findings (ci-2..ci-5) or the cancelled "Behavior portable serial 2" check (ci-6) * no-mistakes(ci): Diagnosed "Behavior portable serial 6": the omitted log section (fetched with gh api) shows the shard failed on tests/fm-calm-pi-extension.test.sh in test_hidden_block_geometry_e2e with "Pi Calm hidden-block geometry E2E did not complete the /reload viewport transition" (exit=1, duration_ms=23437; the family line pure-contract-unit count=3 duration_ms=51193 failed=1 matches devin-harness 3203 + calm-pi 23437 + task-delivery 24553). Root cause: tests/fm-calm-pi-extension.test.sh:2831 wait_for_geometry_transition detected the /reload by SAMPLING a single intermediate frame - it polled tmux capture-pane for Pi's transient "Reloading keybindings, extensions, skills, prompts, themes, and context files..." box and only accepted the rebuilt transcript afterwards (elif gated on saw_transient). Pi shows that box only while the reload runs and replaces it via dismissReloadBox when done, so on a loaded runner the box can live entirely between two polls; saw_transient then stays 0 forever and the 600-attempt wait times out although the reload fully succeeded. Reproduced locally under CPU oversubscription with CI's Pi 1.0.4: 1 failure in 8 runs, diagnostics proving the mechanism ("DIAG: transition timeout saw_transient=0 attempt=600") while the captured viewport already showed the completed reload status row and the intact collapsed transcript. Not a Pi-version regression: 1.0.4's handleReloadCommand and both banner strings are identical to the installed 0.85.1, and shard 6 of the previous pipeline run (job 112490351105) passed this same script on Pi 1.0.4. Invariant violated: a TUI E2E assertion must key off a durable state the UI retains, never off one intermediate frame polling may miss. Sibling enumeration: the helper was defined once and called once; grep for transient/saw_/two-stage waits across tests/, bin/ and .pi/ found no other one-shot-frame wait - every other wait in the file keys off durable text presence/absence or off continuously repeating animation frames (the working-ship motion checks) where sampling eventually connects; no doc or other test referenced the helper. Fix (smallest, one helper): renamed it wait_for_geometry_reload and made it wait for the durable status row Pi appends once the reload completed and the chat was rebuilt ("Reloaded keybindings, extensions, skills, prompts, themes, and context files", a persistent chat child via showStatus, present in both 0.85.1 and 1.0.4) together with CALM_GEOMETRY_FINAL. That marker is absent before the reload and never appears if the reload throws, so the check is strictly stronger than before (a failed reload now fails the test instead of passing on transient-then-final). One file changed: tests/fm-calm-pi-extension.test.sh, 2 hunks, no production code. Verification: deterministic before/after - with polls spaced 0.4s apart (the loaded-runner worst case) the pre-fix helper fails with exactly the CI message and the post-fix helper passes; 12/12 consecutive passes of the isolated test under load with Pi 1.0.4; full file 15/15 ok exit 0 under LANG=C.utf8 (matching the runner locale); bash -n, shellcheck -S warning and bin/fm-lint.sh clean; scratch repro files and the temp Pi 1.0.4 install removed, git status shows only the one intended modification. Notes: (1) the flake is not caused by this branch - that test file is untouched by the PR and the race is pre-existing; fixed because it is genuine nondeterminism and the check cannot otherwise go green. (2) Under far harsher load than CI applies a separate unrelated flake appeared in the same test: Pi's "Warning: tmux extended-keys is off..." row occasionally lands between the collapsed [skill] ahoy row and the final response, making assert_geometry_gap see 5 rows instead of 2. Different cause, never seen in CI, and its plausible remedies would change key delivery for the other TUI tests in the file, so it was left alone rather than growing this fix --------- Co-authored-by: Quartermaster --- .../skills/operational-home-layout/SKILL.md | 1 + bin/fm-startup-growth-check.sh | 417 ++++++++++++ docs/configuration.md | 22 + docs/scripts.md | 1 + tests/fm-calm-pi-extension.test.sh | 18 +- tests/fm-contributions.test.sh | 3 + tests/fm-startup-growth-check.test.sh | 629 ++++++++++++++++++ 7 files changed, 1084 insertions(+), 7 deletions(-) create mode 100755 bin/fm-startup-growth-check.sh create mode 100755 tests/fm-startup-growth-check.test.sh diff --git a/.agents/skills/operational-home-layout/SKILL.md b/.agents/skills/operational-home-layout/SKILL.md index 82fa94fc061..2786265f63b 100644 --- a/.agents/skills/operational-home-layout/SKILL.md +++ b/.agents/skills/operational-home-layout/SKILL.md @@ -91,6 +91,7 @@ state/ runtime records and signals; gitignored tool-updates.check.sh generated watched-tool update poll shim and its .check-trust binding; present only after bin/fm-tool-update-check.sh arm; its report record .tool-updates is what keeps one pending update from being reported on every poll mail.check.sh generated received-mail poll shim and its .check-trust binding; present only after bin/fm-mail-check.sh arm; report record .mail-check (mail schema: docs/configuration.md "Mail plane") .mail-seen .mail-woken .mail-retry .mail-retry-pos .mail-turn .mail-seen.lock mail-plane poll cursor, emission journal, transient-fetch retry set, retry-scan position, contended-slot turn flag, and overlapping-poll lock; written only by bin/fm-mail.sh (mail schema: docs/configuration.md "Mail plane") + startup-growth.check.sh generated daily startup-growth poll shim and its .check-trust binding; present only after bin/fm-startup-growth-check.sh arm; its record .startup-growth-check holds the daily gate, the per-file growth baselines, and the last reported finding set, so removing it re-baselines growth silently and repeats a standing finding such as a budget overrun once (docs/configuration.md "Daily startup growth check") pending-replies/ parent-owned secondmate pending-reply records (correlation id, delivery vs reply, recovery, escalation); fm-pending-reply-lib.sh procevent/ registered process-to-event sources, one private record per canonical source id; written only by bin/fm-procevent.sh, and their presence alone keeps supervision required (`process-event-sources` skill) procevent-inbox/ private captured results and their durable handled-acknowledgement markers; source output lives here and never in an event line diff --git a/bin/fm-startup-growth-check.sh b/bin/fm-startup-growth-check.sh new file mode 100755 index 00000000000..3543666d727 --- /dev/null +++ b/bin/fm-startup-growth-check.sh @@ -0,0 +1,417 @@ +#!/usr/bin/env bash +# fm-startup-growth-check.sh - daily cheap growth check for startup memory and instruction surfaces. +# +# Usage: +# fm-startup-growth-check.sh [check] +# fm-startup-growth-check.sh arm +# fm-startup-growth-check.sh disarm +# fm-startup-growth-check.sh --help +# +# `check` evaluates at most once every 86400 seconds, one daily evaluation. +# Polls inside that interval only read this check's small state record and stay +# silent. +# +# A due evaluation uses metadata only: regular-file safety checks plus stat(1) +# byte sizes. It does not run the startup digest, bootstrap, network checks, +# model calls, repository refreshes, /stow, or full preference/learning +# rereads. The budget total, its verdict, and its secondmate exception come +# from `bin/fm-startup-memory-budget.sh report`, the single owner of +# config/startup-memory-budget, and are never re-derived here. data/projects.md +# and data/secondmates.md are printed in full by every session start too, so +# they are watched for prompt growth without entering that budget total. +# The tracked set is the startup entrypoints session start executes directly +# plus the agent instruction files, not every script and library the startup +# path reaches; those bytes are code/instruction size, not LLM prompt cost. +# +# A secondmate home is never notified about the primary-owned +# data/captain-shared.md it cannot edit: the owner suppresses the budget overrun +# it causes alone, and this check suppresses its per-file growth there while +# still recording the observation. +# +# Growth is measured against a retained per-file baseline rather than only +# against the previous evaluation, so accumulation that stays under one day's +# threshold is still caught. A surface seen for the first time is baselined +# silently, including the first content of an optional file that was absent when +# the check started; an established baseline survives the file disappearing and +# coming back. Reporting a file rebases its baseline to the reported size, so +# accepted growth then stays silent. The thresholds are fixed: +# 2048 bytes for tracked startup/instruction files +# 250 estimated tokens, ceil(bytes / 3), for printed startup memory files +# Budget overrun is always meaningful. +# +# A due evaluation also removes the empty temporary records a killed +# evaluation can leave in state/: only files matching its own mint pattern +# that are empty and untouched for an hour, never a record with bytes in it. +# +# `arm` writes state/startup-growth.check.sh and binds its bytes with +# fm-check-register.sh so the existing watcher slow-check cadence invokes the +# daily gate. `disarm` removes the shim, trust binding, and report record. +set -u +export LC_ALL=C + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +FM_ROOT="${FM_ROOT_OVERRIDE:-$(cd "$SCRIPT_DIR/.." && pwd)}" +FM_HOME="${FM_HOME:-${FM_ROOT_OVERRIDE:-$FM_ROOT}}" +STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" +CONFIG_DIR="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" +DATA_DIR="${FM_DATA_OVERRIDE:-$FM_HOME/data}" +CHECK_ID=startup-growth +CHECK_SHIM="$STATE/$CHECK_ID.check.sh" +CHECK_TRUST="$STATE/$CHECK_ID.check-trust" +RECORD="$STATE/.startup-growth-check" +RECORD_SCHEMA_LINE=$'schema\tfm-startup-growth-check-v1' +REGISTER_BIN="$SCRIPT_DIR/fm-check-register.sh" +BUDGET_BIN="$SCRIPT_DIR/fm-startup-memory-budget.sh" + +# shellcheck source=bin/fm-pr-lib.sh +. "$SCRIPT_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-startup-memory-budget-lib.sh +. "$SCRIPT_DIR/fm-startup-memory-budget-lib.sh" +# shellcheck source=bin/fm-line-cap-lib.sh +. "$SCRIPT_DIR/fm-line-cap-lib.sh" +# shellcheck source=bin/fm-check-lib.sh +. "$SCRIPT_DIR/fm-check-lib.sh" + +usage() { + sed -n '2,48{s/^# \{0,1\}//;p;}' "$0" +} + +fail() { + printf 'fm-startup-growth-check: %s\n' "$1" >&2 + exit 1 +} + +now_epoch() { + case "${FM_STARTUP_GROWTH_NOW:-}" in + ''|*[!0-9]*) date +%s ;; + *) printf '%s\n' "$FM_STARTUP_GROWTH_NOW" ;; + esac +} + +INTERVAL=86400 +BYTE_THRESHOLD=2048 +TOKEN_THRESHOLD=250 +MAX_LINE=1000 +ORPHAN_GRACE=3600 +ORPHAN_SWEEP_LIMIT=64 +PRIMARY_OWNED_MEMORY= +if [ -e "$FM_HOME/.fm-secondmate-home" ] || [ -L "$FM_HOME/.fm-secondmate-home" ]; then + PRIMARY_OWNED_MEMORY=data/captain-shared.md +fi + +file_size() { + if [ "$(uname)" = Darwin ]; then + /usr/bin/stat -f %z "$1" 2>/dev/null + else + stat -c %s "$1" 2>/dev/null + fi +} + +file_mtime() { + if [ "$(uname)" = Darwin ]; then + /usr/bin/stat -f %m "$1" 2>/dev/null + else + stat -c %Y "$1" 2>/dev/null + fi +} + +# A kill landing between mktemp(1) and the traps that own the temporary record +# leaves an empty scratch file nothing else would ever remove. A due +# evaluation sweeps those, bounded on every axis: only the mint pattern, only +# empty regular files, only ones untouched for ORPHAN_GRACE seconds, and at +# most ORPHAN_SWEEP_LIMIT per evaluation. A concurrent evaluation's live +# scratch is minutes younger than that grace, and a scratch carrying any +# record bytes is never a candidate, so neither published baselines nor work in +# flight can be removed here. +sweep_orphan_records() { # + local now=$1 scratch mtime swept=0 + for scratch in "$STATE"/.startup-growth-check.??????; do + [ "$swept" -lt "$ORPHAN_SWEEP_LIMIT" ] || break + [ -f "$scratch" ] && [ ! -L "$scratch" ] && [ ! -s "$scratch" ] || continue + mtime=$(file_mtime "$scratch") || continue + case "$mtime" in ''|*[!0-9]*) continue ;; esac + [ $((now - mtime)) -ge "$ORPHAN_GRACE" ] || continue + rm -f -- "$scratch" || true + swept=$((swept + 1)) + done +} + +append_finding() { + if [ -z "$FINDINGS" ]; then + FINDINGS=$1 + else + FINDINGS="$FINDINGS; $1" + fi +} + +stat_surface() { # + local kind=$1 display=$2 path=$3 absence_ok=$4 bytes tokens prev_baseline baseline delta presence=present + if [ ! -e "$path" ] && [ ! -L "$path" ]; then + bytes=0 + presence=absent + [ "$absence_ok" = yes ] || append_finding "missing $kind $display" + elif [ -L "$path" ] || [ ! -f "$path" ]; then + bytes=0 + presence=unsafe + append_finding "unsafe $kind $display" + else + bytes=$(file_size "$path") || true + case "$bytes" in + ''|*[!0-9]*) + bytes=0 + presence=unreadable + append_finding "unreadable $kind $display" + ;; + esac + fi + + prev_baseline=$(awk -F '\t' -v p="$display" '$1 == p { print $5; found=1; exit } END { if (!found) print "" }' "$OLD_RECORD" 2>/dev/null || true) + case "$prev_baseline" in + ''|*[!0-9]*) prev_baseline= ;; + esac + + if [ "$presence" != present ]; then + baseline=${prev_baseline:--} + elif [ -z "$prev_baseline" ] || [ "$bytes" -le "$prev_baseline" ]; then + baseline=$bytes + else + baseline=$prev_baseline + delta=$((bytes - baseline)) + case "$kind" in + memory|printed-memory) + tokens=$(fm_startup_memory_estimated_tokens_for_bytes "$delta") || tokens=0 + if [ "$tokens" -ge "$TOKEN_THRESHOLD" ]; then + baseline=$bytes + [ "$display" = "$PRIMARY_OWNED_MEMORY" ] \ + || append_finding "$kind growth $display +${tokens} estimated_tokens (+${delta} bytes, total ${bytes} bytes)" + fi + ;; + tracked) + if [ "$delta" -ge "$BYTE_THRESHOLD" ]; then + append_finding "tracked startup surface growth $display +${delta} bytes (total ${bytes} bytes)" + baseline=$bytes + fi + ;; + esac + fi + + printf '%s\t%s\t%s\t%s\t%s\n' "$display" "$kind" "$presence" "$bytes" "$baseline" >> "$NEW_RECORD" || exit 1 +} + +write_record_atomically() { + local tmp=$1 dest=$2 state_device + [ -d "$STATE" ] && [ ! -L "$STATE" ] || return 1 + state_device=$(fm_pr_file_device "$STATE") || return 1 + fm_pr_regular_destination_on_device_or_absent "$dest" "$state_device" || return 1 + mv -f -- "$tmp" "$dest" +} + +record_usable() { + local line + [ -f "$RECORD" ] && [ ! -L "$RECORD" ] || return 1 + IFS= read -r line < "$RECORD" || return 1 + [ "$line" = "$RECORD_SCHEMA_LINE" ] +} + +read_last_eval() { + record_usable || return 0 + awk -F '\t' '$1 == "last_eval" { print $2; exit }' "$RECORD" 2>/dev/null || true +} + +check_due() { + local now last age + now=$(now_epoch) + last=$(read_last_eval) + case "$last" in + ''|*[!0-9]*) printf '%s\n' "$now"; return 0 ;; + esac + age=$((now - last)) + if [ "$age" -lt 0 ] || [ "$age" -ge "$INTERVAL" ]; then + printf '%s\n' "$now" + return 0 + fi + return 1 +} + +evaluate_budget() { + local report line reason valid=yes budget='' total='' status='' exception='' + if ! report=$(FM_HOME="$FM_HOME" FM_CONFIG_OVERRIDE="$CONFIG_DIR" FM_DATA_OVERRIDE="$DATA_DIR" \ + "$BUDGET_BIN" report 2>&1); then + reason=${report##*startup-memory-budget: } + append_finding "startup memory budget unavailable owner=bin/fm-startup-memory-budget.sh reason=${reason//$'\n'/ }" + return 0 + fi + while IFS= read -r line; do + case "$line" in + effective_budget_tokens=*) budget=${line#*=} ;; + total_estimated_tokens=*) total=${line#*=} ;; + budget_status=*) status=${line#*=} ;; + exception=*) exception=${line#*=} ;; + esac + done < <(printf '%s\n' "$report") + case "$budget:$total" in + *[!0-9:]*|:*|*:) valid=no ;; + esac + case "$status" in + within-budget|over-budget) ;; + *) valid=no ;; + esac + case "$exception" in + ''|primary-owned-shared-file-alone-exceeds-budget) ;; + *) valid=no ;; + esac + if [ "$valid" = no ]; then + append_finding "startup memory budget unavailable owner=bin/fm-startup-memory-budget.sh reason=unparseable report" + return 0 + fi + printf '%s\t%s\t%s\t%s\t%s\n' memory_budget "$budget" "$total" "$status" "$exception" >> "$NEW_RECORD" || exit 1 + [ "$status" = over-budget ] && [ -z "$exception" ] || return 0 + append_finding "startup memory budget overrun total_estimated_tokens=$total budget=$budget owner=bin/fm-startup-memory-budget.sh" +} + +run_check() { + local now reported_previous + if ! now=$(check_due); then + return 0 + fi + [ -d "$STATE" ] && [ ! -L "$STATE" ] || fail "state directory is unavailable" + sweep_orphan_records "$now" + OLD_RECORD=$RECORD + record_usable || OLD_RECORD=/dev/null + reported_previous=$(awk -F '\t' '$1 == "reported" { print substr($0, index($0, "\t") + 1); exit }' "$OLD_RECORD" 2>/dev/null || true) + NEW_RECORD=$(mktemp "$STATE/.startup-growth-check.XXXXXX") || exit 1 + trap 'rm -f -- "${NEW_RECORD:-}"' EXIT + trap 'rm -f -- "${NEW_RECORD:-}"; exit 1' HUP INT TERM + FINDINGS= + printf '%s\n' "$RECORD_SCHEMA_LINE" > "$NEW_RECORD" || exit 1 + printf '%s\t%s\n' last_eval "$now" >> "$NEW_RECORD" || exit 1 + + stat_surface tracked AGENTS.md "$FM_ROOT/AGENTS.md" no + stat_surface tracked CLAUDE.md "$FM_ROOT/CLAUDE.md" yes + stat_surface tracked bin/fm-session-start.sh "$FM_ROOT/bin/fm-session-start.sh" no + stat_surface tracked bin/fm-bootstrap.sh "$FM_ROOT/bin/fm-bootstrap.sh" no + stat_surface tracked bin/fm-supervision-instructions.sh "$FM_ROOT/bin/fm-supervision-instructions.sh" no + stat_surface printed-memory data/projects.md "$DATA_DIR/projects.md" yes + stat_surface printed-memory data/secondmates.md "$DATA_DIR/secondmates.md" yes + stat_surface memory data/captain.md "$DATA_DIR/captain.md" yes + stat_surface memory data/captain-shared.md "$DATA_DIR/captain-shared.md" yes + stat_surface memory data/learnings.md "$DATA_DIR/learnings.md" yes + + evaluate_budget + + if [ -n "$FINDINGS" ]; then + if [ "$FINDINGS" != "$reported_previous" ]; then + fm_cap_line "startup-growth: $FINDINGS" "$MAX_LINE" + fi + printf '%s\t%s\n' reported "$FINDINGS" >> "$NEW_RECORD" || exit 1 + fi + write_record_atomically "$NEW_RECORD" "$RECORD" || fail "could not publish report record" + NEW_RECORD= +} + +SHIM_TMP= +ARM_BACKUP= + +shim_write() { # + local want=$1 device=$2 + fm_pr_regular_destination_on_device_or_absent "$CHECK_SHIM" "$device" || return 1 + if [ -e "$CHECK_SHIM" ] && [ "$(fm_pr_file_mode "$CHECK_SHIM")" = 700 ] \ + && [ "$(cat "$CHECK_SHIM" 2>/dev/null)" = "$want" ]; then + return 0 + fi + SHIM_TMP=$(umask 077; mktemp "$STATE/.startup-growth-check-shim.XXXXXX" 2>/dev/null) || return 1 + if ! printf '%s\n' "$want" > "$SHIM_TMP" \ + || ! chmod 0700 "$SHIM_TMP" \ + || ! fm_pr_private_file_valid "$SHIM_TMP" 700 "$device" \ + || ! fm_pr_regular_destination_on_device_or_absent "$CHECK_SHIM" "$device" \ + || ! mv -f -- "$SHIM_TMP" "$CHECK_SHIM"; then + rm -f -- "$SHIM_TMP" + SHIM_TMP= + return 1 + fi + SHIM_TMP= + fm_pr_private_file_valid "$CHECK_SHIM" 700 "$device" +} + +shim_backup() { # + local device=$1 tmp + tmp=$(umask 077; mktemp "$STATE/.startup-growth-check-shim.XXXXXX" 2>/dev/null) || return 1 + if ! cat "$CHECK_SHIM" > "$tmp" 2>/dev/null \ + || ! chmod 0700 "$tmp" \ + || ! fm_pr_private_file_valid "$tmp" 700 "$device"; then + rm -f -- "$tmp" + return 1 + fi + printf '%s\n' "$tmp" +} + +arm_rollback() { + [ -z "$SHIM_TMP" ] || rm -f -- "$SHIM_TMP" + SHIM_TMP= + if [ -n "$ARM_BACKUP" ]; then + mv -f -- "$ARM_BACKUP" "$CHECK_SHIM" 2>/dev/null || rm -f -- "$ARM_BACKUP" + ARM_BACKUP= + if fm_custom_check_registered "$STATE" "$CHECK_ID"; then + return 0 + fi + fi + rm -f -- "$CHECK_SHIM" "$CHECK_TRUST" +} + +arm_failed() { # + trap - HUP INT TERM + arm_rollback + fail "$1" +} + +arm() { + local state_device home want + [ -d "$STATE" ] && [ ! -L "$STATE" ] || fail "state directory is unavailable" + case "$FM_HOME" in + /*) home=$FM_HOME ;; + *) home=$(CDPATH='' cd -- "$FM_HOME" 2>/dev/null && pwd -P) || fail "cannot resolve FM_HOME $FM_HOME" ;; + esac + state_device=$(fm_pr_file_device "$STATE") || fail "state directory is unavailable" + want=$(printf '%s\n' \ + '#!/usr/bin/env bash' \ + "export FM_HOME=$(printf '%q' "$home")" \ + "exec $(printf '%q' "$SCRIPT_DIR/fm-startup-growth-check.sh") check") + ARM_BACKUP= + if [ -f "$CHECK_SHIM" ] && [ ! -L "$CHECK_SHIM" ]; then + ARM_BACKUP=$(shim_backup "$state_device") || fail "could not save the existing check shim" + fi + trap 'arm_failed "arming was interrupted"' HUP INT TERM + shim_write "$want" "$state_device" || arm_failed "check shim path is unavailable" + FM_HOME="$home" "$REGISTER_BIN" "$CHECK_ID" >/dev/null || arm_failed "could not register the check shim" + trap - HUP INT TERM + [ -z "$ARM_BACKUP" ] || rm -f -- "$ARM_BACKUP" + ARM_BACKUP= + printf 'armed: state/%s.check.sh\n' "$CHECK_ID" +} + +disarm() { + rm -f -- "$CHECK_SHIM" "$CHECK_TRUST" "$RECORD" + printf 'disarmed: state/%s.check.sh\n' "$CHECK_ID" +} + +case "${1:-check}" in + check) + [ "$#" -le 1 ] || { usage >&2; exit 2; } + run_check + ;; + arm) + [ "$#" -eq 1 ] || { usage >&2; exit 2; } + arm + ;; + disarm) + [ "$#" -eq 1 ] || { usage >&2; exit 2; } + disarm + ;; + -h|--help|help) + usage + ;; + *) + usage >&2 + exit 2 + ;; +esac diff --git a/docs/configuration.md b/docs/configuration.md index b37cc82c2aa..63695826c71 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -667,6 +667,28 @@ The internal [`/stow` skill](../.agents/skills/stow/SKILL.md) owns curation and The helper's header owns exact parsing, publication, and report output mechanics. +### Daily startup growth check + +A home can arm a lightweight daily growth monitor with `bin/fm-startup-growth-check.sh arm`. +It writes `state/startup-growth.check.sh` and binds it through the existing authenticated watcher-check mechanism, so no extra daemon or scheduler is installed. +Registering it is a reason to watch on the same terms as the [watched-tool check](#watched-tool-updates-configwatched-toolsjson), so an armed home keeps needing a watcher after its last task is torn down. +Use `bin/fm-startup-growth-check.sh disarm` to remove the check and its local report record. + +The check evaluates at most once per day and stays silent when nothing meaningful changed. +A due evaluation uses file metadata and byte sizes before any content inspection: it asks `bin/fm-startup-memory-budget.sh report` for the budget verdict over `data/captain.md`, `data/captain-shared.md`, and `data/learnings.md`, watches the `data/projects.md` and `data/secondmates.md` that session start also prints in full for growth without entering that budget total, and separately watches the tracked startup/instruction owner files described by the script header. +`bin/fm-startup-memory-budget.sh` remains the sole owner of the budget total and its verdict, so the check never re-derives either: when that owner annotates an overrun caused by the primary-owned `data/captain-shared.md` alone, a secondmate home is not woken about an overrun it cannot act on. +A secondmate home is likewise not notified about per-file growth of that same primary-owned `data/captain-shared.md`, which it receives read-only; the growth is still observed and recorded, and a primary home reports it normally. +Those tracked bytes are code and instruction-surface size, not prompt-memory cost. +The check does not run session-start, bootstrap, network checks, model calls, repository refreshes, `/stow`, or full preference/learnings rereads. + +Growth is measured against a per-file baseline retained in the check's own state record, so accumulation that stays under one day's threshold is still caught once it adds up; reporting a file rebases its baseline to the reported size, so accepted growth then stays silent. +A surface observed for the first time is baselined silently, including the first content of an optional file that did not exist yet when the check was armed, and an established baseline survives that file disappearing and coming back. +The fixed growth thresholds are inspectable in the script header: 2048 bytes for tracked startup/instruction files and 250 estimated tokens for the printed startup-memory files. +Budget overrun, unsafe or unreadable inputs, missing required tracked owner files, or material growth are reported once and deduplicated until the finding changes or clears; the report line is delivered before the check advances its own record, so a state-publication failure can repeat a finding but never swallow one. +That one line goes out through the shared per-line digest cut, so an over-long finding set carries the repo's `[truncated]` marker instead of ending mid-finding, while deduplication keeps comparing the full uncapped set. +Older bulk learning files remain reference-only; this monitor neither loads nor merges them. +A reported review need is only a recommendation, not cleanup authority. + ## Stow pass horizon (config/stow-pass-horizon) `config/stow-pass-horizon` is an optional local, gitignored presence flag that opts this home in to the pass-count decay horizon in the internal [`/stow` skill](../.agents/skills/stow/SKILL.md). diff --git a/docs/scripts.md b/docs/scripts.md index 3e2f475c830..e6e8da4bb6d 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -135,6 +135,7 @@ The shared no-mistakes gate lifecycle boundary is summarized in [architecture.md | `fm-check-unregister.sh` | Retire a custom watcher check and its trust binding by validated task id | | `fm-check-lib.sh` | Validate custom-check registrations and prepare private execution snapshots | | `fm-tool-update-check.sh` | Report watched tooling with an update available, and updates installed but left inert by PATH order | +| `fm-startup-growth-check.sh` | Daily metadata-only growth check for startup memory and tracked startup/instruction surfaces | | `fm-pr-lib.sh` | Own canonical task and PR validation plus private atomic PR-poll publication, merge-notification identity, and retirement | | `fm-pr-poll.sh` | Provide the byte-static watcher program for validated pull-request, merge-request, and Gerrit-change poll sidecars | | `fm-contributions.sh` | Observe owned publications, retain exact-head judgments, measure required actors, and wake on maintainer signals | diff --git a/tests/fm-calm-pi-extension.test.sh b/tests/fm-calm-pi-extension.test.sh index f4baded12f4..f1f917f650a 100755 --- a/tests/fm-calm-pi-extension.test.sh +++ b/tests/fm-calm-pi-extension.test.sh @@ -2833,13 +2833,17 @@ TS return 1 } - wait_for_geometry_transition() { - local file=$1 transient_text=$2 final_text=$3 attempt=0 saw_transient=0 + # Pi's "Reloading..." box is a single intermediate frame, so no polling + # interval can be guaranteed to sample it on a loaded machine. Wait instead + # for the durable status row Pi appends to the transcript once the reload has + # completed and the chat has been rebuilt: it is absent before the reload and + # never appears when the reload fails. + wait_for_geometry_reload() { + local file=$1 reloaded_text=$2 final_text=$3 attempt=0 while [ "$attempt" -lt 600 ]; do capture_geometry_viewport "$file" || true - if grep -Fq "$transient_text" "$file" 2>/dev/null; then - saw_transient=1 - elif [ "$saw_transient" -eq 1 ] && grep -Fq "$final_text" "$file" 2>/dev/null; then + if grep -Fq "$reloaded_text" "$file" 2>/dev/null && + grep -Fq "$final_text" "$file" 2>/dev/null; then return 0 fi sleep 0.01 @@ -2894,9 +2898,9 @@ TS tmux -L "$TMUX_SOCKET" send-keys -t "$TMUX_SESSION" -l '/reload' tmux -L "$TMUX_SOCKET" send-keys -t "$TMUX_SESSION" Enter - wait_for_geometry_transition \ + wait_for_geometry_reload \ "$snapshot" \ - "Reloading keybindings, extensions, skills, prompts, themes, and context files..." \ + "Reloaded keybindings, extensions, skills, prompts, themes, and context files" \ "CALM_GEOMETRY_FINAL" \ || fail "Pi Calm hidden-block geometry E2E did not complete the /reload viewport transition" assert_geometry_gap "$snapshot" "reloaded native Calm transcript" diff --git a/tests/fm-contributions.test.sh b/tests/fm-contributions.test.sh index 9af32fc2950..47ccb09854b 100755 --- a/tests/fm-contributions.test.sh +++ b/tests/fm-contributions.test.sh @@ -1020,6 +1020,9 @@ test_arm_plumbs_a_configured_budget_into_the_check_shim() { wrap_forge "$home" mutate_record "$home" delivery '.records[0].checked_at="2026-09-15T08:00:00Z"' cp "$home/data/delivery/contributions.json" "$home/prior.json" + # Freeze the clock: an unfrozen one can tick past the one-second budget + # before the first forge call, so nothing is ever observed. + /bin/date +%s > "$home/forge/clock" printf 'hang\n' > "$home/forge/fault" if [ "$mode" = configured ]; then with_home "$home" env FM_CONTRIBUTIONS_BUDGET=1 "$ROOT/bin/fm-contributions.sh" arm >/dev/null \ diff --git a/tests/fm-startup-growth-check.test.sh b/tests/fm-startup-growth-check.test.sh new file mode 100755 index 00000000000..6fbec8b66e1 --- /dev/null +++ b/tests/fm-startup-growth-check.test.sh @@ -0,0 +1,629 @@ +#!/usr/bin/env bash +# Behavioral coverage for the daily startup growth check. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" +# shellcheck source=bin/fm-pr-lib.sh +. "$ROOT/bin/fm-pr-lib.sh" + +TMP_ROOT=$(fm_test_tmproot fm-startup-growth-check) +CHECK="$ROOT/bin/fm-startup-growth-check.sh" + +make_world() { + local name=$1 root home + root="$TMP_ROOT/$name/root" + home="$TMP_ROOT/$name/home" + mkdir -p "$root/bin" "$home/config" "$home/data" "$home/state" + printf '# Firstmate\n' > "$root/AGENTS.md" + printf 'See AGENTS.md\n' > "$root/CLAUDE.md" + printf '#!/usr/bin/env bash\nexit 0\n' > "$root/bin/fm-session-start.sh" + printf '#!/usr/bin/env bash\nexit 0\n' > "$root/bin/fm-bootstrap.sh" + printf '#!/usr/bin/env bash\nexit 0\n' > "$root/bin/fm-supervision-instructions.sh" + printf '7500\n' > "$home/config/startup-memory-budget" + printf 'projects\n' > "$home/data/projects.md" + printf 'secondmates\n' > "$home/data/secondmates.md" + printf 'captain\n' > "$home/data/captain.md" + printf 'shared\n' > "$home/data/captain-shared.md" + printf 'learnings\n' > "$home/data/learnings.md" + printf '%s|%s\n' "$root" "$home" +} + +run_check() { + local root=$1 home=$2 now=$3 out status=0 + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_STARTUP_GROWTH_NOW="$now" "$CHECK" check 2>&1) || status=$? + printf '%s\t%s\n' "$status" "$out" +} + +output_part() { printf '%s' "$1" | cut -f2-; } +status_part() { printf '%s' "$1" | cut -f1; } + +add_bytes() { + local path=$1 count=$2 + dd if=/dev/zero bs=1 count="$count" 2>/dev/null | tr '\000' x >> "$path" +} + +budget_total() { # + awk -F '\t' '$1 == "memory_budget" { print $3; exit }' "$1/state/.startup-growth-check" +} + +test_initial_baseline_is_silent_and_records_metadata() { + local rec root home result out + rec=$(make_world baseline) + root=${rec%%|*} + home=${rec#*|} + result=$(run_check "$root" "$home" 1000) + [ "$(status_part "$result")" = 0 ] || fail "baseline check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "baseline without findings should stay silent: $out" + assert_grep $'last_eval\t1000' "$home/state/.startup-growth-check" "baseline did not record the evaluation time" + assert_grep $'AGENTS.md\ttracked\tpresent' "$home/state/.startup-growth-check" "baseline did not record tracked startup metadata" + assert_grep $'data/learnings.md\tmemory\tpresent' "$home/state/.startup-growth-check" "baseline did not record memory metadata" + assert_grep $'data/projects.md\tprinted-memory\tpresent' "$home/state/.startup-growth-check" "baseline did not record printed projects metadata" + assert_grep $'data/secondmates.md\tprinted-memory\tpresent' "$home/state/.startup-growth-check" "baseline did not record printed secondmates metadata" +} + +test_same_day_poll_does_not_touch_surfaces() { + local rec root home result out + rec=$(make_world same-day) + root=${rec%%|*} + home=${rec#*|} + run_check "$root" "$home" 1000 >/dev/null + rm -f "$root/AGENTS.md" + ln -s /no/such/place "$root/AGENTS.md" + result=$(run_check "$root" "$home" 1200) + [ "$(status_part "$result")" = 0 ] || fail "same-day poll failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "same-day poll inspected surfaces instead of staying gated: $out" + assert_grep $'last_eval\t1000' "$home/state/.startup-growth-check" "same-day poll rewrote the daily record" +} + +test_due_growth_reports_once_and_dedupes() { + local rec root home result out + rec=$(make_world growth) + root=${rec%%|*} + home=${rec#*|} + run_check "$root" "$home" 1000 >/dev/null + add_bytes "$root/AGENTS.md" 2500 + add_bytes "$home/data/learnings.md" 900 + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "growth check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'tracked startup surface growth AGENTS.md +2500 bytes' "tracked growth was not reported" + assert_contains "$out" 'memory growth data/learnings.md +300 estimated_tokens (+900 bytes' "memory growth was not reported as estimated prompt cost" + result=$(run_check "$root" "$home" 173802) + [ "$(status_part "$result")" = 0 ] || fail "dedupe check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "unchanged persistent finding was repeated: $out" +} + +test_gradual_growth_below_daily_threshold_is_reported_cumulatively() { + local rec root home result out now day + rec=$(make_world cumulative) + root=${rec%%|*} + home=${rec#*|} + now=1000 + result=$(run_check "$root" "$home" "$now") + [ "$(status_part "$result")" = 0 ] || fail "cumulative baseline failed: $(output_part "$result")" + for day in 1 2 3; do + add_bytes "$root/AGENTS.md" 700 + add_bytes "$home/data/learnings.md" 300 + now=$((now + 86401)) + result=$(run_check "$root" "$home" "$now") + [ "$(status_part "$result")" = 0 ] || fail "cumulative day $day failed: $(output_part "$result")" + out=$(output_part "$result") + if [ "$day" -lt 3 ]; then + [ -z "$out" ] || fail "sub-threshold day $day should stay silent: $out" + fi + done + assert_contains "$out" 'tracked startup surface growth AGENTS.md +2100 bytes' "cumulative tracked growth was not reported once it added up" + assert_contains "$out" 'memory growth data/learnings.md +300 estimated_tokens (+900 bytes' "cumulative memory growth was not reported once it added up" + + now=$((now + 86401)) + result=$(run_check "$root" "$home" "$now") + [ "$(status_part "$result")" = 0 ] || fail "post-report check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "reported growth was repeated after the baseline was rebased: $out" +} + +test_printed_memory_growth_is_reported_without_entering_the_budget_total() { + local rec root home result out before after + rec=$(make_world printed) + root=${rec%%|*} + home=${rec#*|} + run_check "$root" "$home" 1000 >/dev/null + before=$(budget_total "$home") + add_bytes "$home/data/projects.md" 900 + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "printed-memory check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'printed-memory growth data/projects.md +300 estimated_tokens (+900 bytes' "printed startup memory growth was not reported" + after=$(budget_total "$home") + assert_equals "$before" "$after" "printed startup memory changed the budget total it must not own" +} + +test_first_content_of_an_optional_file_is_baselined_silently() { + local rec root home result out + rec=$(make_world first-content) + root=${rec%%|*} + home=${rec#*|} + rm -f "$home/data/secondmates.md" + result=$(run_check "$root" "$home" 1000) + [ "$(status_part "$result")" = 0 ] || fail "absent-surface baseline failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "an absent optional surface should stay silent: $out" + + add_bytes "$home/data/secondmates.md" 1000 + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "first-content check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "first content of an optional file was reported as growth: $out" + + add_bytes "$home/data/secondmates.md" 900 + result=$(run_check "$root" "$home" 173802) + [ "$(status_part "$result")" = 0 ] || fail "post-baseline growth check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'printed-memory growth data/secondmates.md +300 estimated_tokens (+900 bytes' "growth above the silently established baseline was not reported" +} + +test_established_baseline_survives_disappearance_and_restoration() { + local rec root home result out + rec=$(make_world restored) + root=${rec%%|*} + home=${rec#*|} + add_bytes "$home/data/projects.md" 3000 + run_check "$root" "$home" 1000 >/dev/null + + rm -f "$home/data/projects.md" + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "disappearance check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "an absent optional surface should stay silent: $out" + + printf 'projects\n' > "$home/data/projects.md" + add_bytes "$home/data/projects.md" 3000 + result=$(run_check "$root" "$home" 173802) + [ "$(status_part "$result")" = 0 ] || fail "restoration check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "restoring a file at its established size was reported as growth: $out" + + add_bytes "$home/data/projects.md" 900 + result=$(run_check "$root" "$home" 260203) + [ "$(status_part "$result")" = 0 ] || fail "post-restoration growth check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'printed-memory growth data/projects.md +300 estimated_tokens (+900 bytes' "growth above the preserved baseline was not reported after restoration" +} + +test_secondmate_is_not_notified_about_primary_owned_shared_growth() { + local rec root home result out + rec=$(make_world shared-growth) + root=${rec%%|*} + home=${rec#*|} + : > "$home/.fm-secondmate-home" + run_check "$root" "$home" 1000 >/dev/null + add_bytes "$home/data/captain-shared.md" 900 + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "secondmate shared-growth check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "a secondmate was notified about growth of the read-only primary-owned shared file: $out" + + rec=$(make_world shared-growth-primary) + root=${rec%%|*} + home=${rec#*|} + run_check "$root" "$home" 1000 >/dev/null + add_bytes "$home/data/captain-shared.md" 900 + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "primary shared-growth check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'memory growth data/captain-shared.md +300 estimated_tokens (+900 bytes' "a primary home did not report growth of its own shared file" +} + +test_budget_overrun_reports_and_separates_prompt_cost() { + local rec root home result out + rec=$(make_world overrun) + root=${rec%%|*} + home=${rec#*|} + printf '10\n' > "$home/config/startup-memory-budget" + add_bytes "$home/data/captain.md" 90 + add_bytes "$root/bin/fm-bootstrap.sh" 3000 + result=$(run_check "$root" "$home" 1000) + [ "$(status_part "$result")" = 0 ] || fail "overrun check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'startup memory budget overrun total_estimated_tokens=' "budget overrun was not reported" + assert_not_contains "$out" 'tracked startup surface growth' "initial tracked growth should not be inferred without a baseline" + assert_not_contains "$out" 'bin/fm-bootstrap.sh' "initial tracked bytes were incorrectly counted as prompt-memory overrun evidence" + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "repeated overrun check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "standing budget overrun was reported again instead of deduplicated: $out" +} + +test_secondmate_is_not_woken_about_the_primary_owned_shared_overrun() { + local rec root home result out + rec=$(make_world secondmate) + root=${rec%%|*} + home=${rec#*|} + printf '10\n' > "$home/config/startup-memory-budget" + add_bytes "$home/data/captain-shared.md" 900 + : > "$home/.fm-secondmate-home" + result=$(run_check "$root" "$home" 1000) + [ "$(status_part "$result")" = 0 ] || fail "secondmate overrun check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_not_contains "$out" 'startup memory budget overrun' "a secondmate was woken about the primary-owned shared overrun it cannot act on" + + rm -f "$home/.fm-secondmate-home" + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "primary overrun check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'startup memory budget overrun total_estimated_tokens=' "the same overrun was not reported in a primary home" +} + +test_metadata_read_failure_keeps_the_retained_baseline() { + local rec root home result out fakebin real_stat + if [ "$(uname)" = Darwin ]; then + printf 'note - metadata read failure is exercised through the stat -c branch; skipping on Darwin\n' + return 0 + fi + rec=$(make_world unreadable) + root=${rec%%|*} + home=${rec#*|} + add_bytes "$root/AGENTS.md" 5000 + result=$(run_check "$root" "$home" 1000) + [ "$(status_part "$result")" = 0 ] || fail "unreadable baseline failed: $(output_part "$result")" + + real_stat=$(command -v stat) || fail "no stat(1) on PATH" + fakebin="$TMP_ROOT/unreadable/fakebin" + mkdir -p "$fakebin" + cat > "$fakebin/stat" <&1) \ + || fail "unreadable due run failed: $out" + assert_contains "$out" 'unreadable tracked AGENTS.md' "a failed size read was not reported as unreadable" + + result=$(run_check "$root" "$home" 173802) + [ "$(status_part "$result")" = 0 ] || fail "post-failure check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "a failed metadata read destroyed the retained baseline and fabricated growth: $out" +} + +test_due_unsafe_inputs_are_reported_but_absent_optional_memory_is_not() { + local rec root home result out + rec=$(make_world unsafe) + root=${rec%%|*} + home=${rec#*|} + run_check "$root" "$home" 1000 >/dev/null + rm -f "$home/data/captain-shared.md" "$home/data/learnings.md" + ln -s "$home/data/captain.md" "$home/data/learnings.md" + result=$(run_check "$root" "$home" 87401) + [ "$(status_part "$result")" = 0 ] || fail "unsafe check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'unsafe memory data/learnings.md' "unsafe symlinked memory was not reported" + assert_not_contains "$out" 'missing memory data/captain-shared.md' "absent optional shared memory should not be reported" + result=$(run_check "$root" "$home" 173802) + [ "$(status_part "$result")" = 0 ] || fail "repeated unsafe check failed: $(output_part "$result")" + out=$(output_part "$result") + [ -z "$out" ] || fail "standing unsafe finding was reported again instead of deduplicated: $out" +} + +test_over_long_finding_set_is_capped_with_the_shared_marker() { + local rec root home deep seg out reported + rec=$(make_world capped) + root=${rec%%|*} + home=${rec#*|} + seg= + while [ "${#seg}" -lt 100 ]; do + seg="${seg}memory" + done + deep="$home/data" + while [ "${#deep}" -lt 1200 ]; do + deep="$deep/$seg" + done + mkdir -p "$deep" + ln -s "$home/data/captain.md" "$deep/learnings.md" + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_DATA_OVERRIDE="$deep" FM_STARTUP_GROWTH_NOW=1000 \ + "$CHECK" check 2>/dev/null) || fail "capped check failed" + assert_contains "$out" 'unsafe memory data/learnings.md' "the leading finding was lost" + [ "${#out}" -le 1000 ] || fail "the wake line was emitted uncapped at ${#out} characters" + assert_contains "$out" ' [truncated]' "the capped wake line carries no truncation marker" + reported=$(awk -F '\t' '$1 == "reported" { print substr($0, index($0, "\t") + 1); exit }' \ + "$home/state/.startup-growth-check") + [ "${#reported}" -gt "${#out}" ] \ + || fail "the dedupe record stored the capped line instead of the full finding set" +} + +test_unknown_budget_verdict_fields_are_reported_as_unparseable() { + local rec root home fixbin out + fixbin=$(make_isolated_bin verdict 0) + + cat > "$fixbin/fm-startup-memory-budget.sh" <<'STUB' +#!/usr/bin/env bash +printf 'role=primary +' +printf 'effective_budget_tokens=7500 +' +printf 'total_estimated_tokens=10 +' +printf 'budget_status=sideways +' +STUB + chmod 0755 "$fixbin/fm-startup-memory-budget.sh" + rec=$(make_world verdict-status) + root=${rec%%|*} + home=${rec#*|} + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_STARTUP_GROWTH_NOW=1000 \ + "$fixbin/fm-startup-growth-check.sh" check 2>&1) || fail "unknown-status check failed: $out" + assert_contains "$out" 'reason=unparseable report' "an unrecognized budget_status was accepted as within-budget" + + cat > "$fixbin/fm-startup-memory-budget.sh" <<'STUB' +#!/usr/bin/env bash +printf 'role=primary +' +printf 'effective_budget_tokens=10 +' +printf 'total_estimated_tokens=7500 +' +printf 'budget_status=over-budget +' +printf 'exception=some-unrelated-annotation +' +STUB + chmod 0755 "$fixbin/fm-startup-memory-budget.sh" + rec=$(make_world verdict-exception) + root=${rec%%|*} + home=${rec#*|} + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_STARTUP_GROWTH_NOW=1000 \ + "$fixbin/fm-startup-growth-check.sh" check 2>&1) || fail "unknown-exception check failed: $out" + assert_contains "$out" 'reason=unparseable report' "an unrecognized exception annotation silently suppressed the overrun" +} + +test_record_with_a_foreign_schema_marker_is_not_trusted() { + local rec root home record out first + rec=$(make_world foreign-schema) + root=${rec%%|*} + home=${rec#*|} + record="$home/state/.startup-growth-check" + { + printf 'schema\tfm-startup-growth-check-v2\n' + printf 'last_eval\t1000\n' + printf 'AGENTS.md\tpresent\t1\t1\t1\n' + } > "$record" + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_STARTUP_GROWTH_NOW=1200 "$CHECK" check 2>&1) \ + || fail "foreign-schema check failed: $out" + [ -z "$out" ] || fail "a record from another schema was reinterpreted instead of re-baselined: $out" + IFS= read -r first < "$record" + assert_equals "$(printf 'schema\tfm-startup-growth-check-v1')" "$first" "the foreign record was kept instead of replaced" + assert_grep $'last_eval\t1200' "$record" "the foreign record's last_eval gated the evaluation instead of being ignored" +} + +test_findings_are_delivered_even_when_the_record_cannot_be_published() { + local rec root home out status=0 leftover + rec=$(make_world unpublishable) + root=${rec%%|*} + home=${rec#*|} + printf '5\n' > "$home/config/startup-memory-budget" + ln -s /no/such/place "$home/state/.startup-growth-check" + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_STARTUP_GROWTH_NOW=1000 "$CHECK" check 2>/dev/null) || status=$? + [ "$status" != 0 ] || fail "an unpublishable report record should surface as a failure" + assert_contains "$out" 'startup memory budget overrun' "the finding was dropped because the record could not be published" + leftover=$(cd "$home/state" && ls -1 .startup-growth-check.?????? 2>/dev/null || true) + [ -z "$leftover" ] || fail "a failed evaluation leaked its temporary record: $leftover" +} + +# Copies the check and the libraries it sources into an isolated bin, so the +# helpers it execs can be replaced: the budget owner with a stub report, or the +# register with a failing one. +make_isolated_bin() { # + local name=$1 code=$2 bin + bin="$TMP_ROOT/$name/bin" + mkdir -p "$bin" + cp "$ROOT/bin/fm-startup-growth-check.sh" "$ROOT/bin/fm-pr-lib.sh" \ + "$ROOT/bin/fm-startup-memory-budget-lib.sh" "$ROOT/bin/fm-line-cap-lib.sh" \ + "$ROOT/bin/fm-check-lib.sh" "$ROOT/bin/fm-startup-memory-budget.sh" "$bin/" + if [ "$code" = 0 ]; then + cp "$ROOT/bin/fm-check-register.sh" "$bin/fm-check-register.sh" + else + printf '#!/usr/bin/env bash\nexit %s\n' "$code" > "$bin/fm-check-register.sh" + chmod 0755 "$bin/fm-check-register.sh" + fi + printf '%s\n' "$bin" +} + +test_rearming_an_unchanged_binding_does_not_replace_the_shim() { + local rec root home bin before after out + rec=$(make_world rearm-noop) + root=${rec%%|*} + home=${rec#*|} + bin=$(make_isolated_bin rearm-noop 0) + FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$bin/fm-startup-growth-check.sh" arm >/dev/null \ + || fail "first arm failed" + before=$(fm_pr_file_inode "$home/state/startup-growth.check.sh") + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$bin/fm-startup-growth-check.sh" arm 2>&1) \ + || fail "re-arm of an unchanged binding failed: $out" + after=$(fm_pr_file_inode "$home/state/startup-growth.check.sh") + assert_equals "$before" "$after" "re-arming replaced a shim whose bytes were already correct" + out=$(env -u FM_HOME FM_ROOT_OVERRIDE="$root" FM_STARTUP_GROWTH_NOW=1000 \ + "$home/state/startup-growth.check.sh" 2>&1) || fail "the re-armed shim no longer runs: $out" + assert_present "$home/state/.startup-growth-check" "the re-armed shim did not run the daily check" +} + +test_failed_first_arm_leaves_the_home_plainly_unarmed() { + local rec root home bin status=0 out leftover + rec=$(make_world arm-fail) + root=${rec%%|*} + home=${rec#*|} + bin=$(make_isolated_bin arm-fail 1) + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$bin/fm-startup-growth-check.sh" arm 2>&1) || status=$? + [ "$status" != 0 ] || fail "a failed registration reported a successful arm" + assert_absent "$home/state/startup-growth.check.sh" "a failed first arm left an unregistered shim behind" + assert_absent "$home/state/startup-growth.check-trust" "a failed first arm left a trust binding behind" + leftover=$(cd "$home/state" && ls -1 .startup-growth-check-shim.?????? 2>/dev/null || true) + [ -z "$leftover" ] || fail "a failed arm leaked its staged shim: $leftover" +} + +test_failed_rearm_keeps_the_previously_armed_shim_and_trust() { + local rec root home bin good bad shim trust status=0 out + rec=$(make_world rearm-fail) + root=${rec%%|*} + home=${rec#*|} + good=$(make_isolated_bin rearm-fail 0) + FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$good/fm-startup-growth-check.sh" arm >/dev/null \ + || fail "first arm failed" + shim=$(cat "$home/state/startup-growth.check.sh") + trust=$(cat "$home/state/startup-growth.check-trust") + + bad=$(make_isolated_bin rearm-fail-bad 1) + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$bad/fm-startup-growth-check.sh" arm 2>&1) || status=$? + [ "$status" != 0 ] || fail "a failed re-registration reported a successful arm" + assert_present "$home/state/startup-growth.check.sh" "a failed re-arm disarmed a working home" + assert_present "$home/state/startup-growth.check-trust" "a failed re-arm removed the trust binding of a working home" + assert_equals "$shim" "$(cat "$home/state/startup-growth.check.sh")" "a failed re-arm changed the armed shim" + assert_equals "$trust" "$(cat "$home/state/startup-growth.check-trust")" "a failed re-arm changed the trust binding" + out=$(env -u FM_HOME FM_ROOT_OVERRIDE="$root" FM_STARTUP_GROWTH_NOW=1000 \ + "$home/state/startup-growth.check.sh" 2>&1) || fail "the preserved shim no longer runs: $out" + assert_present "$home/state/.startup-growth-check" "the preserved shim did not run the daily check" +} + +test_interrupted_evaluation_abandons_its_partial_record() { + local rec root home bin ready pid status=0 waited=0 first leftover result out + rec=$(make_world interrupted) + root=${rec%%|*} + home=${rec#*|} + run_check "$root" "$home" 1000 >/dev/null + add_bytes "$root/AGENTS.md" 1500 + + bin=$(make_isolated_bin interrupted 0) + ready="$TMP_ROOT/interrupted/budget-entered" + cat > "$bin/fm-startup-memory-budget.sh" < "$ready" +while [ -f "$ready" ]; do + sleep 0.05 +done +printf 'role=primary\n' +printf 'effective_budget_tokens=7500\n' +printf 'total_estimated_tokens=10\n' +printf 'budget_status=within-budget\n' +STUB + chmod 0755 "$bin/fm-startup-memory-budget.sh" + + FM_ROOT_OVERRIDE="$root" FM_HOME="$home" FM_STARTUP_GROWTH_NOW=87401 \ + "$bin/fm-startup-growth-check.sh" check >/dev/null 2>&1 & + pid=$! + while [ ! -f "$ready" ]; do + [ "$waited" -lt 200 ] || fail "the check never reached its budget call" + waited=$((waited + 1)) + sleep 0.05 + done + kill -TERM "$pid" || fail "could not signal the running check" + rm -f "$ready" + wait "$pid" || status=$? + [ "$status" != 0 ] || fail "an interrupted evaluation finished as if it had published a record" + + IFS= read -r first < "$home/state/.startup-growth-check" + assert_equals "$(printf 'schema\tfm-startup-growth-check-v1')" "$first" "an interrupted evaluation published a partial record" + assert_grep $'last_eval\t1000' "$home/state/.startup-growth-check" "an interrupted evaluation advanced the daily gate" + leftover=$(cd "$home/state" && ls -1 .startup-growth-check.?????? 2>/dev/null || true) + [ -z "$leftover" ] || fail "an interrupted evaluation left its temporary record behind: $leftover" + + add_bytes "$root/AGENTS.md" 1000 + result=$(run_check "$root" "$home" 173802) + [ "$(status_part "$result")" = 0 ] || fail "post-interrupt check failed: $(output_part "$result")" + out=$(output_part "$result") + assert_contains "$out" 'tracked startup surface growth AGENTS.md +2500 bytes' "the retained baselines did not survive an interrupted evaluation" +} + +test_aged_empty_orphan_records_are_swept_without_touching_live_work() { + local rec root home state now result out + rec=$(make_world orphan-sweep) + root=${rec%%|*} + home=${rec#*|} + state="$home/state" + now=$(date +%s) + run_check "$root" "$home" "$((now - 90000))" >/dev/null + add_bytes "$root/AGENTS.md" 2500 + + : > "$state/.startup-growth-check.aaaaaa" + printf 'schema\tfm-startup-growth-check-v1\n' > "$state/.startup-growth-check.bbbbbb" + touch -t 202001010000 "$state/.startup-growth-check.aaaaaa" "$state/.startup-growth-check.bbbbbb" + : > "$state/.startup-growth-check.cccccc" + + result=$(run_check "$root" "$home" "$now") + [ "$(status_part "$result")" = 0 ] || fail "sweeping evaluation failed: $(output_part "$result")" + out=$(output_part "$result") + assert_absent "$state/.startup-growth-check.aaaaaa" "an interrupted evaluation's empty temporary record was never swept" + assert_present "$state/.startup-growth-check.bbbbbb" "the sweep removed an orphan that still held record bytes" + assert_present "$state/.startup-growth-check.cccccc" "the sweep removed a concurrent evaluation's live temporary record" + assert_contains "$out" 'tracked startup surface growth AGENTS.md +2500 bytes' "the sweep cost the evaluation its retained baselines" + assert_grep $'last_eval\t'"$now" "$state/.startup-growth-check" "the sweeping evaluation did not publish its own record" +} + +test_orphan_sweep_stays_inside_the_daily_cadence() { + local rec root home state now result + rec=$(make_world orphan-sweep-gated) + root=${rec%%|*} + home=${rec#*|} + state="$home/state" + now=$(date +%s) + run_check "$root" "$home" "$now" >/dev/null + : > "$state/.startup-growth-check.aaaaaa" + touch -t 202001010000 "$state/.startup-growth-check.aaaaaa" + result=$(run_check "$root" "$home" "$((now + 200))") + [ "$(status_part "$result")" = 0 ] || fail "same-day poll failed: $(output_part "$result")" + assert_present "$state/.startup-growth-check.aaaaaa" "a same-day poll did work instead of staying gated" +} + +test_arm_and_disarm_use_authenticated_custom_check() { + local rec root home out + rec=$(make_world arm) + root=${rec%%|*} + home=${rec#*|} + out=$(FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$CHECK" arm) + assert_contains "$out" 'armed: state/startup-growth.check.sh' "arm did not announce the check shim" + assert_present "$home/state/startup-growth.check.sh" "arm did not write the check shim" + assert_present "$home/state/startup-growth.check-trust" "arm did not register trust for the check shim" + + out=$(env -u FM_HOME FM_ROOT_OVERRIDE="$root" FM_STARTUP_GROWTH_NOW=1000 "$home/state/startup-growth.check.sh" 2>&1) \ + || fail "registered check shim failed: $out" + [ -z "$out" ] || fail "registered shim baseline run should stay silent: $out" + assert_present "$home/state/.startup-growth-check" "registered shim did not run the daily check in the pinned home" + assert_absent "$root/state/.startup-growth-check" "registered shim resolved the home from the environment instead of its pinned value" + add_bytes "$root/AGENTS.md" 2500 + out=$(env -u FM_HOME FM_ROOT_OVERRIDE="$root" FM_STARTUP_GROWTH_NOW=87401 "$home/state/startup-growth.check.sh" 2>&1) \ + || fail "registered check shim failed on growth: $out" + assert_contains "$out" 'startup-growth: tracked startup surface growth AGENTS.md +2500 bytes' "registered shim did not report growth to the watcher" + + FM_ROOT_OVERRIDE="$root" FM_HOME="$home" "$CHECK" disarm >/dev/null + assert_absent "$home/state/startup-growth.check.sh" "disarm left the check shim" + assert_absent "$home/state/startup-growth.check-trust" "disarm left the trust binding" +} + +test_initial_baseline_is_silent_and_records_metadata +test_same_day_poll_does_not_touch_surfaces +test_due_growth_reports_once_and_dedupes +test_gradual_growth_below_daily_threshold_is_reported_cumulatively +test_printed_memory_growth_is_reported_without_entering_the_budget_total +test_first_content_of_an_optional_file_is_baselined_silently +test_established_baseline_survives_disappearance_and_restoration +test_secondmate_is_not_notified_about_primary_owned_shared_growth +test_budget_overrun_reports_and_separates_prompt_cost +test_secondmate_is_not_woken_about_the_primary_owned_shared_overrun +test_metadata_read_failure_keeps_the_retained_baseline +test_due_unsafe_inputs_are_reported_but_absent_optional_memory_is_not +test_findings_are_delivered_even_when_the_record_cannot_be_published +test_record_with_a_foreign_schema_marker_is_not_trusted +test_over_long_finding_set_is_capped_with_the_shared_marker +test_unknown_budget_verdict_fields_are_reported_as_unparseable +test_interrupted_evaluation_abandons_its_partial_record +test_aged_empty_orphan_records_are_swept_without_touching_live_work +test_orphan_sweep_stays_inside_the_daily_cadence +test_arm_and_disarm_use_authenticated_custom_check +test_rearming_an_unchanged_binding_does_not_replace_the_shim +test_failed_first_arm_leaves_the_home_plainly_unarmed +test_failed_rearm_keeps_the_previously_armed_shim_and_trust +pass "fm-startup-growth-check" From 2e8cd9e9179c1d852f59d3b90fb19be98f4d7bf3 Mon Sep 17 00:00:00 2001 From: Tiago Date: Tue, 6 Oct 2026 23:24:48 -0300 Subject: [PATCH 11/13] fix(bin): reopen the remote-reply continuity decision on a later break (#6708) * fix: reopen a remote-reply continuity break after repair A later break for the same route and reason was swallowed after the operator resolved the first one, because the status line matched for the life of the log. The continuity ingest now appends again when the cursor has moved or retirement has reset that episode, and an unchanged re-read still appends nothing. status_event_recorded is unchanged. Its other callers are the pending-reply escalation, which already decides its own episode, the parent-channel note append, and the remote document transfer note. * no-mistakes(review): seed continuity episode for already recorded break line * no-mistakes(document): document when a remote-reply continuity break reopens * no-mistakes(ci): The adapter now stores the continuity episode record before it appends the `blocked` line, so a failed store appends nothing. The changes are uncommitted in the worktree, in `bin/fm-procevent-remote-reply.sh` and `tests/fm-remote-reply.test.sh`. **Invariant:** a continuity `blocked` line is on the parent status log only when the episode record for that break is already stored. Only the `continuity-broken` branch of `cmd_ingest` appends that line, so the fix is at that one place. **What changed in `cmd_ingest`:** - It decides first whether the break needs a line, then stores the episode record, then appends. - If storing the record fails, it stops with "cannot record continuity episode" and appends nothing. - If the line is already on the log and no episode record exists, it stores the record and does not append. **One addition you did not ask for:** if the append fails after the record is stored, the adapter puts the earlier record back (or deletes the new one when none existed). Without that, the retry would read as the unchanged repeat and append nothing, which is the issue 6701 failure again. **Deviation from your test instruction:** the test does not use `chmod` on the cursor directory. `write_continuity_episode` runs `chmod 700` on that directory before every write, so a read-only directory is made writable again. The test instead makes `mktemp` fail for the episode's temporary file in that directory, the same way the existing receipt-failure test does. **Tests added to `tests/fm-remote-reply.test.sh`:** - Store failure: the second break exits 1, appends no `blocked` line and opens no decision. One retry after storage recovers exits 3 and appends a single line. - Append failure: with the status log read-only, the third break exits 1 and appends nothing. One retry after the log is writable exits 3 and appends a single line. **Verification:** `bash tests/fm-remote-reply.test.sh` ends with "ALL TESTS PASSED" with the fix. Against the script at commit b750c828 the same test file fails at "a continuity break appended its line before its episode was stored". `shellcheck -S warning` reports only an unused loop variable at line 667 of the test file, which this change does not touch. I ran no other test files. The Greptile Review check log could not be retrieved, so I worked from the finding text alone * fix: record a continuity break's reader position on its status line A later break at another cursor is then a different line, so the existing duplicate check appends it and reopens the decision. An unchanged re-read builds the same line and appends nothing. * fix: reopen a continuity break after an identical restore A retirement that puts the same bytes back used to rebuild the recorded line, so the later break stayed closed. The retirement count on that line makes the later break distinct. * no-mistakes(review): remove continuity match for full-prefix line without retirement count * no-mistakes(document): clarify what a continuity break status line records * fix: remove the reply cursor before recording retirement A stop between those steps must leave the count unchanged, so an unchanged continuity break still builds the same line. --- bin/fm-procevent-remote-reply.sh | 58 +++++++- docs/remote-secondmates.md | 6 + tests/fm-remote-reply.test.sh | 230 +++++++++++++++++++++++++++++++ 3 files changed, 291 insertions(+), 3 deletions(-) diff --git a/bin/fm-procevent-remote-reply.sh b/bin/fm-procevent-remote-reply.sh index 7d545c0508b..a7d82904aa7 100755 --- a/bin/fm-procevent-remote-reply.sh +++ b/bin/fm-procevent-remote-reply.sh @@ -128,6 +128,7 @@ source_id() { cursor_path() { printf '%s/%s.cursor\n' "$CURSOR_DIR" "$1"; } ingest_receipt_path() { printf '%s/%s.%s.ingested\n' "$CURSOR_DIR" "$1" "$2"; } +retirement_count_path() { printf '%s/%s.retirements\n' "$CURSOR_DIR" "$1"; } mirrored_source_path() { printf '%s/.remote-reply-mirrored-%s\n' "$STATE" "$1"; } read_cursor() { # ; sets CURSOR_OFFSET and CURSOR_HASH @@ -164,6 +165,43 @@ write_cursor() { # mv -f -- "$tmp" "$path" } +# A missing file is zero and is not created. Ingest only reads this. +# Retirement is the one writer, so a crash during ingest cannot change it. +read_retirement_count() { # ; sets RETIREMENT_COUNT + local path count lines + path=$(retirement_count_path "$1") + RETIREMENT_COUNT=0 + [ -e "$path" ] || [ -L "$path" ] || return 0 + [ -f "$path" ] && [ ! -L "$path" ] || die "reply retirement count is unsafe: $path" + lines=$(grep -c '^count=' "$path" 2>/dev/null || true) + [ "$lines" = 1 ] || die "reply retirement count is invalid: $path" + count=$(sed -n 's/^count=//p' "$path") + case "$count" in ''|*[!0-9]*) die "reply retirement count is invalid: $path" ;; esac + RETIREMENT_COUNT=$count +} + +write_retirement_count() { # + local id=$1 count=$2 path tmp + case "$count" in ''|*[!0-9]*) return 1 ;; esac + mkdir -p "$CURSOR_DIR" || return 1 + chmod 700 "$CURSOR_DIR" 2>/dev/null || true + path=$(retirement_count_path "$id") + [ ! -L "$path" ] || return 1 + tmp=$(umask 077; mktemp "$CURSOR_DIR/.retirements.XXXXXX") || return 1 + printf 'count=%s\n' "$count" > "$tmp" || { rm -f -- "$tmp"; return 1; } + chmod 600 "$tmp" || { rm -f -- "$tmp"; return 1; } + if ! mv -f -- "$tmp" "$path"; then + rm -f -- "$tmp" + return 1 + fi +} + +# Twelve characters distinguish breaks in the status line. The cursor keeps +# the full digest the reader uses. +continuity_prefix() { + printf '%.12s' "$CURSOR_HASH" +} + ingest_receipt_matches() { # local path stored actual count path=$(ingest_receipt_path "$1" "$2") @@ -544,7 +582,12 @@ cmd_ingest() { die "result does not continue the current cursor for $id" fi if [ "$class" = continuity-broken ]; then - line="blocked [key=remote-reply-continuity-$id]: remote reply continuity broke for $id ($reason)" + # The same offset, prefix, and retirement count build the same line, so a + # retry appends nothing. Retirement removes the cursor before it records + # the next count, so a later break is a new line even when the restored + # bytes match, and a stop between those steps leaves the count unchanged. + read_retirement_count "$id" + line="blocked [key=remote-reply-continuity-$id]: remote reply continuity broke for $id ($reason) at offset ${CURSOR_OFFSET} prefix $(continuity_prefix) retirements ${RETIREMENT_COUNT}" append_rc=0 if status_event_recorded "$status_file" "$line"; then append_rc=1 @@ -726,7 +769,7 @@ cmd_retire_quiesce_locked() { } cmd_retire_finalize_locked() { - local id=${1:-} force=${2:-} sid path + local id=${1:-} force=${2:-} sid path cursor validate_id "$id" [ -z "$force" ] || [ "$force" = --force ] || die "invalid retirement option: $force" sid=$(source_id "$id") @@ -742,7 +785,16 @@ cmd_retire_finalize_locked() { done fi fi - rm -f -- "$(cursor_path "$id")" + # Remove the cursor first. A stop before the count write leaves that count + # unchanged, so the same break still builds the same line. + cursor=$(cursor_path "$id") + rm -f -- "$cursor" || die "cannot remove remote reply cursor" + if [ -e "$cursor" ] || [ -L "$cursor" ]; then + die "cannot remove remote reply cursor" + fi + read_retirement_count "$id" + write_retirement_count "$id" "$((RETIREMENT_COUNT + 1))" \ + || die "cannot record remote reply retirement" rm -f -- "$CURSOR_DIR/$id".*.ingested rm -f -- "$(fm_pending_reply_remote_channel_watermark_path "$STATE" "$id")" } diff --git a/docs/remote-secondmates.md b/docs/remote-secondmates.md index cb725a7b65c..980fbceaf8c 100644 --- a/docs/remote-secondmates.md +++ b/docs/remote-secondmates.md @@ -588,6 +588,12 @@ The [process-to-event operating contract](configuration.md#process-to-event-sour The source log is never truncated or consumed. A shortened or changed prefix stops the relay and surfaces a continuity failure instead of silently resetting the cursor. +The failure appends one `blocked` line to the parent status stream, which opens a decision. +The line records the reason, the reader position (the cursor offset and the first 12 characters of the prefix hash), and the retirement count (how many times the route has been retired). +Reading the same break again, with the cursor where it was and the count unchanged, appends nothing. +A later break at a different reader position, or after another retirement, appends a new `blocked` line and opens the decision again. +A line written before that position was recorded does not match, so the next break appends the new line once. + ### SSH exit 255 and unavailable homes An SSH exit status of 255 always means transport failure or unknown remote completion. diff --git a/tests/fm-remote-reply.test.sh b/tests/fm-remote-reply.test.sh index f5e8aef0c5d..92f6d9e73d0 100755 --- a/tests/fm-remote-reply.test.sh +++ b/tests/fm-remote-reply.test.sh @@ -1024,6 +1024,15 @@ assert_absent "$PARENT/state/procevent/$SID.source" "continuity break was re-arm remote_env "$ADAPTER" ingest ios "$RESULT_TWELVE" >/dev/null 2>&1 || true [ "$(grep -cF 'blocked [key=remote-reply-continuity-ios]' "$PARENT/state/ios.status")" -eq 1 ] \ || fail "continuity replay duplicated the escalation" +first_offset=$(sed -n 's/^offset=//p' "$PARENT/state/remote-replies/ios.cursor") +first_hash=$(sed -n 's/^prefix_sha256=//p' "$PARENT/state/remote-replies/ios.cursor" | tr 'A-F' 'a-f') +first_prefix=$(printf '%.12s' "$first_hash") +assert_grep "at offset ${first_offset} prefix ${first_prefix} retirements 0" "$PARENT/state/ios.status" \ + "continuity break did not record the reader position" +assert_no_grep "prefix ${first_hash}" "$PARENT/state/ios.status" \ + "continuity break recorded the full prefix hash" +assert_absent "$PARENT/state/remote-replies/ios.retirements" \ + "a route that has never been retired gained a retirement count" status_line_at_epoch "$(grep -F 'blocked [key=remote-reply-continuity-ios]' "$PARENT/state/ios.status")" >/dev/null \ || fail "new continuity escalation has unknown emission time" if [ "${FM_TEST_EVIDENCE:-0}" = 1 ]; then @@ -1064,8 +1073,229 @@ remote_env "$ADAPTER" handle ios "$GEN" "$RESULT_TWELVE" >/dev/null 2>&1 || [ "$ || fail "pending continuity result could not be acknowledged after retirement refusal" remote_env "$ADAPTER" retire ios >/dev/null assert_absent "$PARENT/state/remote-replies/ios.cursor" "adapter retirement left its cursor" +recorded_retirements=$(cat "$PARENT/state/remote-replies/ios.retirements" 2>/dev/null || true) +[ "$recorded_retirements" = count=1 ] \ + || fail "adapter retirement did not record its count (got: ${recorded_retirements:-absent})" assert_absent "$PARENT/state/remote-replies/ios.caught-up" \ "adapter retirement left a caught-up watermark a later route could inherit" pass "remote reply retirement quiesces and refuses unhandled captured results" +# Empty the remote log under the committed cursor and handle the break the +# next blocking source reports. Sets RESULT_BREAK. +break_repaired_route() { #