From 6789876442d0fb6da9f70d86399a2930c5073ae2 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Thu, 13 Aug 2026 23:30:31 -0700 Subject: [PATCH 1/9] chore: ignore scratchpad/ at the repo root (#2359) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index cae904c651..27c23e4f53 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ projects/ state/ data/ +scratchpad/ .no-mistakes/ .lavish/ .fm-secondmate-home From f1a4af426d7199c1781bc91ccd143b8e1f732d10 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:32:23 -0700 Subject: [PATCH 2/9] fix(ci): fail hung Herdr behavior runs in 20 minutes (#2413) A wedged family-run step was occupying the runner until the 75-minute job cap; bound that step so cleanup and timing artifacts still upload. --- .github/workflows/ci.yml | 9 +++++++-- docs/fm-test-portable-shards.md | 11 ++++++----- tests/fm-test-run.test.sh | 35 +++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 297d70ceeb..5495ec4494 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -170,8 +170,10 @@ jobs: tests-herdr: name: Behavior tests (Herdr) runs-on: ubuntu-latest - # Real Herdr is slower than the portable suite; this is a hang tripwire, - # not the expected healthy end of the lane (estimate 15-40 min first cut). + # Healthy runs finish around 7 minutes. This job cap is a last-resort hang + # tripwire, not the expected end of the lane. The family-run step owns the + # tighter bound so a wedged suite fails fast with always() cleanup and + # timing artifacts still uploaded (docs/fm-test-portable-shards.md). timeout-minutes: 75 steps: - uses: actions/checkout@v6 @@ -252,6 +254,9 @@ jobs: mkdir -p "$RUNNER_TEMP/fm-herdr" bin/fm-herdr-ci-cleanup.sh snapshot "$RUNNER_TEMP/fm-herdr/sessions-before.json" - name: Run real-Herdr family (serial, required) + # Comfortably above the ~7 min healthy wall and far below the 75 min + # job backstop. A hang must fail this step so cleanup still runs. + timeout-minutes: 20 run: | set -eu mkdir -p "$RUNNER_TEMP/fm-test" diff --git a/docs/fm-test-portable-shards.md b/docs/fm-test-portable-shards.md index 5268627c2a..5cf681a501 100644 --- a/docs/fm-test-portable-shards.md +++ b/docs/fm-test-portable-shards.md @@ -105,10 +105,11 @@ Portable shards, each portable serial shard, and the Herdr lane upload runner-ge ## Timeouts -| Job | timeout-minutes | Rationale | -|---|---:|---| -| portable parallel 1/2 | 10 | The measured shard sums are about three minutes and the timeout is a hang tripwire. | -| portable serial 1-4 | 15 | Each balanced shard is about five minutes, leaving roughly 3x hang-tripwire margin. | -| Herdr | 40 | The real-Herdr lane keeps its dedicated timeout. | +| Lane | Bound | Rationale | +|---|---|---| +| portable parallel 1/2 | job `timeout-minutes: 10` | The measured shard sums are about three minutes and the timeout is a hang tripwire. | +| portable serial 1-4 | job `timeout-minutes: 15` | Each balanced shard is about five minutes, leaving roughly 3x hang-tripwire margin. | +| Herdr | family-run step `timeout-minutes: 20`; job `timeout-minutes: 75` backstop | Healthy runs finish around 7 minutes, so the step bound is the hang tripwire (cleanup and timing artifacts still upload) while the job cap stays a last-resort backstop. | Timeouts are hang tripwires rather than expected healthy durations. +`.github/workflows/ci.yml` owns the exact numbers. diff --git a/tests/fm-test-run.test.sh b/tests/fm-test-run.test.sh index 21bdd69ba5..8fe26e6f47 100755 --- a/tests/fm-test-run.test.sh +++ b/tests/fm-test-run.test.sh @@ -627,6 +627,40 @@ SH pass "jobs scheduler runs proven scripts; failure propagates; non-proven refused" } +test_herdr_ci_family_run_has_a_step_timeout() { + # The required Herdr lane's hang tripwire is the family-run *step* bound, not + # the 75-minute job cap. Parse the workflow as YAML so nested `with.name` + # artifact keys cannot masquerade as the step contract. + command -v ruby >/dev/null 2>&1 \ + || fail "ruby is required to parse .github/workflows/ci.yml as YAML" + local json job_timeout step_timeout + json=$(ruby -ryaml -rjson -e ' +doc = YAML.load_file(ARGV[0]) +job = doc.fetch("jobs").fetch("tests-herdr") +step = job.fetch("steps").find { |s| + s.is_a?(Hash) && s["name"] == "Run real-Herdr family (serial, required)" +} +raise "missing family-run step" if step.nil? +raise "family-run step has no timeout-minutes" unless step.key?("timeout-minutes") +puts JSON.generate( + "job_timeout" => job.fetch("timeout-minutes"), + "step_timeout" => step.fetch("timeout-minutes") +) +' "$ROOT/.github/workflows/ci.yml") \ + || fail "could not parse tests-herdr timeouts from ci.yml" + job_timeout=$(python3 -c 'import json,sys; print(json.load(sys.stdin)["job_timeout"])' <<<"$json") \ + || fail "could not read job timeout from parsed workflow" + step_timeout=$(python3 -c 'import json,sys; print(json.load(sys.stdin)["step_timeout"])' <<<"$json") \ + || fail "could not read step timeout from parsed workflow" + [ "$job_timeout" = 75 ] \ + || fail "tests-herdr job backstop must stay 75 minutes, got $job_timeout" + [ "$step_timeout" = 20 ] \ + || fail "family-run step timeout must be 20 minutes, got $step_timeout" + [ "$step_timeout" -lt "$job_timeout" ] \ + || fail "family-run step timeout must be below the job backstop" + pass "Herdr CI family-run step times out at 20 min under a 75 min job backstop" +} + test_aggregate_json() { local tmp a b tmp=$(mktemp -d "${TMPDIR:-/tmp}/fm-test-run-aggjson.XXXXXX") @@ -685,4 +719,5 @@ test_portable_serial_shards_partition_the_serial_lane test_portable_serial_shard_lane_refusals test_jobs_requires_proven_isolated test_jobs_parallel_scheduler_and_failure_propagation +test_herdr_ci_family_run_has_a_step_timeout test_aggregate_json From 7a3259e5bca780a53ace49d77d086c89536f6f15 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:10:07 -0700 Subject: [PATCH 3/9] fix: keep the public promise reachable when work is routed to a second mate (#2457) The lightweight Relay follow-up link lives in the answering home's own state/.meta, so it can only bind work that home owns. When a Relay-linked request is routed to a second mate, the task record lives in the second mate's home, fm-x-link.sh failed with a bare "no such task ...meta", and nothing else picked the promise up: only the soft acknowledgement was ever posted. The typed promised-final path already supports --work-home secondmate:; the playbook simply never chose it. - fmx-respond now states the routing rule crisply: a task in this home takes the lightweight link, and second-mate-routed work takes a promised-final commitment bound to that home, registered up front with the brief command carried into the routed worker's instructions. - fm-x-link.sh refuses a task with no local record by naming the registered second mate whose home actually holds it and printing the promised-final registration command, with the exact --work-home when the match is unambiguous. A home with no registered second mates keeps the plain error. - fm-backlog-handoff.sh reports, after a successful move, any moved key that still owes a public reply bound to main/, since that binding no longer names the home owning the work. The move itself is never blocked. Docs and the secondmate handoff prose follow the same rule. Tests cover the refusal, its scoping, the unchanged local-link path, and both handoff outcomes at the script boundary. --- .agents/skills/fmx-respond/SKILL.md | 29 ++++-- .../skills/secondmate-provisioning/SKILL.md | 2 + bin/fm-backlog-handoff.sh | 30 +++++- bin/fm-x-link.sh | 56 +++++++++++ docs/architecture.md | 1 + docs/configuration.md | 1 + tests/fm-backlog-handoff.test.sh | 95 +++++++++++++++++++ tests/fm-x-mode.test.sh | 72 ++++++++++++++ 8 files changed, 277 insertions(+), 9 deletions(-) diff --git a/.agents/skills/fmx-respond/SKILL.md b/.agents/skills/fmx-respond/SKILL.md index fd53c0ccc0..4b8e4b0e96 100644 --- a/.agents/skills/fmx-respond/SKILL.md +++ b/.agents/skills/fmx-respond/SKILL.md @@ -51,11 +51,18 @@ How the reply lands depends on whether the work finishes during this turn: - **Work that spawns a real, longer-running job** (dispatching a crewmate, a scout investigation, a ship task) cannot report an outcome yet, so it follows **acknowledge first -> act -> follow up on completion**: 1. **Acknowledge first.** Post an immediate, public-safe reply that you have the captain's order and are on it (the normal answer endpoint, via `bin/fm-x-reply.sh`). This is the legitimate, work-backed version of "aye, will do": it is paired with actually starting the work in the same turn, never a promise left empty. 2. **Act.** Dispatch the work through the normal lifecycle right away. - 3. **Link it for the follow-up, before clearing the inbox.** Associate the spawned task with this mention so completion follow-ups can be posted later: `bin/fm-x-link.sh ` (records the request id, a timestamp, a follow-up counter, and reply platform/budget context). - Do this right after the task is spawned, and always **before** removing the inbox file (step 2f). - Linking before cleanup lets `bin/fm-x-link.sh` copy the context directly from the inbox, while the durable per-request context recorded by the poll preserves it independently for delayed and concurrent follow-ups. - The exact resolution and fail-safe posting contract is owned by `docs/configuration.md`. - If a recovery respawns the same relay request onto a successor task, relink with the paired `--carry-count --carry-ts ` flags plus any prior `x_platform=` and `x_reply_max_chars=` as `--carry-platform --carry-max ` so the successor keeps the consumed follow-up count, original 7-day window, and reply split budget. + 3. **Bind the follow-up to wherever the work actually lives, before clearing the inbox.** + **The decision rule: work that stays in this home takes the lightweight link; work routed to a second mate takes a promised-final commitment bound to that second mate's home.** + There is no third option and no fallback between them - each mechanism can only reach the home it was built for, so choosing the wrong one orphans the public promise. + - **Local task (this home spawned it):** `bin/fm-x-link.sh ` (records the request id, a timestamp, a follow-up counter, and reply platform/budget context). + Do this right after the task is spawned, and always **before** removing the inbox file (step 2f). + Linking before cleanup lets `bin/fm-x-link.sh` copy the context directly from the inbox, while the durable per-request context recorded by the poll preserves it independently for delayed and concurrent follow-ups. + The exact resolution and fail-safe posting contract is owned by `docs/configuration.md`. + If a recovery respawns the same relay request onto a successor task, relink with the paired `--carry-count --carry-ts ` flags plus any prior `x_platform=` and `x_reply_max_chars=` as `--carry-platform --carry-max ` so the successor keeps the consumed follow-up count, original 7-day window, and reply split budget. + - **Second-mate-routed work (the request's project or domain belongs to a registered second mate, so the work is or will be routed there):** the link cannot be used at all. + It writes into this home's own `state/.meta`, and a routed task's record lives in the second mate's home, so `bin/fm-x-link.sh` refuses and points you back here. + Register a **typed promised-final commitment bound to that home** up front instead - see "Promised final replies" below for the exact commands - and put its `bin/fm-public-followup.sh brief ` output into the routed worker's instructions so the terminal result comes back as typed data. + Do this in the same turn as the acknowledgement, before routing, so the promise is durable state from the moment it is made. 4. **Follow up on genuine milestones, sparingly.** Firstmate gets up to **three** follow-ups per mention, within a 7-day window, chained in the same thread - spend them only on changes the captain would actually want to hear about (e.g. investigation done and a build started, work shipped or ready, or the task failing), never on routine internal churn. A task without a promised-final commitment posts its final outcome - shipped / reported / merged / failed - with `--final`, which clears the link regardless of how many follow-ups remain. A typed promised-final commitment uses the deterministic consumer instead. That posting happens on the task's milestone and completion wakes (see "Completion follow-up" below), not this turn. @@ -142,9 +149,10 @@ Treat `state/x-inbox/` as the source of truth and process **every** file you fin When in doubt between an instruction and a question, do the smallest safe lifecycle step the request implies; when in doubt between a question and bare politeness, lean toward skipping - a needless reply is noise on a public bot. c. **Act on an actionable request through the normal lifecycle.** Treat it exactly as a captain prompt typed in session: run ordinary intake (resolve the project), then file the backlog item, dispatch a crewmate, start a scout, or ship through the gate - whatever the request calls for. **Destructive, irreversible, or security-sensitive work is the exception** (Relay is a public, relayed channel and does not carry full in-session trust): do not execute it from the mention. Flag it to the captain through the normal trusted channel first - the same carve-out as `yolo` (AGENTS.md §1, §7) - act only on the captain's word, and in step 2d say only that it has been flagged for the captain. - **If the request spawned a real, longer-running task** (you ran `bin/fm-spawn.sh`), link that task to this mention so milestone and completion follow-ups can be posted: `bin/fm-x-link.sh `. + **If the request spawned a real, longer-running task in THIS home** (you ran `bin/fm-spawn.sh` here), link that task to this mention so milestone and completion follow-ups can be posted: `bin/fm-x-link.sh `. **Link here, in step 2c, before the step 2f inbox cleanup** - `bin/fm-x-link.sh` can copy both the mention's reply platform and explicit budget from the still-present inbox payload without a relay lookup. If that local context is incomplete it uses the durable resolution contract in `docs/configuration.md` and warns loudly, while the follow-up path refuses to post unless both values can be resolved authoritatively. + **If intake routes the work to a second mate instead**, do not reach for the link: register the typed promised-final commitment bound to `secondmate:` and brief the routed worker with its reporting command (step 3 of "acknowledge first, act, then follow up on completion", with the commands in "Promised final replies"). Then step 2d's reply is an **acknowledgement** ("on it, captain"), and genuine milestone updates plus the final outcome come later as follow-ups (see "Completion follow-up" below), with the terminal one posted using `--final` when no typed promised-final commitment exists. If the work completed in this turn (a backlog item filed, a question answered), there is no task to link and step 2d reports the outcome directly. d. **Compose the reply.** For a **question**, answer `.text` from the fleet state gathered in step 1. For an **actionable request that completed now**, report the outcome of step 2c (what was done, or - for escalated work - that it has been flagged for the captain). For an **actionable request that spawned a linked task**, acknowledge that you have the order and are on it - milestone updates and the final outcome follow later as completion follow-ups, so do not promise a result you do not yet have. Either way keep it short, in firstmate's voice, and public-safe. @@ -216,13 +224,18 @@ Never carry one in your head: the moment you promise a specific outcome in a pub This section is the sole owner of that procedure. `tasks-axi public-followup --help` owns the typed obligation, its states, and its file contracts; `bin/fm-public-followup.sh --help` owns firstmate's flags; do not restate either here. -**When you promise a final:** +This is also the **only** mechanism that reaches work outside this home. +The lightweight link of step 3 writes into this home's own task record, so it can never bind a second mate's task; `--work-home secondmate:` here can. +So treat second-mate-routed Relay work as a promised final by construction: the acknowledgement you just posted **is** the promise, and there is no other way to keep it. + +**When you promise a final (including every Relay request whose work is routed to a second mate):** 1. Create the typed obligation with `tasks-axi public-followup add` and bind the work with `bind-work`, keeping the public-safe summary and the opaque thread binding in the obligation and the full request context where the poll already put it. 2. Register it with `bin/fm-public-followup.sh register --relation --work-home > --work-id --generation `. This is what makes the commitment reconcilable without you. 3. Put `bin/fm-public-followup.sh brief ` output straight into the worker's brief. It prints the exact reporting command for that binding. + When the work is routed to a second mate rather than spawned here, the routed item's own note carries that same output, so it survives the routing and reaches whoever ends up doing the work. Never ask a worker to find the thread or post the reply: only this home holds the relay consent and the thread binding. **When work reports back, or on a `public-followup ...` check wake, or when the session-start digest lists a public commitment:** @@ -247,7 +260,7 @@ Treat a commitment as kept only after a validated posted receipt or an explicit ## Notes - The direct author is always your own captain (owner-only routing), and in live mode you answer and act on eligible requests **autonomously**: enabling Relay is the captain's standing authorization, so never ask the captain before posting and never hold a worthwhile reply for a chat-side OK. For reply-worthy mentions, dry-run (`FMX_DRY_RUN`) is the only non-posting path; pure acknowledgments use the relay dismiss path instead. -- An actionable mention is **acted on** through the normal lifecycle (intake, backlog, dispatch, investigate, ship), not merely replied to. Work that finishes now gets one outcome reply; work that spawns a real task gets an **acknowledgement now** plus up to three **completion follow-ups** over time, ending with a `--final` one when no typed promised-final commitment exists (link the task with `bin/fm-x-link.sh` so those follow-ups can post). A reply alone, with no work behind an actionable ask, is the bug to avoid. +- An actionable mention is **acted on** through the normal lifecycle (intake, backlog, dispatch, investigate, ship), not merely replied to. Work that finishes now gets one outcome reply; work that spawns a real task gets an **acknowledgement now** plus up to three **completion follow-ups** over time, ending with a `--final` one when no typed promised-final commitment exists. Bind those follow-ups by where the work lives: a task in this home takes `bin/fm-x-link.sh`, and work routed to a second mate takes a promised-final commitment registered with `--work-home secondmate:`, which is the only mechanism that reaches another home. A reply alone, with no work behind an actionable ask, is the bug to avoid. - Destructive, irreversible, or security-sensitive asks are flagged to the captain through the trusted channel first and never run straight from a mention; the public reply says only that it has been flagged. - One answered mention = one reply (plus up to three completion follow-ups for a spawned task, spent only on genuine milestones); a skipped mention posts no reply but is **dismissed at the relay** (`bin/fm-x-dismiss.sh`) so the relay drops it rather than re-offering it (which would otherwise churn every poll and end in an "offline" auto-reply). A single wake may cover several pending mentions - drain them all. - Conversations: `in_reply_to` carries the parent post and optional `in_reply_to_chain` carries the surrounding transcript for continuity; a pure acknowledgment with nothing to answer is dismissed at the relay and skipped, not replied to. The relay already guards against self-replies and caps replies per conversation, so you only judge "is there something to answer here?". diff --git a/.agents/skills/secondmate-provisioning/SKILL.md b/.agents/skills/secondmate-provisioning/SKILL.md index f796f37fd8..0ff1aa3e63 100644 --- a/.agents/skills/secondmate-provisioning/SKILL.md +++ b/.agents/skills/secondmate-provisioning/SKILL.md @@ -198,6 +198,8 @@ It refuses a selected item with a single-space or tab-indented continuation rath It accepts in-scope `## Queued` entries only and refuses `## In flight` and historical `## Done` entries. Done records stay with their home for pruning or archiving. It is idempotent; an item already in the secondmate backlog is skipped. +After a successful move it warns for any moved key that still owes a public relay reply bound to `main/`, because that binding no longer names the home owning the work; rebind the commitment to `secondmate:` through the `fmx-respond` promised-final procedure, which owns those commands. +That same rule governs routing generally: a Relay-linked request whose work goes to a secondmate cannot use the home-local mention link at all and needs a promised-final commitment bound to that secondmate's home. It refuses any destination that is not a genuine seeded firstmate home with safe operational directories and a matching `.fm-secondmate-home` marker, so a move can never land in a project. Do not hand off `local-only` items. diff --git a/bin/fm-backlog-handoff.sh b/bin/fm-backlog-handoff.sh index 3a59f4b132..f773655334 100755 --- a/bin/fm-backlog-handoff.sh +++ b/bin/fm-backlog-handoff.sh @@ -24,7 +24,11 @@ # archiving; # - the multi-key classification and idempotent per-key reporting: a key # already present in the secondmate backlog is reported and skipped, and if -# any key matches neither backlog nothing is moved. +# any key matches neither backlog nothing is moved; +# - warning, after a successful move, when a moved key still owes a public +# relay reply bound to main/, because that binding no longer names the +# home that owns the work. The move is not blocked: rebinding the commitment +# to secondmate: is a relay-side decision the caller makes. # # What `tasks-axi mv ... --to ` owns: moving each full item BLOCK # byte-exact (header, body lines, blank separators, and indented pseudo-headings @@ -266,6 +270,28 @@ seed_backlog_scaffold() { # [ -f "$1" ] || printf '## In flight\n\n## Queued\n\n## Done\n' > "$1" } +# A public commitment made through the relay binds its work by home AND id, so an +# item that leaves this home takes that binding out of sync: reconciliation would +# still look for main/ while the work now lives in the secondmate's home. +# The move itself stays safe and is never blocked - rebinding is a relay-side +# decision the caller owns - but this is the one moment the staleness is +# detectable, so report it loudly instead of letting the promise go quiet. +# A home that never opted into the relay pays one presence check per key here. +warn_stale_public_commitments() { # ... + local id=$1 key out rc + shift + for key in "$@"; do + rc=0 + out=$("$SCRIPT_DIR/fm-public-followup.sh" guard-work main "$key" 2>/dev/null) || rc=$? + [ "$rc" -ne 0 ] || continue + [ -z "$out" ] || printf '%s\n' "$out" >&2 + printf 'warning: %s still owes a public reply bound to main/%s; rebind it to secondmate:%s (tasks-axi public-followup bind-work, then bin/fm-public-followup.sh register --relation --work-home secondmate:%s --work-id %s --generation ) or the promised reply will be reconciled against work this home no longer owns.\n' \ + "$key" "$key" "$id" "$id" "$key" >&2 + done + # Reporting never changes the handoff's own success: the move already landed. + return 0 +} + outbox_item_count() { # awk '/^- \[[ x]\] / { count++ } END { print count + 0 }' "$1" } @@ -410,6 +436,7 @@ remote_handoff() { # remote_deliver_outbox "$id" "$outbox" || return 1 echo "handed off ${#requested[@]} item(s) to remote secondmate $id: ${requested[*]}" [ "${#already[@]}" -eq 0 ] || echo " already staged (recovered): ${already[*]}" + warn_stale_public_commitments "$id" "${requested[@]}" } with_remote_route_locks() { # @@ -576,3 +603,4 @@ echo " into $SUB_BACKLOG" if [ "${#ALREADY[@]}" -gt 0 ]; then echo " already present (skipped): ${ALREADY[*]}" fi +warn_stale_public_commitments "$ID" "${TO_MOVE[@]}" diff --git a/bin/fm-x-link.sh b/bin/fm-x-link.sh index b65415583d..13b881c0c7 100755 --- a/bin/fm-x-link.sh +++ b/bin/fm-x-link.sh @@ -33,6 +33,14 @@ # fm-x-followup.sh on the task's captain-relevant wakes. The meta read/write # lives in fm-x-lib.sh. # +# THE LINK IS HOME-LOCAL BY CONSTRUCTION: it lives in this home's +# state/.meta, so it can only bind work this home owns. Work routed to a +# secondmate lives in that secondmate's home and has no meta here, so a link is +# impossible and the public promise would be silently orphaned. When the task has +# no local meta, this refuses with the promised-final path (bin/fm-public-followup.sh +# register --work-home secondmate:) named, and names the secondmate home the +# task was actually found in whenever a registered LOCAL route holds it. +# # Both ids are relay/firstmate slugs that compose a filename, so they are guarded # against path traversal even though they come from trusted callers. set -u @@ -41,12 +49,15 @@ 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}" +DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" # shellcheck source=bin/fm-x-lib.sh . "$SCRIPT_DIR/fm-x-lib.sh" # shellcheck source=bin/fm-wake-lib.sh . "$SCRIPT_DIR/fm-wake-lib.sh" # shellcheck source=bin/fm-pr-lib.sh . "$SCRIPT_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-secondmate-registry-lib.sh +. "$SCRIPT_DIR/fm-secondmate-registry-lib.sh" usage() { echo "usage: fm-x-link.sh [--carry-count --carry-ts [--carry-platform ] [--carry-max ]]" >&2 @@ -121,9 +132,54 @@ case "$RID" in ''|.*|*[!A-Za-z0-9._-]*) echo "fm-x-link: unsafe request_id: $RID" >&2; exit 2 ;; esac +# Scan this home's registered secondmates for a task record with this id. +# ROUTE_MATCHES gets every LOCAL secondmate whose seeded home actually holds +# state/.meta; ROUTE_REGISTERED is 1 whenever any secondmate is registered at +# all, which covers remote routes whose homes cannot be inspected from here. A +# home with no registry at all learns nothing new and keeps the plain error. +ROUTE_MATCHES= +ROUTE_REGISTERED=0 +scan_secondmate_routes() { # + local id=$1 reg="$DATA/secondmates.md" line home marker + [ -f "$reg" ] && [ ! -L "$reg" ] || return 0 + while IFS= read -r line || [ -n "$line" ]; do + case "$line" in '- '*) ;; *) continue ;; esac + secondmate_registry_parse_line "$line" || continue + ROUTE_REGISTERED=1 + [ "$SECONDMATE_REGISTRY_REMOTE" -eq 0 ] || continue + home=$SECONDMATE_REGISTRY_HOME + case "$home" in /*) ;; *) continue ;; esac + home=$(CDPATH='' cd -- "$home" 2>/dev/null && pwd -P) || continue + [ -f "$home/.fm-secondmate-home" ] && [ ! -L "$home/.fm-secondmate-home" ] || continue + marker=$(sed -n '1p' "$home/.fm-secondmate-home" 2>/dev/null) + [ "$marker" = "$SECONDMATE_REGISTRY_ID" ] || continue + [ -f "$home/state/$id.meta" ] && [ ! -L "$home/state/$id.meta" ] || continue + ROUTE_MATCHES="${ROUTE_MATCHES:+$ROUTE_MATCHES }$SECONDMATE_REGISTRY_ID" + done < "$reg" +} + META="$STATE/$ID.meta" if [ ! -f "$META" ]; then echo "fm-x-link: no such task: state/$ID.meta" >&2 + scan_secondmate_routes "$ID" + if [ -n "$ROUTE_MATCHES" ]; then + printf 'fm-x-link: %s is a second mate task (found in: %s), so this home cannot link it - a link only binds work whose record lives here.\n' \ + "$ID" "$ROUTE_MATCHES" >&2 + elif [ "$ROUTE_REGISTERED" -eq 1 ]; then + printf 'fm-x-link: this home has registered second mates and no record of %s, so the work may be routed to one - a link only binds work whose record lives here.\n' \ + "$ID" >&2 + fi + if [ -n "$ROUTE_MATCHES" ] || [ "$ROUTE_REGISTERED" -eq 1 ]; then + # One unambiguous match is worth naming exactly, so the pointer can be run + # as printed instead of re-derived. + ROUTE_HOME_ARG='secondmate:' + case "$ROUTE_MATCHES" in + ''|*' '*) ;; + *) ROUTE_HOME_ARG="secondmate:$ROUTE_MATCHES" ;; + esac + printf 'fm-x-link: bind the public promise through the promised-final path instead: tasks-axi public-followup add + bind-work, then bin/fm-public-followup.sh register --relation --work-home %s --work-id %s --generation , and put the bin/fm-public-followup.sh brief command into the routed worker instructions.\n' \ + "$ROUTE_HOME_ARG" "$ID" >&2 + fi exit 1 fi diff --git a/docs/architecture.md b/docs/architecture.md index afca3208d7..4b3f0b4492 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -266,6 +266,7 @@ When a reply has a real visual artifact, `--image ` attaches one local PNG Actionable reversible requests run through firstmate's normal intake, backlog, dispatch, investigation, or ship lifecycle. Work that completes in the answering turn gets one outcome reply. Work that spawns a longer-running task gets an acknowledgement reply first; `bin/fm-x-link.sh` records `x_request=`, `x_request_ts=`, `x_followups=0`, and optional reply-platform context in that task's `state/.meta`, while durable per-request context preserves the original platform and budget independently of task links and inbox cleanup. +That link therefore reaches only work whose task record lives in the answering home; work routed to a secondmate is bound instead by a typed promised-final commitment registered with `--work-home secondmate:`, and `bin/fm-x-link.sh` refuses a non-local task with that path named rather than leaving the public promise unbound. Later milestone wakes use `bin/fm-x-followup.sh` to post up to three public-safe follow-ups through the relay's `connector/followup` endpoint, ending with a `--final` one for ordinary Relay-linked work. A typed promised-final commitment owns its terminal reply through `bin/fm-public-followup.sh`; after its receipt is validated, `bin/fm-x-followup.sh --clear ` removes any legacy link without posting another reply. The [Relay configuration reference](configuration.md#relay-env) owns the exact context retention, platform-resolution, and fail-safe posting contract. If recovery relinks the same relay request onto a successor task, `fm-x-link.sh --carry-count --carry-ts --carry-platform --carry-max ` preserves the consumed follow-up count, original 7-day window, and reply split budget instead of granting a fresh local budget or falling back to the wrong platform. diff --git a/docs/configuration.md b/docs/configuration.md index 78ae19bd56..e0466d80d3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -388,6 +388,7 @@ That link stores optional reply-platform context so Discord-originated follow-up Platform/budget resolution is layered and independent of the task link: a per-axis `FMX_REPLY_PLATFORM` / `FMX_REPLY_MAX_CHARS` override (how `bin/fm-x-followup.sh` passes a recorded link's context) wins. For either axis without an override, `bin/fm-x-lib.sh:fmx_resolve_reply_context` owns the source order: the durable per-request registry is consulted first, then the still-present inbox payload, then - for a follow-up posted live by request_id - an authoritative relay lookup via `POST /connector/request-context` (`{request_id}` in, `{platform, reply_max_chars}` back). This is what keeps a delayed request-id follow-up on the original platform's budget even after the inbox is drained and with no task link surviving; the relay step is confined to the live follow-up path so the answer path and every dry-run stay network-free. +The link is home-local by construction, because it lives in that home's own `state/.meta`: work routed to a secondmate has no record here, so `bin/fm-x-link.sh` refuses it, names the registered secondmate home the task was found in when it can, and points at the promised-final path (`bin/fm-public-followup.sh register ... --work-home secondmate:`), which is the only follow-up mechanism that binds work in another home. `bin/fm-x-link.sh` follows the same ordering when recording a fresh link's context and requires `jq`; its request-context lookup is best-effort: no token or `curl`; a non-2xx response; an unresolved response; or a relay version without that endpoint leaves the context unknown. In that case the link is still recorded but `bin/fm-x-link.sh` prints a loud warning; and when either a follow-up's platform or explicit budget cannot be authoritatively resolved from any source, `bin/fm-x-reply.sh` refuses it (fail-safe exit 8) rather than posting with a local default - firstmate holds and retries it once both values are recoverable. Fresh links start with `x_followups=0` and the current timestamp; when relinking the same relay request onto a successor task, pass paired `--carry-count --carry-ts ` flags plus any prior `x_platform=` and `x_reply_max_chars=` as `--carry-platform --carry-max ` so the successor preserves the already-consumed follow-up count, original 7-day window, and reply split budget. diff --git a/tests/fm-backlog-handoff.test.sh b/tests/fm-backlog-handoff.test.sh index 2efd8dd3d5..94b50f8a64 100755 --- a/tests/fm-backlog-handoff.test.sh +++ b/tests/fm-backlog-handoff.test.sh @@ -55,6 +55,99 @@ assert_block_equals() { fi } +# seed_public_commitment : the intake +# half of a promised public reply - the typed obligation, its bound work, and +# this home's registration - so a later handoff can be observed against a real +# unresolved commitment rather than a stub. +seed_public_commitment() { + local home=$1 obligation=$2 work_home=$3 work_id=$4 + printf 'FMX_PAIRING_TOKEN=test-token\n' > "$home/.env" + cp "$ROOT/.tasks.toml" "$home/.tasks.toml" + jq -n '{request_id:"req-handoff", platform:"x", + context_binding:{version:"ctx1", value:"ctx1_req-handoff"}, + public_safe_summary:"looking into the sign-in redirect", + received_at:"2026-07-30T10:00:00Z", + followup_expires_at:"2026-08-06T10:00:00Z", + reservation_expires_at:"2026-08-06T10:00:00Z"}' > "$home/request.json" + jq -n '{type:"pr-merged", project:"alpha", + required_deliverables:["pr_url"], completion_policy:"all-required"}' \ + > "$home/expected.json" + jq -n --arg h "$work_home" --arg w "$work_id" \ + '{relation_id:"rel-code", work_ref:{home_id:$h, task_id:$w}, + role:"fulfills", required:true, generation:1}' > "$home/relation.json" + (cd "$home" && tasks-axi public-followup add "$obligation" \ + --request-context-file "$home/request.json" --purpose promised-final \ + --expected-final-file "$home/expected.json" --expires-at 2026-10-01T00:00:00Z) >/dev/null \ + || fail "could not create the public commitment" + (cd "$home" && tasks-axi public-followup bind-work "$obligation" \ + --relation-file "$home/relation.json") >/dev/null \ + || fail "could not bind work to the public commitment" + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$home" "$ROOT/bin/fm-public-followup.sh" register \ + "$obligation" --relation rel-code --work-home "$work_home" --work-id "$work_id" \ + --generation 1 >/dev/null \ + || fail "could not register the public commitment" +} + +# A public promise binds its work by home AND id. Handing that work to a +# secondmate leaves the binding naming a home that no longer owns it, which used +# to go unnoticed until the promised reply was never delivered. The move itself +# stays safe; the staleness must be reported at the moment it is created. +test_handoff_warns_when_a_moved_item_still_owes_a_public_reply() { + local home="$TMP_ROOT/pf-stale-main" + local sub="$TMP_ROOT/pf-stale-sub" + command -v jq >/dev/null 2>&1 || { echo "skip: jq not found (required by the public-commitment guard)"; return 0; } + setup_homes "$home" "$sub" + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] promised-item - fix the sign-in redirect (repo: alpha) +- [ ] plain-item - unrelated queued work (repo: alpha) + +## Done +EOF + seed_public_commitment "$home" pf-handoff main promised-item + + local out rc=0 + out=$(FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design promised-item plain-item 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "handoff must still succeed while reporting the stale binding: $out" + assert_contains "$out" "handed off 2 item(s)" "the move itself must still be reported" + assert_grep 'promised-item' "$sub/data/backlog.md" "the promised item did not reach the secondmate backlog" + assert_contains "$out" "promised-item still owes a public reply bound to main/promised-item" \ + "the stale public-commitment binding was not reported" + # The report must come from a genuinely unresolved commitment, not from a state + # the guard merely could not verify. + assert_contains "$out" "public commitment pf-handoff is still" \ + "the report did not carry the unresolved commitment the guard actually found" + assert_contains "$out" "--work-home secondmate:design" \ + "the report did not name the rebinding that keeps the promise reachable" + case "$out" in + *"plain-item still owes"*) fail "an item with no public commitment must not be reported" ;; + esac + + pass "handoff reports a moved item whose public commitment still binds this home" +} + +# A home that never opted into the relay must pay nothing and say nothing here. +test_handoff_is_silent_about_public_commitments_without_the_relay() { + local home="$TMP_ROOT/pf-silent-main" + local sub="$TMP_ROOT/pf-silent-sub" + setup_homes "$home" "$sub" + cat > "$home/data/backlog.md" <<'EOF' +## Queued +- [ ] quiet-item - ordinary queued work (repo: alpha) + +## Done +EOF + + local out rc=0 + out=$(FM_HOME="$home" "$ROOT/bin/fm-backlog-handoff.sh" design quiet-item 2>&1) || rc=$? + [ "$rc" -eq 0 ] || fail "handoff failed in a relay-free home: $out" + case "$out" in + *"public reply"*) fail "a relay-free home must not mention public commitments: $out" ;; + esac + assert_grep 'quiet-item' "$sub/data/backlog.md" "the item did not reach the secondmate backlog" + pass "handoff says nothing about public commitments in a relay-free home" +} + test_body_moves_when_followed_by_another_item() { local home="$TMP_ROOT/body-next-item-main" local sub="$TMP_ROOT/body-next-item-sub" @@ -550,5 +643,7 @@ test_noncanonical_indented_continuations_refuse_without_changes test_indented_heading_is_not_section_boundary test_registry_home_with_pre_home_parentheses test_registry_home_missing_field_fails_cleanly +test_handoff_warns_when_a_moved_item_still_owes_a_public_reply +test_handoff_is_silent_about_public_commitments_without_the_relay echo "ALL TESTS PASSED" diff --git a/tests/fm-x-mode.test.sh b/tests/fm-x-mode.test.sh index 7a5eb2b032..0c011002b3 100755 --- a/tests/fm-x-mode.test.sh +++ b/tests/fm-x-mode.test.sh @@ -2487,6 +2487,75 @@ test_link_rejects_unsafe_and_missing() { pass "fm-x-link rejects unsafe ids, missing meta, and missing arguments" } +# A home with no secondmates at all learns nothing from the registry, so its +# missing-task error must stay the plain one instead of routing every typo at a +# mechanism that does not apply. +test_link_missing_task_without_secondmates_stays_plain() { + local home err rc + home="$TMP_ROOT/link-no-secondmates"; mkdir -p "$home/state" "$home/data" + err="$TMP_ROOT/link-no-secondmates.err" + PATH="$BASE_PATH" FM_HOME="$home" "$ROOT/bin/fm-x-link.sh" no-such req-1 >/dev/null 2>"$err"; rc=$? + expect_code 1 "$rc" "plain missing-task exit" + assert_grep "no such task: state/no-such.meta" "$err" "the plain missing-task error must still be reported" + assert_no_grep "fm-public-followup.sh register" "$err" \ + "a home with no second mates must not be pointed at the promised-final path" + pass "fm-x-link keeps the plain missing-task error when no second mate is registered" +} + +# The link writes into THIS home's own state/.meta, so it can never bind work +# that lives in a secondmate home. Refusing with a bare "no such task" left the +# public promise silently orphaned; the refusal must name the secondmate holding +# the task and the promised-final path that can actually bind it. +test_link_refuses_secondmate_routed_task_with_promised_final_pointer() { + local main sub err out rc + main="$TMP_ROOT/link-secondmate-main"; mkdir -p "$main/state" "$main/data" + sub="$TMP_ROOT/link-secondmate-sub"; mkdir -p "$sub/state" + printf 'sm-axi\n' > "$sub/.fm-secondmate-home" + printf 'window=w\nworktree=/wt\nkind=ship\n' > "$sub/state/routed-k1.meta" + printf '# Second mates\n\n- sm-axi - owns the axi domain (home: %s; scope: axi tooling; projects: axi; added 2026-01-01)\n' \ + "$sub" > "$main/data/secondmates.md" + err="$TMP_ROOT/link-secondmate.err" + out=$(PATH="$BASE_PATH" FM_HOME="$main" "$ROOT/bin/fm-x-link.sh" routed-k1 req-routed 2>"$err"); rc=$? + expect_code 1 "$rc" "secondmate-routed link exit" + [ -z "$out" ] || fail "a refused link must print no success line (got: $out)" + assert_grep "sm-axi" "$err" "the refusal must name the second mate holding the task" + assert_grep "--work-home secondmate:sm-axi" "$err" \ + "the refusal must name the exact promised-final binding for that second mate" + assert_grep "fm-public-followup.sh register" "$err" \ + "the refusal must point at the promised-final registration command" + assert_absent "$main/state/routed-k1.meta" "a refused link must not create a local record" + assert_no_grep "x_request=" "$sub/state/routed-k1.meta" \ + "a refused link must not write into the second mate's task record" + # The guardrail is scoped to the missing-record case: a task this home does own + # still links normally with second mates registered. + printf 'window=w\nworktree=/wt\nkind=ship\n' > "$main/state/local-k1.meta" + out=$(PATH="$BASE_PATH" FM_HOME="$main" FMX_NOW_OVERRIDE=1700000000 \ + "$ROOT/bin/fm-x-link.sh" local-k1 req-local 2>/dev/null); rc=$? + expect_code 0 "$rc" "local link exit with second mates registered" + assert_grep "x_request=req-local" "$main/state/local-k1.meta" \ + "a local task must still link while second mates are registered" + pass "fm-x-link refuses a second-mate-routed task and points at the promised-final path" +} + +# A remote secondmate's home cannot be inspected from here, and neither can a +# task the parent never recorded, so the refusal degrades to naming the routing +# possibility and the mechanism rather than silently reporting a missing file. +test_link_missing_task_with_secondmates_points_at_promised_final() { + local home err rc + home="$TMP_ROOT/link-remote-secondmate"; mkdir -p "$home/state" "$home/data" + printf '# Second mates\n\n- sm-far - owns the far domain (host: box; root: /srv/fm; home: /srv/fm/home; scope: far things; projects: far; added 2026-01-01)\n' \ + > "$home/data/secondmates.md" + err="$TMP_ROOT/link-remote-secondmate.err" + PATH="$BASE_PATH" FM_HOME="$home" "$ROOT/bin/fm-x-link.sh" unknown-k1 req-unknown >/dev/null 2>"$err"; rc=$? + expect_code 1 "$rc" "unknown-task link exit with second mates registered" + assert_grep "no such task: state/unknown-k1.meta" "$err" "the concrete missing record must still be reported" + assert_grep "fm-public-followup.sh register" "$err" \ + "an unlocatable task in a home with second mates must be pointed at the promised-final path" + assert_grep "--work-home secondmate:" "$err" \ + "an unlocatable task must leave the second mate id for the caller to fill in" + pass "fm-x-link points an unlocatable task at the promised-final path when second mates exist" +} + # --- fm-x-followup: detect, post up to 3 follow-ups, manage the link -------- mk_linked_task() { # [starting-count] @@ -2874,6 +2943,9 @@ test_link_recovery_relink_carries_discord_context_after_inbox_drain test_link_carry_count_validation test_meta_rewrites_do_not_depend_on_tmpdir test_link_rejects_unsafe_and_missing +test_link_missing_task_without_secondmates_stays_plain +test_link_refuses_secondmate_routed_task_with_promised_final_pointer +test_link_missing_task_with_secondmates_points_at_promised_final test_followup_check_states test_followup_check_expired_prunes_link test_followup_check_cap_reached_prunes_link From 196fb65b06aabe15625bd05af1e73afb611fa688 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:41:15 -0700 Subject: [PATCH 4/9] docs(skills): add remote-secondmate recovery hint for false-negative verdicts (#2456) * fix(skills): hint that remote secondmate liveness verdicts false-negative fm-crew-state and fm-send routinely misreport a live remote secondmate as dead; confirm against the pane before relaunching, and relaunch only through fm-spawn.sh, never raw herdr pane surgery. * no-mistakes: apply CI fixes --- .agents/skills/secondmate-provisioning/SKILL.md | 1 + .agents/skills/stuck-crewmate-recovery/SKILL.md | 3 +++ tests/fm-procevent.test.sh | 9 +++++++++ 3 files changed, 13 insertions(+) diff --git a/.agents/skills/secondmate-provisioning/SKILL.md b/.agents/skills/secondmate-provisioning/SKILL.md index 0ff1aa3e63..b878c6f765 100644 --- a/.agents/skills/secondmate-provisioning/SKILL.md +++ b/.agents/skills/secondmate-provisioning/SKILL.md @@ -215,6 +215,7 @@ Use the recorded `home=` in meta. If meta is missing but `data/secondmates.md` still registers the secondmate, respawn from the registry entry and its persistent home. For a remote route, the same command probes and relaunches only on the configured host. An SSH transport failure or unreadable remote endpoint remains unknown and must be reconciled on that host; never launch a local replacement. +`stuck-crewmate-recovery`'s remote-secondmate note owns why the endpoint-dead and send-failed verdicts that seem to justify this are themselves unreliable. Respawn re-resolves the secondmate harness from current config, uses the same guarded pre-launch sync, and re-propagates inherited local material, so recovered secondmates converge inherited config items and shared captain preferences whenever their home validates; tracked-file sync remains guarded separately. If the secondmate is already running and only inherited local material changed, prefer `bin/fm-config-push.sh` over respawning. To move a live LOCAL secondmate onto a newly pinned harness, model, or effort without a full recovery, set `config/secondmate-harness` and then relaunch it with `bin/fm-control.sh relaunch`, which re-resolves that pin, stops the agent, and launches the replacement in the same home ([`docs/agent-control.md`](../../../docs/agent-control.md)). diff --git a/.agents/skills/stuck-crewmate-recovery/SKILL.md b/.agents/skills/stuck-crewmate-recovery/SKILL.md index db8b6a08d4..cf741b9d95 100644 --- a/.agents/skills/stuck-crewmate-recovery/SKILL.md +++ b/.agents/skills/stuck-crewmate-recovery/SKILL.md @@ -23,6 +23,9 @@ The target window's harness is recorded as `harness=` in `state/.meta`. This procedure covers ordinary `kind=ship` and `kind=scout` direct reports. Load `secondmate-provisioning` instead for `kind=secondmate` recovery. +For a REMOTE secondmate, `fm-crew-state`'s `unknown`/`worktree gone` and `fm-send`'s `remote send failed`/`delivery unconfirmed` verdicts are unreliable and routinely false-negative; do not conclude the mate is dead or the send failed from those alone, confirm against the actual remote pane first. +Recover a genuinely stuck remote mate only through `bin/fm-spawn.sh --secondmate`, never raw herdr pane close/kill surgery, which strands the endpoint binding. + Treat the digest's endpoint result as a presence signal, not proof that the task's work or validation run is gone. Read the targeted current state with `bin/fm-crew-state.sh ` before deciding to relaunch. A no-mistakes run matched to the crew's branch and current code remains authoritative when the endpoint is dead: handle a terminal or parked run through the normal lifecycle, and keep supervising an active run instead of creating a duplicate worker. diff --git a/tests/fm-procevent.test.sh b/tests/fm-procevent.test.sh index 738281aecd..f92cc198b5 100755 --- a/tests/fm-procevent.test.sh +++ b/tests/fm-procevent.test.sh @@ -408,8 +408,17 @@ assert_present "$HSELF/state/procevent-inbox/self-src.1.handled" "the self-annou if [ -e "$HSELF/state/.wake-queue" ] && grep -q 'procevent selfann self-src 1' "$HSELF/state/.wake-queue"; then fail "a fully autohandled self-announcing capture still published a duplicate check wake" fi +# This self-announcing source's child returns instantly, so reconcile would +# restart it and that detached poll would race the failing-path start below for +# the source claim - non-deterministically stealing its sequence or the claim +# itself. Retire it before the re-announcement check so reconcile starts no +# competing poll, then re-register for the failing-path capture, the same +# retire-before-reconcile discipline the blocker-backed sources rely on. +pe_adapter "$HSELF" retire self-src >/dev/null out=$(pe_adapter "$HSELF" reconcile) assert_contains "$out" "published=0" "reconcile re-announced a capture its adapter already acknowledged" +assert_contains "$out" "started=0" "reconcile restarted an always-ready acknowledged source and raced the next start" +pe_adapter "$HSELF" register selfann self-src -- /bin/echo "self announced" >/dev/null : > "$HSELF/state/selfann-fail" out=$(pe_adapter "$HSELF" start self-src 2>&1) assert_contains "$out" "not-autohandled: self-src" "a failed self-announcing application was reported as applied" From ef35d799a846d676c2fd30b1d1e3ed47b0fb2c22 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:43:02 -0700 Subject: [PATCH 5/9] fix(calm): keep Pi's export confirmation visible (#2461) Pi 0.83.0 added a status line to every tool-expansion change, and Pi updates the previous status line in place when two status messages arrive back to back. Calm's post-export redraw cycled tool expansion on the macrotask right after Pi printed "Session exported to: ", so both expansion status lines coalesced over that confirmation and the captain was left with no record of where their export landed. Calm now repaints only the tool rows it presents, by invalidating each row through the render context Pi hands its render slots, and requests the surrounding redraw through setStatus. Neither appends to the transcript. The repaint is still needed because Pi can re-render a row asynchronously - the built-in edit row invalidates itself once its diff is ready - and that re-render can land inside the window where /export forces stock rendering. The real-terminal /export case now asserts the confirmation is still on screen after the redraw has settled, and that the redraw restored every Calm-hidden row, instead of only racing the moment the confirmation first appeared. --- .pi/extensions/fm-calm.ts | 33 +++++++++++++-- docs/calm-mode-feasibility.md | 65 +++++++++++++++++++++++++++++- tests/fm-calm-pi-extension.test.sh | 35 +++++++++++++++- 3 files changed, 128 insertions(+), 5 deletions(-) diff --git a/.pi/extensions/fm-calm.ts b/.pi/extensions/fm-calm.ts index e5e92649eb..1141e6edf1 100644 --- a/.pi/extensions/fm-calm.ts +++ b/.pi/extensions/fm-calm.ts @@ -195,6 +195,22 @@ export default function (pi: ExtensionAPI) { registerFirstmateSyntheticPresentation(pi); + // Every on-screen tool row Calm currently presents, keyed by the row-local state Pi + // hands its render slots, so Calm can repaint exactly those rows without touching + // Pi's transcript. Pi can re-render a row at any time - the built-in edit row + // invalidates itself once its diff is ready - so a row can be redrawn during the + // window where /export forces stock rendering and keep that stock content + // afterwards. Rows Pi's exporter renders are excluded: those use throwaway state + // and never appear on screen. Cleared per session lifetime, which rebuilds the rows. + const calmToolRowRepaints = new Map void>(); + const rememberCalmToolRow = (state: object, invalidate: unknown): void => { + if (exportRendering || typeof invalidate !== "function") return; + calmToolRowRepaints.set(state, invalidate as () => void); + }; + const repaintCalmToolRows = (): void => { + for (const invalidate of calmToolRowRepaints.values()) invalidate(); + }; + function wrapBuiltIn( factory: DefinitionFactory, ): ToolDefinition { @@ -262,6 +278,7 @@ export default function (pi: ExtensionAPI) { theme: RenderTheme, context: RenderContext, ) { + rememberCalmToolRow(context.state as object, context.invalidate); if (exportRendering) return originalRenderCall(args, theme, context); if (calmPresentationHides("assistant-tool-call")) return new Container(); if (originalSelfShell) return originalRenderCall(args, theme, context); @@ -280,6 +297,7 @@ export default function (pi: ExtensionAPI) { theme: RenderTheme, context: RenderContext, ) { + rememberCalmToolRow(context.state as object, context.invalidate); if (exportRendering) return originalRenderResult(result, options, theme, context); if (calmPresentationHides("tool-result")) return new Container(); if (originalSelfShell) return originalRenderResult(result, options, theme, context); @@ -392,6 +410,7 @@ export default function (pi: ExtensionAPI) { pi.on("session_start", (_event, ctx) => { reportBuiltInLosses(); + calmToolRowRepaints.clear(); exportRendering = false; setCalmPresentation(loadCalmPreference()); setCalmStockExportRendering(false); @@ -423,9 +442,17 @@ export default function (pi: ExtensionAPI) { exportRendering = false; setCalmStockExportRendering(false); publishPresentationState(); - const expanded = ctx.ui.getToolsExpanded(); - ctx.ui.setToolsExpanded(!expanded); - ctx.ui.setToolsExpanded(expanded); + // Repaint the rows Calm presents, never the whole transcript. Pi's export + // prints "Session exported to: " immediately before this runs, and + // since Pi 0.83.0 setToolsExpanded() emits its own status line; consecutive + // status lines coalesce, so a tools-expanded round-trip here silently + // overwrote the confirmation and left the captain no record of where their + // export landed. Invalidating the rows individually repaints the same + // content with no status line of its own, and setStatus adds the redraw the + // rows that consult Calm live in render(), such as operational user rows, + // need without appending anything to the transcript. + repaintCalmToolRows(); + ctx.ui.setStatus("firstmate-calm", undefined); }, 0); }); }); diff --git a/docs/calm-mode-feasibility.md b/docs/calm-mode-feasibility.md index 336e72eda1..683e6946ff 100644 --- a/docs/calm-mode-feasibility.md +++ b/docs/calm-mode-feasibility.md @@ -191,7 +191,8 @@ Calm classifies only at Pi's transcript-presentation owner through the canonical The session-start nudge already originates as a non-displayed custom message, so it remains on that existing path while retaining model context and session persistence. Legacy Calm custom entries and messages remain in existing session artifacts, and their presentation entry still uses the supported zero-height renderer while active. -Cycling tool expansion and restoring its original value rebuilds controllable rows and leaves final `Ctrl+O` state unchanged. +Toggling Calm cycles tool expansion and restores its original value, which rebuilds controllable rows and leaves final `Ctrl+O` state unchanged. +Returning from stock export rendering instead invalidates only the tool rows Calm currently presents: Pi 0.83.0 made every expansion change emit its own status line, and Pi coalesces consecutive status lines, so an expansion cycle there overwrote the `Session exported to:` confirmation the export had just printed. Exported and shared HTML retain genuine user prompts, genuine assistant responses, current operational user messages, ordinary tool rendering, and the complete session artifact. Serialized session data and Pi 0.81.1's sidebar tree also retain legacy hidden operational custom messages. @@ -437,3 +438,65 @@ right-heading: <| over \__/~~-~~~-~ At 3 columns the sprite fell back to a single exact-width row, `<|~`. Escape aborted the run leaving `Operation aborted`, no boat, and no stale sprite rows, and the trial exited 0 after deleting its temporary state. + +## 2026-08-15 Pi 0.84.1 export-confirmation verification + +Pi 0.83.0 added a status line to every tool-expansion change, which silently broke the `/export` confirmation under Calm on Pi 0.83.0 and newer. +Pi appends `Session exported to: ` through `showStatus`, which updates the previous status line in place whenever two status messages arrive back to back with nothing else added to the chat. +Calm's post-export redraw cycled tool expansion on the macrotask right after that, so both of its expansion status lines coalesced over the confirmation and left no record of where the export landed. +Calm now invalidates only the tool rows it presents and requests the redraw through `setStatus`, neither of which appends to the transcript. + +Pi source evidence, from the installed release's own changelog and interactive mode: + +```text +$ pi --version +0.84.1 + +CHANGELOG.md, 0.83.0 "Fixed": +- Added a status line when the tool output expansion is toggled ([#7180](https://github.com/earendil-works/pi/issues/7180)). + +interactive-mode setToolsExpanded: + setToolsExpanded(expanded) { + if (expanded === this.toolOutputExpanded) + return; + ... + this.showStatus(`Tool output: ${expanded ? "expanded" : "collapsed"}`); + } +``` + +The regression is pinned by the real-terminal `/export` case in `tests/fm-calm-pi-extension.test.sh`, which now asserts the confirmation is still on screen after Calm's redraw has settled and that the redraw restored every Calm-hidden row. +Reverting only the extension fix fails that assertion deterministically rather than racing the roughly 50ms window the confirmation used to survive: + +```text +not ok - Calm's post-export repaint overwrote Pi's export confirmation (missing: 'Session exported to: .../calm-export.html') +``` + +```text +$ tests/fm-calm-pi-extension.test.sh +ok - Pi calm resolves its persistent home independently of Pi's launch directory +ok - Pi calm compatibility evidence never rejects a Pi version for being newer than 0.82.0, and still fails closed on a missing or malformed version +ok - a missing collapsed-thinking presentation API degrades only that Calm adapter with a clear skip reason, while the rest of Calm still registers +ok - missing Pi presentation class exports reach the independent adapter degradation path +ok - Calm registers none of its 7 built-in tool wrappers at load while config/calm is off, and all 7 synchronously at load while config/calm is on +ok - Calm's first same-session /calm activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns prominently and logs the contested name, and only rows constructed before that activation - the documented bound - fail to retroactively collapse +ok - Pi calm centralizes transcript visibility, preserves execution/export data, keeps Pi's stock working row visible while no run is active, and persists its choice across session starts +ok - Pi calm on collapses mid-turn assistant working notes to zero height while Calm off keeps them, leaves streaming, truncated-final, and genuine final replies untouched, never mutates the messages, ignores every /calm argument, and restores a legacy persisted max as ordinary Calm on +ok - Pi operational follow-up E2E processes exact user-role notifications once while Calm hides current and adjacent rows, Calm off and absent render them, and restart preserves semantics +ok - Pi Calm native /skill:ahoy geometry keeps every collapsed thinking and tool block at zero height while preserving expansion, history, restart, and Calm-off rendering +ok - Pi Calm working ship moves on a slow independent cadence over faster fixed-cell blue water, paints the complete boat standard yellow with balanced resets, keeps ANSI-stripped width exact, flips the directional sail on the exact bounce at both edges and every width, clamps visible and hidden resizes, falls back deterministically when narrow, freezes and resumes column/direction across settle/start without hidden-time jumps or duplicate timers, resets only on a fresh session, and installs and removes one scheduler-owning widget across starts, settle, abort, failure, shutdown, reload, replacement, and Calm toggles while leaving Calm-off visibility untouched +ok - Pi calm native E2E replaces the stock working row with a moving, resize-clamped working ship that freezes and resumes across two working periods in one Pi session, clears on abort, keeps captain turns visible, hides exact operational user rows without changing persistence, restores stock rendering Calm-off, survives restart, and preserves export plus Ctrl+O behavior + +$ tests/fm-pi-primary-types.test.sh +ok - tracked Pi extensions pass strict no-emit typecheck against Pi 0.80.10 + +$ bin/fm-lint.sh +fm-lint.sh: ShellCheck 0.11.0 (pinned 0.11.0) + +$ bin/fm-doc-audience-check.sh +fm-doc-audience-check: ok surfaces=68 local_links=253 + +$ bin/fm-test-run.sh --changed --base origin/main +FM_TEST_SUMMARY total=46 failed=0 skipped_gate=16 duration_ms=279390 +FM_TEST_SUMMARY_FAMILY family=live-harness-optin count=16 duration_ms=431 failed=0 +FM_TEST_SUMMARY_FAMILY family=pure-contract-unit count=30 duration_ms=277700 failed=0 +``` diff --git a/tests/fm-calm-pi-extension.test.sh b/tests/fm-calm-pi-extension.test.sh index 3565b0e51a..5284491ec9 100755 --- a/tests/fm-calm-pi-extension.test.sh +++ b/tests/fm-calm-pi-extension.test.sh @@ -3079,7 +3079,7 @@ JS } test_interactive_terminal_e2e() { - local project config home session_file export_file export_dom default_snapshot expanded_snapshot hidden_snapshot active_before_snapshot active_hidden_snapshot export_snapshot restored_snapshot working_snapshot working_response_snapshot restarted_snapshot resumed_restored_snapshot hash_before hash_after now version chrome chrome_pid chrome_wait active_wait active_screen_wait boat_frame_one boat_frame_two boat_resized_snapshot boat_focus_snapshot boat_cleared_snapshot boat_hull_line boat_sail_line boat_column_one boat_column_two boat_line boat_color_snapshot boat_color_line boat_water_snapshot boat_water_line boat_water_first boat_water_changed boat_narrow_snapshot boat_narrow_sails boat_freeze_snapshot boat_resume_snapshot boat_freeze_column boat_freeze_sail boat_resume_column boat_resume_sail + local project config home session_file export_file export_dom default_snapshot expanded_snapshot hidden_snapshot active_before_snapshot active_hidden_snapshot export_snapshot export_settled_snapshot restored_snapshot working_snapshot working_response_snapshot restarted_snapshot resumed_restored_snapshot hash_before hash_after now version chrome chrome_pid chrome_wait active_wait active_screen_wait boat_frame_one boat_frame_two boat_resized_snapshot boat_focus_snapshot boat_cleared_snapshot boat_hull_line boat_sail_line boat_column_one boat_column_two boat_line boat_color_snapshot boat_color_line boat_water_snapshot boat_water_line boat_water_first boat_water_changed boat_narrow_snapshot boat_narrow_sails boat_freeze_snapshot boat_resume_snapshot boat_freeze_column boat_freeze_sail boat_resume_column boat_resume_sail if ! command -v pi >/dev/null 2>&1 || ! command -v tmux >/dev/null 2>&1; then echo "skip: pi or tmux not found for Pi calm interactive E2E" return 0 @@ -3099,6 +3099,7 @@ test_interactive_terminal_e2e() { active_before_snapshot="$TMP_ROOT/active-before.txt" active_hidden_snapshot="$TMP_ROOT/active-hidden.txt" export_snapshot="$TMP_ROOT/export.txt" + export_settled_snapshot="$TMP_ROOT/export-settled.txt" restored_snapshot="$TMP_ROOT/restored.txt" working_snapshot="$TMP_ROOT/working.txt" working_response_snapshot="$TMP_ROOT/working-response.txt" @@ -3556,6 +3557,38 @@ for (const current of ["CURRENT_WATCHER_E2E", "CURRENT_TURN_END_E2E", "CURRENT_A } if (!tree.includes("firstmate-synthetic-input") || !tree.includes("/tmp/probe.status")) process.exit(1); JS + # Calm returns the transcript to its own presentation once the export has been + # rendered. That repaint runs on the macrotask right after Pi prints the export + # confirmation, so it must not overwrite it: the captain has to keep seeing where + # their export landed. The export-data assertions above take seconds of real time, + # so this snapshot is taken well after that repaint has settled rather than racing it. + tmux -L "$TMUX_SOCKET" capture-pane -p -t "$TMUX_SESSION" -S -600 >"$export_settled_snapshot" + assert_contains "$(cat "$export_settled_snapshot")" "Session exported to: $export_file" \ + "Calm's post-export repaint overwrote Pi's export confirmation" + assert_not_contains "$(cat "$export_settled_snapshot")" "fm_watch_arm_pi" \ + "/export left the Firstmate watcher tool call shell in the Calm transcript" + assert_not_contains "$(cat "$export_settled_snapshot")" "watcher: started Pi extension arm child" \ + "/export left the Firstmate watcher tool result in the Calm transcript" + assert_not_contains "$(cat "$export_settled_snapshot")" "FIRSTMATE WATCHER WAKE: signal: /tmp/probe.status" \ + "/export left a synthetic Firstmate user-role presentation in the Calm transcript" + assert_not_contains "$(cat "$export_settled_snapshot")" "Thinking..." \ + "/export left collapsed thinking labels in the Calm transcript" + assert_not_contains "$(cat "$export_settled_snapshot")" "I will run one command." \ + "/export left a mid-turn assistant working note in the Calm transcript" + for hidden in \ + CURRENT_WATCHER_E2E \ + CURRENT_TURN_END_E2E \ + CURRENT_AWAY_E2E \ + CURRENT_FROM_FIRSTMATE_E2E \ + CURRENT_LAUNCH_BRIEF_E2E + do + assert_not_contains "$(cat "$export_settled_snapshot")" "$hidden" \ + "/export left operational input $hidden in the Calm transcript" + done + assert_contains "$(cat "$export_settled_snapshot")" "Show a deterministic tool example." \ + "/export removed a genuine user prompt from the Calm transcript" + assert_contains "$(cat "$export_settled_snapshot")" "The deterministic tool example is complete." \ + "/export removed genuine assistant conversation from the Calm transcript" tmux -L "$TMUX_SOCKET" send-keys -t "$TMUX_SESSION" -l "/calm" tmux -L "$TMUX_SOCKET" send-keys -t "$TMUX_SESSION" M-s From e518906a09b40513cb3b3d64d2cbf2775e209c55 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sun, 16 Aug 2026 11:13:11 -0700 Subject: [PATCH 6/9] feat(stow): add open-record persistence to /stow before reset (#2488) * feat(stow): persist the open records a session is holding /stow curated memory and captured session knowledge, but never touched record state, while AGENTS.md called it an "unfinished-work sweep" and the receipt declared the session "safe to reset" - wording that implied a record-correctness guarantee stow does not make. A shipped PR with no backlog item, a queued umbrella whose phases had merged, and four decision holds left open after their answers shipped all survived repeated stows. Add a bounded pass that files record state from the same volatile input the rest of stow already uses: the open threads in context, minutes before the reset destroys them. It creates a record for an unfiled thread and corrects one the session knows is wrong, through the owning path, and states its boundary as part of the contract - it never enumerates the backlog, lists holds, or queries a forge, because it cannot be a reconciliation and must not be read as one. Correct the wording in AGENTS.md and the completion receipt so reset-safe means what it actually guarantees: nothing this session knew was lost. * no-mistakes(review): correct stow decision-hold inspection to read hold via tasks-axi * no-mistakes(document): note /stow open-record persistence in README command catalog * refactor(stow): state open-record persistence as principle, not procedure The first version enumerated triggers, named commands, and prescribed an ordered procedure. That is too rigid for an agent skill: it invites literal execution of a checklist instead of judgment, and every enumerated example is a way for the guidance to go stale. Reduce it to the intent - before a reset, the important open work you are holding in context must end up durably recorded rather than dying with the session, filing what is unfiled and correcting what is stale - and let the agent judge importance, the record, and the owning write path. Keep the scope bound, since it is a decided contract and not a mechanic: this covers the open work the session is holding, never a reconciliation of durable records against repository or forge reality. The wording corrections in AGENTS.md and the completion receipt are unchanged. --- .agents/skills/stow/SKILL.md | 20 +++++++++++++++++--- AGENTS.md | 2 +- README.md | 2 +- docs/architecture.md | 2 ++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/.agents/skills/stow/SKILL.md b/.agents/skills/stow/SKILL.md index 55bd6e52f8..c7d96ce30d 100644 --- a/.agents/skills/stow/SKILL.md +++ b/.agents/skills/stow/SKILL.md @@ -1,6 +1,6 @@ --- name: stow -description: Sweep the current session for uncaptured durable knowledge, file it to disk, and curate the home's tiered, decaying startup memory before a context reset. Use when the captain invokes /stow (e.g. "/stow", "stow what you've learned"), before a session reset or context compaction, or periodically to keep operational memory current. +description: Sweep the current session for uncaptured durable knowledge, file it to disk, persist the open work records this session knows are unfiled or now wrong, and curate the home's tiered, decaying startup memory before a context reset. Use when the captain invokes /stow (e.g. "/stow", "stow what you've learned"), before a session reset or context compaction, or periodically to keep operational memory current. user-invocable: true metadata: internal: true @@ -10,7 +10,7 @@ metadata: # stow -Sweep this session for durable knowledge that exists only in conversation, then leave the next session with a compact current operating map rather than an accumulating journal. +Sweep this session for durable knowledge and open-work record state that exist only in conversation, then leave the next session with a compact current operating map rather than an accumulating journal. Memory entries are tiered and decay between passes, and stale material retires to a cold archive instead of being deleted. This skill writes only through the existing Firstmate ownership and write boundaries. @@ -207,6 +207,17 @@ A local skill exists only in this home, so offloading an entry out of `data/capt A stale unique fact is never deleted, only archived. Do not invent another graduation path. +## Open-record persistence + +The sweep above preserves knowledge; this one preserves the state of work. +A reset destroys whatever exists only in this session, and that includes what you have learned about work already under way, not just facts worth remembering. +So before the reset, make sure the important open work you are holding in context is durably recorded: file what was never filed, and correct what you now know is stale. + +Judge for yourself what is important and which record each thing belongs to, and write it through the owner that already governs that record. +One bound holds: this covers the open work you are actually holding in context, not the records at large. +It is not a reconciliation of durable records against repository or forge reality, cannot become one on input this volatile, and must never be reported as one. +Where the right correction is a judgment you cannot make, leave the record alone and raise the question instead of guessing. + ## One-time migration of unmarked entries Legacy entries carry no markers; an unmarked entry is its file's default tier with unknown age, and unknown age is not guilt. @@ -227,8 +238,11 @@ Report the outcome in plain captain-facing language with all of these facts: - each durable finding filed outside memory and its authoritative owner; - each archived entry's reason, each autonomous offload's live destination and actual relief, and, when a pinned candidate was proposed, the `proposed-offload` section with every candidate's fields; - every unresolved exception, including a primary-owned shared-file constraint in a secondmate home, and every concrete captain decision opened for an over-budget result; -- whether the session is safe to reset, only when all durable findings are captured and the post-pass result is within budget with no exception or pending budget decision. +- each open record this pass filed or corrected, and each one it deliberately left alone with the judgment it is waiting on; +- whether the session is safe to reset, only when all durable findings are captured, every open record this session held is filed or explicitly left with its reason, and the post-pass result is within budget with no exception or pending budget decision. +State what reset-safe means in the same breath as the claim: nothing this session knew has been lost. +It is never a claim that the home's durable records are correct, because this pass checks no record the session did not name. Do not hide an over-budget result behind a reset-safe claim. In a primary home the receipt is written after the cascade below, not instead of it. diff --git a/AGENTS.md b/AGENTS.md index bd40813bf7..0d06b4229b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -247,7 +247,7 @@ Route durable knowledge to its most specific owner: Firstmate never writes a project's `AGENTS.md` directly. A crewmate creates or updates it lazily through the project's selected delivery path, using `bin/fm-ensure-agents-md.sh` and preferring pointers to authoritative sources over copied detail. Keep fleet delivery posture and captain-private strategy out of project memory. -When the captain invokes `/stow`, load the `stow` skill for the complete knowledge-routing and unfinished-work sweep. +When the captain invokes `/stow`, load the `stow` skill for its memory curation, knowledge routing, and persistence of the open work records this session is holding; it files and corrects only the open work that session is holding, and never reconciles the backlog against repository or PR reality. ## 7. Task lifecycle diff --git a/README.md b/README.md index 92fab18637..8ed5226b17 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,7 @@ Claude and grok use the slash form shown here; codex uses the same names with `$ | `/ahoy` | Recap visible session events since the prior real captain message plus visibly unanswered captain decisions, then guide the captain through any open decisions one at a time in agent-judged impact order; fall back to Bearings when invoked as the session's first real captain message | | `/bearings` | Generate a concise four-section chat digest from bounded local fleet and registered-secondmate state; use `/bearings file` to also replace today's dated report in `data/`, and add `include PRs` when live PR enrichment is wanted | | `/updatefirstmate` | Self-update the running firstmate and its secondmates to the latest from origin with fast-forward-only pulls, then re-read instructions and nudge secondmates | -| `/stow` | Sweep the session for uncaptured durable knowledge, curate tiered startup memory with decay and cold archival, enforce each home's budget or surface the required decision, cascade to registered second mates, and report what is safe to reset | +| `/stow` | Sweep the session for uncaptured durable knowledge, persist the open work records this session knows are unfiled or now wrong, curate tiered startup memory with decay and cold archival, enforce each home's budget or surface the required decision, cascade to registered second mates, and report what is safe to reset | Bearings invocation examples: diff --git a/docs/architecture.md b/docs/architecture.md index 4b3f0b4492..ee6749827f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -305,6 +305,8 @@ The full ownership rule - what is project-intrinsic versus fleet-private, and ho `/stow` sweeps the current session for durable knowledge that only exists in conversation and routes each finding to the most specific disk home. Home-domain captain preferences go to `data/captain.md`, cross-domain shared captain preferences go to the primary home's `data/captain-shared.md`, fleet-local operational facts and gotchas go to home-local `data/learnings.md`, project-intrinsic knowledge goes through normal crewmate delivery into that project's committed `AGENTS.md`, and task-scoped notes or undone next steps go to the backlog. Memory writes use inspect-then-update rather than blind append; the internal [`stow` skill](../.agents/skills/stow/SKILL.md) owns tier markers, decay, cold archival, and offload. +The same pass also persists open-work record state the session is holding - filing a thread that was never recorded and correcting one the session knows went stale - bounded to the open work that session is actually holding. +It is deliberately not a reconciliation of durable records against repository or PR reality: its input is the volatile context, so it can only preserve what the session still knows, and no reconciliation that outlives a session exists today. Task-scoped notes use `tasks-axi show --full` followed by `tasks-axi update --body-file `, adding `--archive-body` when the prior body should remain recoverable. The stow pass never writes a skill, but a separately executed, captain-approved migration may move conditional knowledge into a user-owned local skill excluded from the Firstmate clone; changes to Firstmate's tracked skills remain deliberate repository work through the normal PR pipeline. Invoked in a primary home, `/stow` then cascades the same sweep to every registered secondmate, enumerated through `bin/fm-stow-cascade.sh`: each home is accounted and curated against its own startup-memory allowance, a live secondmate sweeps its own session, and a slow or unreachable home is reported as an exception rather than blocking the primary. From 362c508666432e35c8608b6c526464acba366307 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sun, 16 Aug 2026 20:22:49 -0700 Subject: [PATCH 7/9] fix(decisions): close decision holds at answer time via one general keyed-answer path (#2490) * fix(decisions): close captain holds at answer time Firstmate had two "a decision is open" ledgers with asymmetric closing mechanics. The live status-log ledger closes atomically at answer time, because bin/fm-send.sh --resolve-key makes answering a decision be the act that closes it. The durable backlog hold ledger had no such coupling: answering and recording were two separate acts, and only the first was forced by the workflow. That asymmetry lost four real captain decisions. Their answers were captured durably to disk, keyed character for character by the hold decision keys, acknowledged, and even implemented and shipped, yet the holds stayed open for two days and the captain was asked to re-answer decisions already on his own disk. Give the hold ledger the same answer-time-closure property: - bin/fm-decision-hold.sh gains an `answer` subcommand, the hold ledger's counterpart to --resolve-key. It shares one unrouted close implementation with `decline`, so it carries every existing guard - the captain decision file, the active-hold requirement, retry identity, and the refusal to release still-routed work - and differs only in the resolution mode it records. `decline` keeps its stronger meaning that the answer routes no follow-up work at all. - bin/fm-procevent-lavish.sh wires the channel that actually carried the lost answers. `arm --decisions-origin` binds a deck to the origin whose holds it carries, `answers` reads the structured choices out of a captured poll result, `close-decisions` maps each key to its hold and closes it through the command above, and `autohandle` lets the runner apply that at capture time. Safety is preserved rather than traded away. Only rows tagged `choice` are read, so freeform captain prose cannot forge a decision key. Closure is confined to the one bound origin. The decision text is a pure function of the captured result, so a replayed capture is idempotent. A hold that is absent, already closed, or still blocking routed work is skipped and left for `resolve`, never forced. A deck armed without the binding touches no hold at all. And autohandle deliberately never reports full handling, because recording an answer is transcription while acting on it is firstmate's judgement - so the check wake still reaches the handler. fm-send --resolve-key is untouched. * no-mistakes(document): document state/lavish-decisions binding dir in AGENTS.md state inventory * refactor(decisions): make keyed-answer closure one general capability The previous pass gave holds answer-time closure but built it as bespoke Lavish wiring: the review adapter carried the source-to-origin binding, mapped keys to hold identities, wrote decision records, decided what to skip, and closed holds itself. That treated a review deck as a special decision source. It is not - it is an ephemeral discussion format that happens to carry answers. Collapse it into ONE general capability with one owner. bin/fm-decision-hold.sh now owns the whole of "a keyed answer closes its matching hold": - `answers --source ` is the channel-agnostic intake. It reads key/answer/label lines on stdin, maps each key to its hold, and closes it through the same `answer` path, so every guard applies identically whatever channel the answer came from. --source is provenance recorded in the decision, never a behavior switch; there is no per-channel branch and no knowledge of chat, decks, or transports. - `bind`/`unbind`/`binding` own the source-to-origin binding for any channel whose answers arrive detached from their origin. Every channel is now an ordinary caller that only turns what it received into keyed lines: - bin/fm-send.sh (chat) feeds the intake for a key that names an active hold. This also fixes a real gap: once `complete` transfers a decision to its hold it closes the live status copy, so --resolve-key alone could never answer a transferred decision. - bin/fm-procevent.sh feeds it generically. A bound source's captured result goes to ` answers ` and whatever that prints is piped into the intake. The runner names no adapter, parses no result, and carries no decision rule, so any future adapter with an `answers` command works with no change here. - bin/fm-procevent-lavish.sh keeps only `answers`, which reports the structured choices a review captured and stops. It maps nothing to a hold and closes nothing; it lost ~160 lines of decision logic. Feeding is independent of handling, so it never acknowledges a result and never suppresses a wake - recording an answer is transcription, acting on it stays firstmate's judgement. The regression that proves closure now drives a FIXTURE adapter that is not the review adapter, so what is proven is that any bound channel reaches the intake rather than that one channel is wired specially. A new regression drives the real fm-send over a stubbed transport for the chat side. Every prior guarantee still holds, and fm-send's status-log behavior is unchanged. * no-mistakes(review): test(decisions): drop source-content grep from hold-closure regression --- .../skills/decision-hold-lifecycle/SKILL.md | 9 +- .agents/skills/process-event-sources/SKILL.md | 10 + AGENTS.md | 1 + bin/fm-decision-hold.sh | 216 ++++++++++- bin/fm-procevent-lavish.sh | 87 ++++- bin/fm-procevent.sh | 47 ++- bin/fm-send.sh | 75 +++- docs/configuration.md | 7 + docs/decision-hold-lifecycle.md | 57 ++- docs/verification/process-event-sources.md | 2 + tests/fm-decision-hold-lifecycle.test.sh | 348 ++++++++++++++++++ 11 files changed, 828 insertions(+), 31 deletions(-) diff --git a/.agents/skills/decision-hold-lifecycle/SKILL.md b/.agents/skills/decision-hold-lifecycle/SKILL.md index cacc0948fe..43e327dd62 100644 --- a/.agents/skills/decision-hold-lifecycle/SKILL.md +++ b/.agents/skills/decision-hold-lifecycle/SKILL.md @@ -23,6 +23,12 @@ Run the command in the originating work's authoritative `FM_HOME`; main-home wor Do not close a hold merely because the originating investigation completed, its report was archived, its visual review ended, or its task was torn down. When the captain's answer authorizes follow-up work, the hold remains the authoritative Captain's Call item until that answer is durably recorded, dependent work is created in the same backlog and blocked by the hold, and `bin/fm-decision-hold.sh resolve` routes the answer by clearing those dependency edges before closing the hold. When the captain's answer routes no follow-up work at all, such as a declined proposal, `bin/fm-decision-hold.sh decline` records that answer and closes the hold; it never substitutes for routing work the captain did authorize. +When the captain simply answers a hold that has no follow-up work routed behind it yet, `bin/fm-decision-hold.sh answer` records that answer and closes the hold, so answering is closing rather than a separate later act that can be forgotten. +"A keyed answer closes its matching hold" is one capability with one owner, `bin/fm-decision-hold.sh answers`, and every channel that carries a captain answer feeds it the same `` and answer. +A channel never maps a key to a hold, records a decision, or closes anything itself, so no channel is special and a new one needs no new closing logic. +Chat already feeds it: `bin/fm-send.sh --resolve-key` answers a decision in whichever ledger still holds it open, including a decision already transferred to its durable hold. +A captured-answer source feeds it too once bound with `bin/fm-decision-hold.sh bind `; bind before arming the source, and key each structured question by the hold's own decision key. +An unbound source and a question slug that is not a decision key both simply feed nothing: the answer is still captured and firstmate is still woken, and closing falls back to the commands above. A hold closed outside this owner leaves no durable answer, so the completion gate keeps failing until `bin/fm-decision-hold.sh repair` records the decision the captain actually gave; neither unrouted path may stand in for an answer the captain has not given. Resolved findings, recommendations that need no captain choice, and prose that merely sounds decision-like do not create holds. Bearings reads the resulting structured state and must never compensate by scraping historical reports, visual-review artifacts, terminal output, chat, or other prose. @@ -35,7 +41,8 @@ Bearings reads the resulting structured state and must never compensate by scrap 4. Run the script's `complete` command with the full unresolved-key inventory for that review pass. 5. Relay the choices to the captain as decisions from Bearings' Captain's Call section under `AGENTS.md` section 9; do not use the word hold in captain chat. 6. If the captain authorizes dependent work, record it with normal tasks-axi commands and block it by the hold identity. -7. Put the captain's exact durable decision in a file and close the hold with the script's `resolve` command and every routed task, its `decline` command when the answer routes no work, or its `repair` command when the hold was already closed outside the script. +7. Put the captain's exact durable decision in a file and close the hold with the script's `resolve` command and every routed task, its `answer` command when the captain answered a hold with no routed work behind it, its `decline` command when the answer routes no work at all, or its `repair` command when the hold was already closed outside the script. + A hold that a channel already closed by feeding its keyed answer needs none of these; confirm it in step 8 instead. 8. Confirm Bearings no longer shows the closed hold and that any routed work remains in structured backlog state. `bin/fm-decision-hold.sh --help` owns command syntax, identity construction, completion attestation, retry behavior, and close ordering. diff --git a/.agents/skills/process-event-sources/SKILL.md b/.agents/skills/process-event-sources/SKILL.md index 093272c41a..e5fd0c9b1c 100644 --- a/.agents/skills/process-event-sources/SKILL.md +++ b/.agents/skills/process-event-sources/SKILL.md @@ -31,6 +31,16 @@ For a Lavish review artifact: bin/fm-procevent-lavish.sh arm ``` +When a source carries captain answers to decisions that already have durable holds, bind it to their origin BEFORE arming it, so it can never produce an answer that has nowhere to go: + +```sh +bin/fm-decision-hold.sh bind +``` + +The runner then passes each captured result to that source's own adapter `answers` command and pipes the keyed answers it prints into the one keyed-answer intake, which owns every rule about what they mean. +This is generic: any adapter with an `answers` command works, and the runner still wakes you to act on the result. +`decision-hold-lifecycle` owns when a binding is required and what the keys must be. + A configured remote secondmate reply source is armed and handled through `bin/fm-procevent-remote-reply.sh`. Its header owns exact commands, while the adapter owns cursor continuity, validated deduplicated status ingest, path-confined document fetch, acknowledgement, and re-arming after a good delta. A continuity break is escalated once and stays unarmed until an operator deliberately rebases it. diff --git a/AGENTS.md b/AGENTS.md index 0d06b4229b..b0d4f7688b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -107,6 +107,7 @@ state/ runtime records and signals; gitignored 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 (section 13) procevent-inbox/ private captured results and their durable handled-acknowledgement markers; source output lives here and never in an event line + decision-bindings/ private bindings from a captured-answer source id to the captain-hold origin its keyed answers close; written only by bin/fm-decision-hold.sh bind, dropped by unbind and by source retirement (section 13; docs/decision-hold-lifecycle.md) when/ private condition->action watch specs, their trust bindings, and single-fire markers; written only by bin/fm-procevent-when.sh (section 13's process-event-sources trigger) x-inbox/ generated Relay pending mention payloads; fmx-respond drains it (section 14) x-context/ generated Relay durable per-request reply context and one-wake offer markers, keyed by request_id; survives inbox cleanup and expires within seven days (section 14; bin/fm-x-lib.sh) diff --git a/bin/fm-decision-hold.sh b/bin/fm-decision-hold.sh index 523fef6084..6f82e0676d 100755 --- a/bin/fm-decision-hold.sh +++ b/bin/fm-decision-hold.sh @@ -24,6 +24,11 @@ # fm-decision-hold.sh verify # fm-decision-hold.sh resolve \ # --decision-file --routed-to [--routed-to ...] +# fm-decision-hold.sh answer --decision-file +# fm-decision-hold.sh answers --source (keyed answers on stdin) +# fm-decision-hold.sh bind +# fm-decision-hold.sh unbind +# fm-decision-hold.sh binding # fm-decision-hold.sh decline --decision-file # fm-decision-hold.sh repair --decision-file # @@ -35,18 +40,62 @@ # `verify` is read-only and is called by scout teardown so teardown cannot erase a # source before this gate has succeeded. # -# `resolve` and `decline` close active holds; `repair` attests a hold already closed -# outside this script. All three paths require a non-empty captain decision file of -# at most 8192 bytes, record the same durable resolution block in the hold body, and -# store the decision digest plus routed identities so an exact retry is idempotent -# while a changed decision or, for `resolve`, routed set is rejected. New records -# include a `Resolution mode:` naming their path; older routed records remain valid. +# `resolve`, `answer`, and `decline` close active holds; `repair` attests a hold +# already closed outside this script. All four paths require a non-empty captain +# decision file of at most 8192 bytes, record the same durable resolution block in +# the hold body, and store the decision digest plus routed identities so an exact +# retry is idempotent while a changed decision or, for `resolve`, routed set is +# rejected. New records include a `Resolution mode:` naming their path; older +# routed records remain valid. # # `resolve` is the routed path. It requires every --routed-to task to exist and to # be blocked by the hold. It writes the captain decision and routed identities into # the hold body, clears those dependency edges, and only then marks the hold Done. # A failure before the final step leaves the captain hold open. # +# `answer` is the answer-time closure path, the hold ledger's counterpart to +# `fm-send.sh --resolve-key`: it exists so the act that carries the captain's +# answer is the act that closes the hold, instead of leaving closure to a +# separate later call nobody is forced to make. It records the captain's answer +# on an actively held hold, records `(none)` as the routed identities because no +# follow-up work has been routed behind the hold yet, and closes it. It shares +# every guard `decline` has, including the refusal while any task is still +# blocked by the hold, so a decision whose follow-up work is already routed still +# goes through `resolve` and the routed-vs-unrouted distinction survives. It says +# only that the captain answered; `decline` still says the captain answered with +# no follow-up work at all. +# +# ONE KEYED-ANSWER INTAKE, FED BY EVERY CHANNEL. +# "A keyed answer closes its matching hold" is a single capability, owned here +# and nowhere else. `answers` is its channel-agnostic entry point: it reads +# `\t\t