From 92856fd4ffeb996a6a2bd068fd0a811599466153 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:02:20 -0700 Subject: [PATCH 001/254] fix: pre-register Claude trust for secondmate homes (#4262) * fix(spawn): pre-register Claude workspace trust for secondmate homes A claude --secondmate launch skipped workspace-trust registration entirely, so a standalone-clone secondmate home (an explicit ~/fm-homes/ path) had no store entry and its pane wedged on the "Is this a project you trust?" dialog before it read its charter. The step was gated on the task kind rather than on the harness, so the spawn's fail-closed guard had nothing to run against and reported a launch that could never start work. fm-claude-trust.sh gains a secondmate-home mode. A secondmate home is a whole firstmate instance, produced either as a leased worktree or as a standalone clone, so the linked-worktree test cannot decide it and the seed is the evidence instead: the .fm-secondmate-home marker must be a regular file this user owns naming exactly the id being spawned, the home must hold AGENTS.md and bin/, and each operational directory must resolve inside the home. That is the set fm-home-seed.sh writes and fm-spawn.sh's own home validation re-checks, so nothing wider than a home a secondmate spawn would launch into can earn home-level trust. The worktree path is unchanged, and still refuses a home. fm-spawn.sh now runs the registration for every claude launch and keeps refusing the spawn when it fails, rather than launching an agent that would wedge. * no-mistakes(document): Correct Claude secondmate trust guidance --- .../references/common/control-and-recovery.md | 6 +- .../references/harness/claude.md | 8 +- bin/fm-claude-trust.sh | 183 +++++++++++---- bin/fm-spawn.sh | 64 +++--- docs/verification/runtime-backends.md | 43 +++- tests/fm-claude-trust.test.sh | 215 +++++++++++++++++- tests/fm-secondmate-harness.test.sh | 24 +- tests/fm-trace-context-spawn.test.sh | 12 +- 8 files changed, 464 insertions(+), 91 deletions(-) diff --git a/.agents/skills/harness-adapters/references/common/control-and-recovery.md b/.agents/skills/harness-adapters/references/common/control-and-recovery.md index c16a78bb8a8..78115e47174 100644 --- a/.agents/skills/harness-adapters/references/common/control-and-recovery.md +++ b/.agents/skills/harness-adapters/references/common/control-and-recovery.md @@ -17,15 +17,11 @@ Select only its documented trust choice from the active Firstmate home, binding No observed dialog proves only that launch. Each supported harness handles its folder-trust gate differently, and the tool reference owns the detail. -Claude gates a fresh worktree and cannot be answered by key, so the spawn pre-registers the path in Claude's own store. +For Claude, load `references/harness/claude.md`; its workspace-trust section owns the non-key-answerable gate and spawn-time pre-registration for every spawn kind. Cursor suppresses its dialog with launch-time `--trust`, and Muse suppresses its own with `--yolo`. Grok dodges its gate instead of granting trust, because its project picker appears only outside a project and the spawn starts in the isolated git root. Pi gates the fresh-worktree case too, but unlike Claude its dialog is answered with Enter, and `references/harness/pi.md` owns that recipe and where the decision persists. Codex shows a directory-trust dialog on the first run for a repository root. -A Claude secondmate is deliberately not pre-registered, because `../../../bin/fm-spawn.sh` runs its per-harness pre-launch setup only for non-secondmate kinds, so the registration is never invoked for one. -That kind guard is the whole exclusion, because a treehouse-leased secondmate home is itself a linked worktree that the scope test would accept, and only a plain-clone home would be refused as a primary checkout. -The consequence is that a claude secondmate whose home Claude has never trusted meets the workspace-trust dialog itself, and firstmate cannot answer it any more than it can for a crewmate. -This is rarely seen because a secondmate home is persistent and reused, so its trust decision is made once and survives, unlike a per-task worktree that is new every time. Use the tool's exact skill form, or natural language only when no separate command is verified or the form remains uncertain. A successful send or key return is not proof of submission; require the tool-specific postcondition. diff --git a/.agents/skills/harness-adapters/references/harness/claude.md b/.agents/skills/harness-adapters/references/harness/claude.md index 24591bc65e9..437eb77c072 100644 --- a/.agents/skills/harness-adapters/references/harness/claude.md +++ b/.agents/skills/harness-adapters/references/harness/claude.md @@ -16,10 +16,10 @@ Busy hooks verified 2026-07-28 on Claude Code 2.1.220. ## Workspace trust -Claude gates a folder it has never seen behind an interactive workspace-trust dialog, so every fresh task worktree would hit it. -`--dangerously-skip-permissions` does not cover that gate: `claude --help` records that the dialog is skipped only in non-interactive mode, through `-p` or a non-TTY stdout, and a crewmate pane is interactive. -A ship or scout spawn therefore pre-registers the worktree before launch, and the dialog does not appear. -`../../../bin/fm-claude-trust.sh` records `hasTrustDialogAccepted` for that worktree path in `${CLAUDE_CONFIG_DIR:-$HOME}/.claude.json`, and `../../../bin/fm-spawn.sh` refuses the spawn when the write fails rather than launching a worker that would wedge. +Claude gates a folder it has never seen behind an interactive workspace-trust dialog, so every fresh task worktree would hit it, and so would every secondmate home no operator has opened by hand. +`--dangerously-skip-permissions` does not cover that gate: `claude --help` records that the dialog is skipped only in non-interactive mode, through `-p` or a non-TTY stdout, and a spawned pane is interactive. +Every claude spawn therefore pre-registers the directory its pane starts in before launch, and the dialog does not appear: the task worktree for a ship or scout, and the home itself for a `--secondmate` spawn, in either seeded shape (a leased worktree or a standalone clone). +`../../../bin/fm-claude-trust.sh` records `hasTrustDialogAccepted` for that path in `${CLAUDE_CONFIG_DIR:-$HOME}/.claude.json` and owns the structural scope test each shape must pass, and `../../../bin/fm-spawn.sh` refuses the spawn when the registration fails rather than launching an agent that would wedge. Never try to answer the trust dialog with a key. Firstmate's key plane carries only Enter, Escape, and C-c with no arrow navigation, so it cannot move a dialog's selection at all, and the observed rendering starts on `No, exit`, which means a sent Enter ends the session instead of accepting. diff --git a/bin/fm-claude-trust.sh b/bin/fm-claude-trust.sh index 732a6b7c5c4..770dd79cfb5 100755 --- a/bin/fm-claude-trust.sh +++ b/bin/fm-claude-trust.sh @@ -1,26 +1,34 @@ #!/usr/bin/env bash -# Pre-register Claude Code's workspace trust for the isolated task worktree a -# ship/scout spawn is about to launch a claude crewmate into, so the worker -# reaches its brief instead of wedging on the trust dialog. +# Pre-register Claude Code's workspace trust for the directory a claude spawn is +# about to launch into - the isolated task worktree of a ship or scout crewmate, +# or the seeded home of a secondmate - so the agent reaches its brief or charter +# instead of wedging on the trust dialog. # # Usage: fm-claude-trust.sh +# fm-claude-trust.sh --secondmate-home # the isolated task worktree this spawn launches into # the primary checkout that worktree belongs to +# the seeded secondmate home this spawn launches into +# the secondmate id that home must already be marked for # Prints one line naming what it registered; refuses loudly on anything else. # # WHY THIS EXISTS. Claude Code gates a folder it has never seen behind an # interactive workspace-trust dialog, and --dangerously-skip-permissions does # NOT cover it: `claude --help` records that the dialog is skipped only in -# non-interactive mode (-p, or a non-TTY stdout), and a crewmate pane is -# interactive. Every fresh task worktree therefore hits it. The dialog renders +# non-interactive mode (-p, or a non-TTY stdout), and a spawned pane is +# interactive. Every fresh task worktree therefore hits it, and so does every +# secondmate home the operator has not opened by hand. The dialog renders # with the cursor on "No, exit" and firstmate's steering plane carries only # Enter, Escape and C-c with no arrow navigation, so firstmate cannot answer it -# and must not try - pressing Enter would select exit. The worker wedges before +# and must not try - pressing Enter would select exit. The agent wedges before # it ever reads the brief. Registering the trust before launch is the only # control that reaches an interactive pane. # # THE SCOPE TEST IS THE SAFETY PROPERTY, and it is STRUCTURAL rather than a -# path policy. must be a LINKED git worktree - its own git dir, +# path policy. Each mode has its own, because the two directories have entirely +# different shapes on disk. +# +# WORKTREE MODE. must be a LINKED git worktree - its own git dir, # sharing 's common dir - whose top level is exactly the resolved # argument. Git is the ground truth, so the argument is never trusted on its # own word: a primary checkout (git dir == common dir), a worktree of an @@ -45,12 +53,36 @@ # opt-in guard family (FM_*_LIVE_E2E=1) and record the result in # docs/verification/runtime-backends.md, rather than assuming the shape here. # +# SECONDMATE-HOME MODE. A secondmate home is a whole firstmate instance rather +# than a task worktree, and bin/fm-home-seed.sh produces it in two shapes: a +# leased treehouse worktree (linked) and a standalone clone of the firstmate +# repo (a primary checkout). The worktree test above therefore cannot decide +# this case at all - it refuses the standalone clone as a primary checkout, +# which is why a claude secondmate in an explicit ~/fm-homes/ home met the +# dialog with nothing registered. Git shape is not the evidence here; THE SEED +# IS. The home must carry a .fm-secondmate-home marker that is a regular file +# this user owns, never a symlink, naming exactly the passed; it must hold +# the firstmate instance files AGENTS.md and bin/; and each of its data, state, +# config and projects paths must resolve inside the home. That is the set +# bin/fm-home-seed.sh writes and bin/fm-spawn.sh's validate_firstmate_home_for_spawn +# re-checks before launch, so this accepts exactly the homes a secondmate spawn +# will launch into and nothing wider: a plain directory, a project checkout, an +# ordinary firstmate checkout, a home marked for a different secondmate, and a +# home whose operational directory escapes it are each refused. An ABSENT +# operational directory is accepted for the same reason the spawn accepts one - +# a test stricter than the spawn's own would move the wedge from the dialog to +# a refusal without making any unseeded directory less trusted. +# +# Home-level trust is broader than worktree trust, since the pane starts in the +# home and the secondmate works across it, so it is granted on that seed +# evidence alone and never on a caller's word about what a path is. +# # Only the launching user's own store is written: the projects entry for the -# worktree path in ${CLAUDE_CONFIG_DIR:-$HOME}/.claude.json, which must be a +# registered path in ${CLAUDE_CONFIG_DIR:-$HOME}/.claude.json, which must be a # regular file this uid owns. Every unrelated key and project entry is # preserved, and the replacement is atomic. fm-spawn.sh forwards CLAUDE_CONFIG_DIR -# onto the claude launch verbatim rather than resolving it, and the worker's pane -# starts in the task worktree, so only an absolute value names the same store on +# onto the claude launch verbatim rather than resolving it, and the pane starts +# in the registered directory, so only an absolute value names the same store on # both sides; a relative one is refused below rather than guessed at. set -u # Path resolution here must answer from the filesystem, never from the caller's @@ -69,9 +101,36 @@ unset CDPATH \ GIT_DISCOVERY_ACROSS_FILESYSTEM GIT_CONFIG GIT_CONFIG_GLOBAL \ GIT_CONFIG_SYSTEM GIT_CONFIG_NOSYSTEM GIT_CONFIG_COUNT -[ "$#" -eq 2 ] || { echo "usage: fm-claude-trust.sh " >&2; exit 2; } -WT_ARG=$1 -PROJ_ARG=$2 +usage() { + echo "usage: fm-claude-trust.sh " >&2 + echo " fm-claude-trust.sh --secondmate-home " >&2 + exit 2 +} + +# MODE selects which structural scope test decides the argument, and SCOPE_NOUN +# names what the argument was expected to be so every shared refusal below reads +# correctly in both modes. +case "${1:-}" in + --secondmate-home) + [ "$#" -eq 3 ] || usage + MODE=secondmate-home + TARGET_ARG=$2 + SUB_ID=$3 + PROJ_ARG= + SCOPE_NOUN="secondmate home" + ;; + '' | -h | --help) + usage + ;; + *) + [ "$#" -eq 2 ] || usage + MODE=worktree + TARGET_ARG=$1 + SUB_ID= + PROJ_ARG=$2 + SCOPE_NOUN="task worktree" + ;; +esac refuse() { echo "error: refusing to pre-register Claude trust: $1" >&2; exit 1; } @@ -90,10 +149,12 @@ common_dir_of() { (cd -P -- "$dir" && real_dir "$common") } -WT_REAL=$(real_dir "$WT_ARG") || true -[ -n "$WT_REAL" ] || refuse "worktree '$WT_ARG' is not an accessible directory" -PROJ_REAL=$(real_dir "$PROJ_ARG") || true -[ -n "$PROJ_REAL" ] || refuse "project '$PROJ_ARG' is not an accessible directory" +TARGET_REAL=$(real_dir "$TARGET_ARG") || true +[ -n "$TARGET_REAL" ] || refuse "$SCOPE_NOUN '$TARGET_ARG' is not an accessible directory" +if [ "$MODE" = worktree ]; then + PROJ_REAL=$(real_dir "$PROJ_ARG") || true + [ -n "$PROJ_REAL" ] || refuse "project '$PROJ_ARG' is not an accessible directory" +fi CONFIG_DIR=${CLAUDE_CONFIG_DIR:-${HOME:-}} [ -n "$CONFIG_DIR" ] || refuse "neither CLAUDE_CONFIG_DIR nor HOME is set, so the store cannot be located" @@ -116,30 +177,66 @@ if [ -z "$CONFIG_DIR_REAL" ]; then fi [ -n "$CONFIG_DIR_REAL" ] || refuse "Claude config directory '$CONFIG_DIR' does not exist and could not be created" -# A home or config directory is never a task worktree. Checked explicitly so -# the refusal names the real reason instead of the git verdict behind it. -[ "$WT_REAL" != "$CONFIG_DIR_REAL" ] || refuse "'$WT_REAL' is the Claude config directory, not a task worktree" +# The filesystem root, a home directory, and the config directory are never +# something this registers, in either mode. Checked explicitly so the refusal +# names the real reason instead of the scope verdict behind it. +[ "$TARGET_REAL" != / ] || refuse "'/' is the filesystem root, not a $SCOPE_NOUN" +[ "$TARGET_REAL" != "$CONFIG_DIR_REAL" ] || refuse "'$TARGET_REAL' is the Claude config directory, not a $SCOPE_NOUN" if [ -n "${HOME:-}" ]; then HOME_REAL=$(real_dir "$HOME") || true - [ "$WT_REAL" != "${HOME_REAL:-}" ] || refuse "'$WT_REAL' is the home directory, not a task worktree" + [ "$TARGET_REAL" != "${HOME_REAL:-}" ] || refuse "'$TARGET_REAL' is the home directory, not a $SCOPE_NOUN" fi -WT_TOP=$(git -C "$WT_REAL" rev-parse --show-toplevel 2>/dev/null) || true -[ -n "$WT_TOP" ] || refuse "'$WT_REAL' is not inside a git repository" -WT_TOP_REAL=$(real_dir "$WT_TOP") || true -[ "$WT_TOP_REAL" = "$WT_REAL" ] || refuse "'$WT_REAL' is not a worktree root (its root is '${WT_TOP_REAL:-unresolvable}')" +if [ "$MODE" = worktree ]; then + WT_TOP=$(git -C "$TARGET_REAL" rev-parse --show-toplevel 2>/dev/null) || true + [ -n "$WT_TOP" ] || refuse "'$TARGET_REAL' is not inside a git repository" + WT_TOP_REAL=$(real_dir "$WT_TOP") || true + [ "$WT_TOP_REAL" = "$TARGET_REAL" ] || refuse "'$TARGET_REAL' is not a worktree root (its root is '${WT_TOP_REAL:-unresolvable}')" -WT_GIT_DIR=$(git -C "$WT_REAL" rev-parse --absolute-git-dir 2>/dev/null) || true -[ -n "$WT_GIT_DIR" ] || refuse "'$WT_REAL' has no resolvable git directory" -WT_GIT_DIR=$(real_dir "$WT_GIT_DIR") || true -[ -n "$WT_GIT_DIR" ] || refuse "'$WT_REAL' has an unresolvable git directory" -WT_COMMON=$(common_dir_of "$WT_REAL") || true -[ -n "$WT_COMMON" ] || refuse "'$WT_REAL' has no resolvable git common directory" -[ "$WT_GIT_DIR" != "$WT_COMMON" ] || refuse "'$WT_REAL' is a primary checkout, not an isolated worktree" + WT_GIT_DIR=$(git -C "$TARGET_REAL" rev-parse --absolute-git-dir 2>/dev/null) || true + [ -n "$WT_GIT_DIR" ] || refuse "'$TARGET_REAL' has no resolvable git directory" + WT_GIT_DIR=$(real_dir "$WT_GIT_DIR") || true + [ -n "$WT_GIT_DIR" ] || refuse "'$TARGET_REAL' has an unresolvable git directory" + WT_COMMON=$(common_dir_of "$TARGET_REAL") || true + [ -n "$WT_COMMON" ] || refuse "'$TARGET_REAL' has no resolvable git common directory" + [ "$WT_GIT_DIR" != "$WT_COMMON" ] || refuse "'$TARGET_REAL' is a primary checkout, not an isolated worktree" -PROJ_COMMON=$(common_dir_of "$PROJ_REAL") || true -[ -n "$PROJ_COMMON" ] || refuse "project '$PROJ_REAL' is not inside a git repository" -[ "$WT_COMMON" = "$PROJ_COMMON" ] || refuse "'$WT_REAL' is not a worktree of project '$PROJ_REAL'" + PROJ_COMMON=$(common_dir_of "$PROJ_REAL") || true + [ -n "$PROJ_COMMON" ] || refuse "project '$PROJ_REAL' is not inside a git repository" + [ "$WT_COMMON" = "$PROJ_COMMON" ] || refuse "'$TARGET_REAL' is not a worktree of project '$PROJ_REAL'" +else + # The seed evidence, in the order that names the most useful reason first: the + # marker decides whether this is a secondmate home at all, the id decides + # whose, and the instance files and operational directories decide whether it + # is the shape bin/fm-home-seed.sh leaves behind. The marker is the token the + # whole boundary rests on, so it is judged as a file rather than as a value: a + # symlink is refused outright rather than followed, because a link is a way to + # make some other file's bytes stand in for the seed, and a marker this user + # does not own was planted by someone else. + [ -n "$SUB_ID" ] || refuse "no secondmate id was supplied, so '$TARGET_REAL' cannot be matched against its seed marker" + SUB_MARKER="$TARGET_REAL/.fm-secondmate-home" + [ ! -L "$SUB_MARKER" ] || refuse "'$SUB_MARKER' is a symlink; a seeded secondmate home carries the marker as a regular file" + [ -f "$SUB_MARKER" ] || refuse "'$TARGET_REAL' carries no .fm-secondmate-home marker, so it is not a seeded secondmate home" + [ -O "$SUB_MARKER" ] || refuse "'$SUB_MARKER' is not owned by this user" + SUB_MARKER_ID=$(cat "$SUB_MARKER" 2>/dev/null) || true + [ "$SUB_MARKER_ID" = "$SUB_ID" ] || refuse "'$TARGET_REAL' is marked for secondmate '${SUB_MARKER_ID:-unknown}', not '$SUB_ID'" + [ -f "$TARGET_REAL/AGENTS.md" ] || refuse "'$TARGET_REAL' has no AGENTS.md, so it is not a firstmate home" + [ -d "$TARGET_REAL/bin" ] || refuse "'$TARGET_REAL' has no bin/, so it is not a firstmate home" + for sub_dir_name in data state config projects; do + sub_dir="$TARGET_REAL/$sub_dir_name" + if [ -L "$sub_dir" ] && [ ! -e "$sub_dir" ]; then + refuse "'$sub_dir' is a broken symlink, so this home's $sub_dir_name directory cannot be shown to stay inside it" + fi + [ -e "$sub_dir" ] || continue + [ -d "$sub_dir" ] || refuse "'$sub_dir' is not a directory, so '$TARGET_REAL' is not a seeded secondmate home" + sub_dir_real=$(real_dir "$sub_dir") || true + [ -n "$sub_dir_real" ] || refuse "'$sub_dir' cannot be resolved" + case "$sub_dir_real" in + "$TARGET_REAL"/*) ;; + *) refuse "'$sub_dir' resolves to '$sub_dir_real', outside the home, so '$TARGET_REAL' is not a safe secondmate home" ;; + esac + done +fi # The store write needs node, and a missing interpreter refuses like every other # failure here. Degrading instead would launch a worker straight into the dialog @@ -190,11 +287,11 @@ fi # attempts, and it must fail loudly rather than report a trust it did not leave. # ponytail: fingerprint-and-refuse, not a lock; flock is absent on macOS and # cannot stop a vendor session's own rewrite anyway. -if ! node - "$STORE" "$WT_REAL" <<'NODE' +if ! node - "$STORE" "$TARGET_REAL" <<'NODE' const fs = require("node:fs"); const path = require("node:path"); const crypto = require("node:crypto"); -const [store, worktree] = process.argv.slice(2); +const [store, target] = process.argv.slice(2); const readStore = () => { try { return fs.readFileSync(store); @@ -223,12 +320,12 @@ const attempt = () => { if (projects === null || typeof projects !== "object" || Array.isArray(projects)) { throw new Error(`${store} has a non-object "projects" value`); } - let entry = projects[worktree]; + let entry = projects[target]; if (entry === undefined || entry === null || typeof entry !== "object" || Array.isArray(entry)) { entry = {}; } entry.hasTrustDialogAccepted = true; - projects[worktree] = entry; + projects[target] = entry; // Unpredictable name plus an exclusive create: the config directory may be // writable by another local account, and a predictable path could be // pre-created there as a symlink that a plain write would follow into some @@ -250,7 +347,7 @@ const attempt = () => { if (!renamed) fs.rmSync(tmp, { force: true }); } const back = JSON.parse(fs.readFileSync(store, "utf8")); - return back.projects?.[worktree]?.hasTrustDialogAccepted === true ? "recorded" : "dropped"; + return back.projects?.[target]?.hasTrustDialogAccepted === true ? "recorded" : "dropped"; }; try { for (let i = 0; i < 3; i += 1) { @@ -265,11 +362,11 @@ try { console.error(`error: ${err.message}`); process.exit(1); } -console.error(`error: ${store} did not retain trust for ${worktree} after 3 attempts`); +console.error(`error: ${store} did not retain trust for ${target} after 3 attempts`); process.exit(1); NODE then - refuse "could not record trust for '$WT_REAL' in '$STORE'" + refuse "could not record trust for '$TARGET_REAL' in '$STORE'" fi -echo "trusted: $WT_REAL" +echo "trusted: $TARGET_REAL" diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index df1181dd561..13aaad7df50 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -305,13 +305,13 @@ # park owns that home's supervision (docs/supervision-protocols/cursor.md). # claude is the one harness whose pre-launch setup can REFUSE the spawn: before # any per-task state exists, and before its worktree .claude/settings.local.json -# hooks are written, a non-secondmate claude launch pre-registers the worktree in -# the launching user's own Claude trust store through bin/fm-claude-trust.sh, -# because Claude's interactive workspace-trust dialog gates a fresh worktree and -# firstmate cannot answer it. That helper's header owns the structural scope test -# and every refusal; a failed registration stops this spawn rather than launching -# a worker that would wedge on the dialog. A --secondmate launch never runs it, -# so a claude secondmate home keeps its own one-time trust decision. +# hooks are written, every claude launch pre-registers the directory the pane +# starts in - the task worktree, or the secondmate home for a --secondmate spawn - +# in the launching user's own Claude trust store through bin/fm-claude-trust.sh, +# because Claude's interactive workspace-trust dialog gates a folder it has never +# seen and firstmate cannot answer it. That helper's header owns the structural +# scope test for both shapes and every refusal; a failed registration stops this +# spawn rather than launching a worker that would wedge on the dialog. # Every claude launch also carries the attribution-off policy in its per-launch # --settings JSON, so a spawned worker never writes a Co-Authored-By trailer, # Claude-Session link, or generated-with line into a commit or PR body; @@ -3217,27 +3217,35 @@ if [ "$RELAUNCH" -eq 0 ] && [ "$KIND" != secondmate ]; then freshen_spawn_worktree_base "$WT" || exit 1 fi -# Pre-register Claude's workspace trust for the worktree, at the first point the -# worktree is known and before any per-task state is created below. The dialog -# gates the pane before the brief is ever read, and it also gates loading the -# project settings written further down, so nothing armed below takes effect -# without it. bin/fm-claude-trust.sh owns the structural scope test and refuses -# any path that is not this project's own isolated worktree; a refusal blocks the -# spawn rather than launching a worker that would wedge on a dialog firstmate -# cannot answer. Refusing here rather than beside the arm keeps this in the same -# class as the two worktree refusals just above: no temp root, no retired -# relaunch wiring and no busy record exists yet to strand, so the refusal names -# the endpoint the same way they do and leaves nothing else behind. -if [ "$KIND" != secondmate ]; then - case "$HARNESS" in - claude*) - if ! "$FM_ROOT/bin/fm-claude-trust.sh" "$WT" "$PROJ_ABS" >/dev/null; then - echo "error: could not pre-register Claude workspace trust for $WT; refusing to launch a claude worker that would wedge on the trust dialog; inspect window $T" >&2 - exit 1 - fi - ;; - esac -fi +# Pre-register Claude's workspace trust for the directory this launch starts in, +# at the first point that directory is known and before any per-task state is +# created below. The dialog gates the pane before the brief is ever read, and it +# also gates loading the project settings written further down, so nothing armed +# below takes effect without it. EVERY claude launch needs it, a secondmate's +# included: its home is just as unseen by Claude as a fresh worktree, and +# skipping the step for that kind left a standalone-clone secondmate home with +# nothing registered and a pane wedged on a dialog firstmate cannot answer. +# bin/fm-claude-trust.sh owns the structural scope test for both shapes and +# refuses anything that is neither this project's own isolated worktree nor a +# seeded secondmate home marked for this id; a refusal blocks the spawn rather +# than launching a worker that would wedge. Refusing here rather than beside the +# arm keeps this in the same class as the two worktree refusals just above: no +# temp root, no retired relaunch wiring and no busy record exists yet to strand, +# so the refusal names the endpoint the same way they do and leaves nothing else +# behind. +case "$HARNESS" in + claude*) + if [ "$KIND" = secondmate ]; then + spawn_trust_args=(--secondmate-home "$PROJ_ABS" "$ID") + else + spawn_trust_args=("$WT" "$PROJ_ABS") + fi + if ! "$FM_ROOT/bin/fm-claude-trust.sh" "${spawn_trust_args[@]}" >/dev/null; then + echo "error: could not pre-register Claude workspace trust for $WT; refusing to launch a claude worker that would wedge on the trust dialog; inspect window $T" >&2 + exit 1 + fi + ;; +esac # Per-task temp root: /tmp/fm-/ with Go's build temp nested at gotmp/. Go won't # create GOTMPDIR, so mkdir before it is used; fm-teardown removes the whole root. diff --git a/docs/verification/runtime-backends.md b/docs/verification/runtime-backends.md index 1ec8b925be0..80e032d706a 100644 --- a/docs/verification/runtime-backends.md +++ b/docs/verification/runtime-backends.md @@ -300,7 +300,48 @@ That warning rendered in the same shape as the trust dialog, with the selection That gate is not a production blocker, because a normal environment has already accepted it and the treatment arm above ran against the real config and saw neither dialog. This change does not address that warning and does not claim to. -`bin/fm-spawn.sh` therefore pre-registers the task worktree through `bin/fm-claude-trust.sh` before launch, and `tests/fm-claude-trust.test.sh` pins both halves of the scope contract: a fresh worktree is trusted, and an out-of-scope path is refused. +### Secondmate homes + +Verified 2026-09-11 on Claude Code 2.1.269. +A secondmate launches in its own firstmate home rather than a task worktree, and that home meets the same gate. +The control arm launched a standalone-clone secondmate home that the store had no entry for, the way `bin/fm-spawn.sh --secondmate` launches one. + +```sh +tmux -L new-session -d -s ctrl -x 180 -y 44 -c \ + "CLAUDE_CODE_ENABLE_PROMPT_SUGGESTION=false claude --dangerously-skip-permissions" +``` + +``` + Accessing workspace: + /private/tmp/fm-sm-trust-live-69759/fm-homes/livemate-n1 + Quick safety check: Is this a project you created or one you trust? ... + ❯ No, exit + Yes, I trust this folder +``` + +The treatment arm pre-registered that same home through the secondmate-home mode and launched it identically against the operator's real config. + +```sh +bin/fm-claude-trust.sh --secondmate-home livemate-n1 +``` + +``` +trusted: /private/tmp/fm-sm-trust-live-69759/fm-homes/livemate-n1 +``` + +``` + ▐▛███▛█ Claude Code v2.1.269 +▝▜██████▀ Opus 4.8 with high effort · Claude Max + ▝▝ ▝▝ /private/tmp/fm-sm-trust-live-69759/fm-homes/livemate-n1 +... +❯ + ⏵⏵ bypass permissions on (shift+tab to cycle) · ← for agents +``` + +No dialog appeared, the composer was reached, and neither did the machine-scoped bypass warning, because this ran against the real config. +The lab home was deleted and the test entry was removed from the store and verified absent, with the same point-in-time caveat as the worktree arms above. + +`bin/fm-spawn.sh` therefore pre-registers the directory every claude launch starts in through `bin/fm-claude-trust.sh` before launch, and `tests/fm-claude-trust.test.sh` pins both halves of the scope contract for both shapes: a fresh worktree and a seeded secondmate home are trusted, and an out-of-scope path is refused. That automated spawn case runs against a fake claude, so it asserts the store entry and the launch command and nothing more; the live arms above are what establish that the entry actually suppresses the dialog. The composer-classification record below observes the same gate from the other side, where an untrusted worktree left Claude, Grok, and Muse unverified because the guard reads a first-launch trust dialog as an unreadable composer. diff --git a/tests/fm-claude-trust.test.sh b/tests/fm-claude-trust.test.sh index 94211e0e4b4..893840c1901 100755 --- a/tests/fm-claude-trust.test.sh +++ b/tests/fm-claude-trust.test.sh @@ -1,9 +1,10 @@ #!/usr/bin/env bash # Behavior tests for bin/fm-claude-trust.sh and the claude spawn that calls it. # -# Both halves of the contract are load-bearing and both are proven here: a -# legitimate fresh task worktree is trusted so a claude worker reaches its -# brief with no human, and every out-of-scope path is REFUSED rather than +# Both halves of the contract are load-bearing and both are proven here, for +# each directory a claude launch can start in: a legitimate fresh task worktree +# and a seeded secondmate home are trusted so the agent reaches its brief or +# charter with no human, and every out-of-scope path is REFUSED rather than # warned about or quietly skipped. set -u @@ -78,6 +79,53 @@ node_free_path() { # -> a bin dir holding the script's own tools but printf '%s\n' "$dir" } +# --- secondmate homes ------------------------------------------------------- + +# seed_secondmate_home [shape]: the on-disk shape bin/fm-home-seed.sh +# leaves behind - the identity marker, the firstmate instance files, the four +# operational directories, and a charter for the launch to carry. "clone" (the +# default) is the standalone-clone home an explicit ~/fm-homes/ path +# produces, a primary checkout of the firstmate repo; "worktree" is the linked +# worktree a treehouse lease produces. Both shapes are real homes, so both must +# be trusted. +seed_secondmate_home() { + local home=$1 id=$2 shape=${3:-clone} src + case "$shape" in + worktree) + src="$home.src" + fm_git_worktree "$src" "$home" "sm-$id" + ;; + *) + mkdir -p "$home" + fm_git_init_commit "$home" + ;; + esac + mkdir -p "$home/bin" "$home/data" "$home/state" "$home/config" "$home/projects" + printf '# Firstmate\n' > "$home/AGENTS.md" + printf 'charter\n' > "$home/data/charter.md" + printf '%s\n' "$id" > "$home/.fm-secondmate-home" +} + +# run_home_trust [user-home]: invoke the secondmate-home +# mode against an isolated store. +run_home_trust() { + local config=$1 home=$2 id=$3 user_home=${4:-$1} + CLAUDE_CONFIG_DIR="$config" HOME="$user_home" "$TRUST" --secondmate-home "$home" "$id" 2>&1 +} + +# spawn_secondmate_claude : run a real --secondmate claude +# spawn against the isolated store at /claude-config, logging the +# launch to /launch.log. Echoes the spawn output. +spawn_secondmate_claude() { + local case_dir=$1 home=$2 id=$3 primary fakebin + primary="$case_dir/primary" + mkdir -p "$case_dir/claude-config" + fakebin=$(make_spawn_fakebin "$case_dir/fake" claude) + fm_test_spawn_home "$primary" claude + FM_TEST_CLAUDE_CONFIG_DIR="$case_dir/claude-config" FM_FAKE_LAUNCH_LOG="$case_dir/launch.log" \ + fm_test_run_spawn "$primary" "$home" "$fakebin" "$id" "$home" claude --secondmate +} + test_fresh_worktree_is_trusted() { local rec out rec=$(make_case fresh) @@ -440,6 +488,162 @@ test_claude_spawn_pretrusts_its_worktree_and_reaches_the_brief() { pass "fm-spawn.sh: a claude spawn pre-trusts its worktree and launches with the brief" } +# A secondmate home is the second directory a claude launch starts in, and it is +# as unseen by Claude as a fresh worktree. The standalone-clone shape is the one +# that wedged in production: the trust step was skipped for every secondmate, so +# nothing was registered and the pane stopped on the dialog before it read its +# charter. +test_secondmate_standalone_clone_home_is_trusted() { + local case_dir home out + case_dir="$TMP_ROOT/sm-clone-spawn" + home="$case_dir/fm-homes/nomistakes-n1" + seed_secondmate_home "$home" nomistakes-n1 clone + out=$(spawn_secondmate_claude "$case_dir" "$home" nomistakes-n1) + expect_code 0 $? "a claude secondmate spawn into a standalone-clone home must succeed: $out" + assert_trusted "$case_dir/claude-config/.claude.json" "$home" \ + "the claude secondmate spawn did not pre-register trust for its standalone-clone home" + assert_present "$case_dir/launch.log" "the claude secondmate spawn sent no launch command" + assert_grep 'claude --dangerously-skip-permissions' "$case_dir/launch.log" \ + "the launch command was not the claude secondmate launch" + assert_grep "$home/data/charter.md" "$case_dir/launch.log" \ + "the launch command did not carry the charter the secondmate must read" + # The pane must read the SAME store the registration wrote, or the trust would + # land somewhere it never looks and the dialog would appear anyway. + assert_grep "CLAUDE_CONFIG_DIR='$case_dir/claude-config'" "$case_dir/launch.log" \ + "the launch command did not point the secondmate at the store that was trusted" + pass "fm-spawn.sh: a claude secondmate spawn pre-trusts a standalone-clone home" +} + +# The other seeded shape, a treehouse-leased linked worktree. It must be trusted +# through the same seed evidence rather than incidentally, so the registration +# does not depend on which shape the home happens to have. +test_secondmate_leased_worktree_home_is_trusted() { + local case_dir home out + case_dir="$TMP_ROOT/sm-leased-spawn" + home="$case_dir/leased/home" + mkdir -p "$case_dir/leased" + seed_secondmate_home "$home" leased-n1 worktree + out=$(spawn_secondmate_claude "$case_dir" "$home" leased-n1) + expect_code 0 $? "a claude secondmate spawn into a leased worktree home must succeed: $out" + assert_trusted "$case_dir/claude-config/.claude.json" "$home" \ + "the claude secondmate spawn did not pre-register trust for its leased worktree home" + pass "fm-spawn.sh: a claude secondmate spawn pre-trusts a leased worktree home" +} + +# The seed is the whole security boundary for home-level trust, so every path +# that is not a home seeded for THIS secondmate is refused and left untrusted. +# Each row drives one structural property apart from a genuine home. +test_secondmate_home_trust_refuses_everything_unseeded() { + local case_dir config home target out + case_dir="$TMP_ROOT/sm-refusals" + config="$case_dir/claude-config" + mkdir -p "$config" + + # A plain directory: no marker at all. + target="$case_dir/plain" + mkdir -p "$target" + out=$(run_home_trust "$config" "$target" plain-n1) + expect_code 1 $? "a plain directory must be refused: $out" + assert_contains "$out" "no .fm-secondmate-home marker" "the refusal did not name the missing marker" + assert_not_trusted "$config/.claude.json" "$target" "a plain directory was trusted" + + # A firstmate checkout that was never seeded as a secondmate home: every other + # structural signal matches and only the marker is missing. + target="$case_dir/checkout" + seed_secondmate_home "$target" checkout-n1 clone + rm -f "$target/.fm-secondmate-home" + out=$(run_home_trust "$config" "$target" checkout-n1) + expect_code 1 $? "an unseeded firstmate checkout must be refused: $out" + assert_contains "$out" "no .fm-secondmate-home marker" "the refusal did not name the missing marker" + assert_not_trusted "$config/.claude.json" "$target" "an unseeded firstmate checkout was trusted" + + # A home seeded for a DIFFERENT secondmate: one home's trust must not be + # granted while spawning another id. + target="$case_dir/other-mate" + seed_secondmate_home "$target" other-n1 clone + out=$(run_home_trust "$config" "$target" wanted-n1) + expect_code 1 $? "a home marked for another secondmate must be refused: $out" + assert_contains "$out" "other-n1" "the refusal did not name the id the home is marked for" + assert_not_trusted "$config/.claude.json" "$target" "a home marked for another secondmate was trusted" + + # A marker that is a symlink: another file's bytes must not stand in for the + # seed, even when they read as the right id. + target="$case_dir/linked-marker" + seed_secondmate_home "$target" linked-n1 clone + printf 'linked-n1\n' > "$case_dir/planted-id" + ln -sf "$case_dir/planted-id" "$target/.fm-secondmate-home" + out=$(run_home_trust "$config" "$target" linked-n1) + expect_code 1 $? "a symlinked marker must be refused: $out" + assert_contains "$out" "symlink" "the refusal did not name the symlinked marker" + assert_not_trusted "$config/.claude.json" "$target" "a home whose marker is a symlink was trusted" + + # An operational directory that escapes the home: the home's own working + # surface must stay inside it. + target="$case_dir/escaping" + seed_secondmate_home "$target" escaping-n1 clone + rm -rf "$target/projects" + mkdir -p "$case_dir/elsewhere" + ln -s "$case_dir/elsewhere" "$target/projects" + out=$(run_home_trust "$config" "$target" escaping-n1) + expect_code 1 $? "a home whose operational directory escapes it must be refused: $out" + assert_contains "$out" "outside the home" "the refusal did not name the escaping directory" + assert_not_trusted "$config/.claude.json" "$target" "a home whose projects/ escapes it was trusted" + + # The user's own home directory, seeded to prove the marker alone cannot carry + # it: HOME is refused in this mode exactly as it is for a worktree. + target="$case_dir/user-home" + seed_secondmate_home "$target" userhome-n1 clone + out=$(run_home_trust "$config" "$target" userhome-n1 "$target") + expect_code 1 $? "the user's home directory must be refused: $out" + assert_contains "$out" "home directory" "the refusal did not name the home directory" + assert_not_trusted "$config/.claude.json" "$target" "the user's home directory was trusted" + # Prove the seed really would have been accepted, so the guard above is what + # refused rather than an unrelated failure. + out=$(run_home_trust "$config" "$target" userhome-n1 "$case_dir/elsewhere-home") + expect_code 0 $? "the same seeded home must be accepted once it is not HOME: $out" + + pass "fm-claude-trust.sh: home-level trust is refused for everything but a home seeded for this secondmate" +} + +# A secondmate home is not a linked worktree, so worktree mode must keep +# refusing it rather than quietly widening to cover the new case. +test_worktree_mode_still_refuses_a_secondmate_home() { + local case_dir config home out + case_dir="$TMP_ROOT/sm-wrong-mode" + config="$case_dir/claude-config" + home="$case_dir/home" + mkdir -p "$config" + seed_secondmate_home "$home" mode-n1 clone + out=$(run_trust "$config" "$home" "$home") + expect_code 1 $? "worktree mode must still refuse a standalone-clone home: $out" + assert_contains "$out" "primary checkout" "the refusal did not name the primary checkout" + assert_not_trusted "$config/.claude.json" "$home" "worktree mode trusted a standalone-clone home" + pass "fm-claude-trust.sh: worktree mode still refuses a secondmate home" +} + +# The fail-closed half for secondmates: when the home's trust genuinely cannot be +# recorded, the spawn must refuse rather than launch a pane that would wedge on +# the dialog. This is the guard that never fired while the step was skipped. +test_secondmate_spawn_fails_closed_when_home_trust_cannot_be_recorded() { + local case_dir home out + case_dir="$TMP_ROOT/sm-failclosed" + home="$case_dir/fm-homes/failclosed-n1" + # Root owns /etc/passwd, so a store resolving to it is refused as another + # user's file. Running as root would own it and make the refusal vacuous. + if [ "$(id -u)" = 0 ]; then + pass "fm-spawn.sh: a claude secondmate spawn refuses when home trust cannot be recorded (skipped as root)" + return 0 + fi + seed_secondmate_home "$home" failclosed-n1 clone + mkdir -p "$case_dir/claude-config" + ln -s /etc/passwd "$case_dir/claude-config/.claude.json" + out=$(spawn_secondmate_claude "$case_dir" "$home" failclosed-n1) + expect_code 1 $? "a secondmate spawn whose trust registration is refused must fail: $out" + assert_contains "$out" "workspace trust" "the spawn did not report the trust refusal" + assert_absent "$case_dir/launch.log" "a secondmate was launched into a home whose trust could not be recorded" + pass "fm-spawn.sh: a claude secondmate spawn refuses when home trust cannot be recorded" +} + test_fresh_worktree_is_trusted test_registration_is_idempotent test_primary_checkout_is_refused @@ -460,3 +664,8 @@ test_missing_node_is_refused test_scope_refusal_stays_fail_closed_without_node test_claude_spawn_pretrusts_its_worktree_and_reaches_the_brief test_refused_spawn_leaves_no_task_state +test_secondmate_standalone_clone_home_is_trusted +test_secondmate_leased_worktree_home_is_trusted +test_secondmate_home_trust_refuses_everything_unseeded +test_worktree_mode_still_refuses_a_secondmate_home +test_secondmate_spawn_fails_closed_when_home_trust_cannot_be_recorded diff --git a/tests/fm-secondmate-harness.test.sh b/tests/fm-secondmate-harness.test.sh index 83668e21593..9a6ebe2df1f 100755 --- a/tests/fm-secondmate-harness.test.sh +++ b/tests/fm-secondmate-harness.test.sh @@ -64,6 +64,12 @@ fm_git_identity fmtest fmtest@example.com TMP_ROOT=$(fm_test_tmproot fm-secondmate-harness) export FM_BACKEND=tmux +# Every claude launch pre-registers workspace trust for the directory it starts +# in, and for a secondmate that directory is the home (bin/fm-claude-trust.sh). +# Several cases here resolve claude, so every spawn below pins a throwaway HOME +# with an empty CLAUDE_CONFIG_DIR and puts node on the spawn's PATH; without the +# first, this suite would write the developer's real ~/.claude.json. + # =========================================================================== # A) fm-harness.sh secondmate resolution + fallback (deterministic detect_own) # =========================================================================== @@ -426,6 +432,10 @@ make_noop_tmux() { exit 0 SH chmod +x "$fakebin/tmux" + # BASE_PATH deliberately omits the developer's node, which the trust + # registration below needs, so link the real one in rather than presenting a + # node-less spawn host no real fleet member looks like. + ln -sf "$(command -v node)" "$fakebin/node" printf '%s\n' "$fakebin" } @@ -455,7 +465,7 @@ spawn_secondmate() { [ -n "$harness" ] && spawn_args+=("$harness") spawn_args+=(--secondmate) PATH="$fakebin:$BASE_PATH" TMUX='' CLAUDECODE=1 \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$world/home" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$world/home" HOME="$world/home/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$world/home/state" FM_DATA_OVERRIDE="$world/home/data" \ FM_PROJECTS_OVERRIDE="$world/home/projects" FM_CONFIG_OVERRIDE="$world/home/config" \ FM_SPAWN_NO_GUARD=1 \ @@ -568,7 +578,7 @@ test_spawn_unverified_secondmate_harness_refused() { err="$w/spawn.err" rc=0 PATH="$fakebin:$BASE_PATH" TMUX='' CLAUDECODE=1 \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" HOME="$w/home/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$w/home/state" FM_DATA_OVERRIDE="$w/home/data" \ FM_PROJECTS_OVERRIDE="$w/home/projects" FM_CONFIG_OVERRIDE="$w/home/config" \ FM_SPAWN_NO_GUARD=1 \ @@ -595,7 +605,7 @@ test_spawn_cursor_secondmate_launches_with_its_primary_contract() { : > "$launchlog" rc=0 PATH="$fakebin:$BASE_PATH" TMUX='' CLAUDECODE=1 \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" HOME="$w/home/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$w/home/state" FM_DATA_OVERRIDE="$w/home/data" \ FM_PROJECTS_OVERRIDE="$w/home/projects" FM_CONFIG_OVERRIDE="$w/home/config" \ FM_SPAWN_NO_GUARD=1 FM_FAKE_LAUNCH_LOG="$launchlog" FM_FAKE_PANE_PATH="$sm" \ @@ -661,6 +671,10 @@ exit 0 SH chmod +x "$fakebin/tmux" fm_fake_exit0 "$fakebin" pi + # BASE_PATH deliberately omits the developer's node, which the trust + # registration below needs, so link the real one in rather than presenting a + # node-less spawn host no real fleet member looks like. + ln -sf "$(command -v node)" "$fakebin/node" printf '%s\n' "$fakebin" } @@ -674,7 +688,7 @@ spawn_secondmate_capture() { fakebin=$(make_launch_capturing_tmux "$world/tmux-$id") : > "$launchlog" PATH="$fakebin:$BASE_PATH" TMUX='' CLAUDECODE=1 \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$world/home" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$world/home" HOME="$world/home/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$world/home/state" FM_DATA_OVERRIDE="$world/home/data" \ FM_PROJECTS_OVERRIDE="$world/home/projects" FM_CONFIG_OVERRIDE="$world/home/config" \ FM_SPAWN_NO_GUARD=1 FM_FAKE_LAUNCH_LOG="$launchlog" \ @@ -2572,7 +2586,7 @@ SH chmod +x "$fakebin/rm" launchlog="$w/spawn-quarantine.launch.log" out=$(PATH="$fakebin:$BASE_PATH" TMUX='' CLAUDECODE=1 \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$w/home" HOME="$w/home/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$w/home/state" FM_DATA_OVERRIDE="$w/home/data" \ FM_PROJECTS_OVERRIDE="$w/home/projects" FM_CONFIG_OVERRIDE="$w/home/config" \ FM_SPAWN_NO_GUARD=1 FM_FAKE_LAUNCH_LOG="$launchlog" \ diff --git a/tests/fm-trace-context-spawn.test.sh b/tests/fm-trace-context-spawn.test.sh index d38fbf787d1..b9a61736246 100755 --- a/tests/fm-trace-context-spawn.test.sh +++ b/tests/fm-trace-context-spawn.test.sh @@ -214,8 +214,12 @@ run_two_level() { smlog="$base/sm-launch.log" smfake=$(make_spawn_fakebin "$base/sm-fake") : > "$smlog" + # A claude secondmate spawn pre-registers workspace trust for the HOME it + # launches into (bin/fm-claude-trust.sh), so this runs against a throwaway + # HOME; without it this suite would write the developer's real ~/.claude.json. + mkdir -p "$base/user-home" env FM_TRACE_CONTEXT="$penv" \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$prim" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$prim" HOME="$base/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$prim/state" FM_DATA_OVERRIDE="$prim/data" \ FM_PROJECTS_OVERRIDE="$prim/projects" FM_CONFIG_OVERRIDE="$prim/config" \ FM_SPAWN_NO_GUARD=1 CLAUDECODE=1 TMUX="fake,1,0" \ @@ -387,8 +391,12 @@ test_duplicate_secondmate_spawn_does_not_converge_trace_context() { printf 'charter\n' > "$sm/data/charter.md" fake=$(make_spawn_fakebin "$base/fake") + # A claude secondmate spawn pre-registers workspace trust for the HOME it + # launches into (bin/fm-claude-trust.sh), so this runs against a throwaway + # HOME; without it this suite would write the developer's real ~/.claude.json. + mkdir -p "$base/user-home" out=$(env -u FM_TRACE_CONTEXT \ - FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$prim" \ + FM_ROOT_OVERRIDE="$ROOT" FM_HOME="$prim" HOME="$base/user-home" CLAUDE_CONFIG_DIR='' \ FM_STATE_OVERRIDE="$prim/state" FM_DATA_OVERRIDE="$prim/data" \ FM_PROJECTS_OVERRIDE="$prim/projects" FM_CONFIG_OVERRIDE="$prim/config" \ FM_SPAWN_NO_GUARD=1 CLAUDECODE=1 TMUX="fake,1,0" \ From c191eacd36a9a44db0fb1888c67fe3dbfe534919 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:06:14 -0700 Subject: [PATCH 002/254] fix: ignore superseded failed GitHub check runs (#4258) * fix(pr-merge): judge each required check by its current run When the base branch advances, GitHub cancels a pull request's in-flight run and re-triggers it. The cancelled run stays in statusCheckRollup beside the passing re-run, so the rollup can hold several runs of one check name at the same head while GitHub itself reports the pull request CLEAN. github_checks_not_green judged every run independently, so that superseded failure refused a genuinely mergeable pull request and pushed the operator toward a needless --allow-red. Group the rollup by the reported name and judge each check by its current run. Supersession is proven, never assumed: a name leaves the red set only when every one of its non-green runs is strictly older than one of its green runs, dated by the forge's own settled timestamp - a check run's completedAt once its status is COMPLETED, or a status context's createdAt - and only in the whole-second UTC form GitHub emits, which is the one spelling that orders correctly as plain text. A run with no such timestamp is never superseded, so a still-running, queued or undated run keeps its check red, and a name with no green run at all stays red. An unnamed entry is grouped alone so two unrelated unnamed checks are never treated as one. Every comparison is one-directional: it can only clear a failure a later success provably replaced, and never clears a check whose current run failed, is pending, or is missing. No other guard moves - the pull request must still be open, undrafted, mergeable, conflict-free and head-bound, and --allow-red still waives exactly its named check with every other check green. Live reproduction: PR #4224 read CLEAN with an old FAILURE and a newer SUCCESS for one check name and was refused; it now verifies, while #4208 and #4210, whose latest runs failed, still refuse. * no-mistakes(review): Use check-run start times for safe supersession * no-mistakes(document): Clarify GitHub check-rollup documentation --- bin/fm-pr-merge.sh | 86 ++++++++++-- docs/architecture.md | 1 + tests/fm-pr-merge.test.sh | 284 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 358 insertions(+), 13 deletions(-) diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 174c3188dae..0b8aa79aeb2 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -11,7 +11,9 @@ # A GitHub merge is refused unless every pre-merge condition holds, each read # live at merge time rather than taken from recorded metadata: the pull request # is open, not a draft, mergeable, free of conflicts, and every unwaived check -# is green at the exact current head commit. Every failing condition is reported, not +# is green at the exact current head commit, where github_checks_not_green below +# owns what makes a check green and judges each one by its current run. +# Every failing condition is reported, not # just the first. The verified head is then passed to gh as # --match-head-commit, so a push that lands between that read and the merge # fails the merge instead of landing commits nothing verified. Reading that @@ -443,22 +445,80 @@ FIELDS } # Every GitHub check that is not green in the given live pull-request JSON, one -# name per line: a status context whose state is not SUCCESS, or a check run -# that has not completed with SUCCESS, NEUTRAL, or SKIPPED (so a pending -# check is not green either). Exits nonzero when the rollup cannot be read, so -# a malformed answer is a failed read and never an empty red set. +# name per line. An entry is green when it is a status context whose state is +# SUCCESS, or a check run that completed with SUCCESS, NEUTRAL, or SKIPPED (so +# a pending check is not green either). Exits nonzero when the rollup cannot be +# read, so a malformed answer is a failed read and never an empty red set. +# +# The rollup can hold several runs of one check name at the same head, because +# GitHub cancels a pull request's in-flight run when the base branch advances +# and re-triggers it; the cancelled run stays in the rollup beside the passing +# re-run. A check is therefore judged by its current run rather than by any run +# that a later one superseded, which is what makes this agree with GitHub's own +# CLEAN mergeStateStatus instead of refusing a pull request GitHub considers +# mergeable. +# +# Supersession applies only among check runs with the same reported name. A +# name is dropped from the red set only when every non-green run is COMPLETED, +# has a whole-second UTC startedAt, and started strictly before a green run. +# Status contexts are never grouped or superseded, and every non-green one is +# reported independently. A still-running, queued, undated, or tied check run +# stays red. A name whose runs are all green needs no timestamp, while a name +# with no green run stays red. +# +# The reported name is also what --allow-red matches. An unnamed check run is +# grouped alone and can neither supersede nor be superseded, because unrelated +# unnamed checks must not be treated as one. github_checks_not_green() { local json=$1 printf '%s' "$json" | jq -r ' + def settled_at: + if type == "string" and test("^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}Z$") + then . else null end; if (.statusCheckRollup | type) != "array" then error("no check rollup") else . end - | .statusCheckRollup[] - | if .__typename == "CheckRun" then - {name: (.name // ""), ok: (.status == "COMPLETED" and (.conclusion == "SUCCESS" or .conclusion == "NEUTRAL" or .conclusion == "SKIPPED"))} - else - {name: (.context // ""), ok: (.state == "SUCCESS")} - end - | select(.ok | not) - | if .name == "" then "(unnamed check)" else .name end + | [ .statusCheckRollup + | to_entries[] + | .key as $i + | .value + | if .__typename == "CheckRun" then + { + kind: "check_run", + name: (.name // ""), + completed: (.status == "COMPLETED"), + ok: (.status == "COMPLETED" and (.conclusion == "SUCCESS" or .conclusion == "NEUTRAL" or .conclusion == "SKIPPED")), + at: (.startedAt | settled_at) + } + | . + {group: (if .name == "" then ["", $i] else [.name, -1] end)} + else + {kind: "status_context", name: (.context // ""), ok: (.state == "SUCCESS")} + end + ] + | . as $entries + | ( + ($entries[] + | select(.kind == "status_context" and (.ok | not)) + | .name + ), + ($entries + | [.[] | select(.kind == "check_run")] + | group_by(.group)[] + | { + name: .[0].name, + reds: [.[] | select(.ok | not)], + newest_green: ([.[] | select(.ok) | .at | select(. != null)] | max) + } + | select( + (.reds | length) > 0 + and ( + .newest_green == null + or any(.reds[]; (.completed | not) or .at == null) + or ([.reds[] | .at] | max) >= .newest_green + ) + ) + | .name + ) + ) + | if . == "" then "(unnamed check)" else . end ' 2>/dev/null || return 1 } diff --git a/docs/architecture.md b/docs/architecture.md index 7ab4e496f7d..a02e4b9a1e5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -314,6 +314,7 @@ This repo uses that setting, and its own `.no-mistakes/` directory remains local PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling the forge CLI. The helper requires a full canonical URL and rejects malformed URLs or repo override flags before recording merge state. A `https://github.com///pull/` URL requires `gh` and `jq`, is merged only after one live read confirms the pull request is open, not a draft, mergeable, conflict-free, and every unwaived check is green at the current head, then `gh pr merge` binds that verified head with `--match-head-commit`. +A check run is green when its current run is green, because GitHub leaves a cancelled run in the rollup beside the passing re-run it triggered when the base branch advanced; `bin/fm-pr-merge.sh`'s `github_checks_not_green` owns the rule, which uses `startedAt` to clear only an older completed check run that a passing run with the same name provably replaced, while unfinished check runs and non-green status contexts stay red. `--auto`, `--admin`, and branch-deletion flags are refused unless `--attended-override` is passed for an explicit captain instruction; that override never skips the live green check, the away-grant check, or a captain hold. An attended `--allow-red ` may appear once, waives only GitHub checks with that exact name, and is refused while the away-posture record exists. A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 7d29879a6d0..5f4122b001e 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -77,6 +77,41 @@ write_github_red_json() { JSON } +# One CheckRun rollup entry the way GitHub reports it. A conclusion or timestamp +# of "-" is emitted as JSON null. Args: name status conclusion [startedAt] +# [completedAt] +check_run() { + local name=$1 status=$2 conclusion=$3 started=${4:--} completed=${5:-${4:--}} + local conclusion_json='null' started_json='null' completed_json='null' + [ "$conclusion" = - ] || conclusion_json="\"$conclusion\"" + [ "$started" = - ] || started_json="\"$started\"" + [ "$completed" = - ] || completed_json="\"$completed\"" + printf '{"__typename":"CheckRun","name":"%s","status":"%s","conclusion":%s,"startedAt":%s,"completedAt":%s}' \ + "$name" "$status" "$conclusion_json" "$started_json" "$completed_json" +} + +status_context() { + local name=$1 state=$2 + printf '{"__typename":"StatusContext","context":"%s","state":"%s"}' "$name" "$state" +} + +# Live GitHub JSON whose rollup holds the given entries verbatim, so a test can +# put several runs of one check name at the same head the way GitHub does after +# it cancels a pull request's in-flight run and re-triggers it. mergeStateStatus +# stays CLEAN because that is what GitHub reports for exactly this case. +# Args: case_dir head_sha ... +write_github_rollup_json() { + local case_dir=$1 head=$2 entry rollup='' + shift 2 + for entry in "$@"; do + rollup="${rollup:+$rollup,}$entry" + done + printf '%s\n' "$head" > "$case_dir/github-head" + cat > "$case_dir/github-view.json" < "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "github-superseded-red: a failed run replaced by a passing re-run must merge"$'\n'"$(cat "$case_dir/stderr")" + assert_logged_gh_merge "$case_dir" 90 example/repo --squash + pass "fm-pr-merge merges when a failed check run was replaced by a passing re-run" +} + +# Legacy status contexts remain independent from check runs, even when their +# reported names match. +test_check_runs_never_supersede_status_contexts() { + local case_dir rc head + head=cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd + case_dir=$(make_case github-cross-check-kind) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(status_context ci FAILURE)" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:09Z)" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/97 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "github-cross-check-kind: a failing status context must refuse" + assert_grep "check 'ci' is not green" "$case_dir/stderr" \ + "github-cross-check-kind: the status context was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "github-cross-check-kind: a passing check run hid a failing status context" + pass "fm-pr-merge never lets a check run supersede a legacy status context" +} + +# The inverse, and the one that matters most: a check whose current run failed is +# still red however many earlier runs of it passed. +test_current_failed_check_run_still_refuses() { + local case_dir rc head + head=dddddddddddddddddddddddddddddddddddddddd + case_dir=$(make_case github-current-red) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:01Z)" \ + "$(check_run ci COMPLETED FAILURE 2026-01-01T00:00:09Z)" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/91 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "github-current-red: a currently failing check must refuse" + assert_grep "check 'ci' is not green" "$case_dir/stderr" \ + "github-current-red: the red check was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "github-current-red: gh pr merge ran on a currently failing check" + pass "fm-pr-merge still refuses when a check's current run failed after an earlier pass" +} + +# Run generation follows startedAt rather than the order overlapping runs finish. +test_late_finishing_old_success_does_not_hide_current_failure() { + local case_dir rc head + head=dededededededededededededededededededede + case_dir=$(make_case github-old-success-finishes-last) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:01Z 2026-01-01T00:00:10Z)" \ + "$(check_run ci COMPLETED FAILURE 2026-01-01T00:00:09Z 2026-01-01T00:00:09Z)" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/98 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "github-old-success-finishes-last: the later-started failure must refuse" + assert_grep "check 'ci' is not green" "$case_dir/stderr" \ + "github-old-success-finishes-last: the current failure was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "github-old-success-finishes-last: completion order hid the current failure" + pass "fm-pr-merge uses start order when the old success finishes last" +} + +# A cancelled old run may settle after the passing re-run that superseded it. +test_late_finishing_old_cancellation_is_superseded() { + local case_dir head + head=dfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdfdf + case_dir=$(make_case github-old-cancellation-finishes-last) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED CANCELLED 2026-01-01T00:00:01Z 2026-01-01T00:00:10Z)" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:09Z 2026-01-01T00:00:09Z)" + + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/99 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "github-old-cancellation-finishes-last: the passing re-run must merge"$'\n'"$(cat "$case_dir/stderr")" + assert_logged_gh_merge "$case_dir" 99 example/repo --squash + pass "fm-pr-merge supersedes an old cancellation that finishes last" +} + +# A re-run that has not finished proves nothing, so it can neither be superseded +# nor supersede: the check stays red whether the run it replaces passed or failed. +test_unfinished_rerun_keeps_a_check_red() { + local case_dir rc head prior + head=eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee + for prior in FAILURE SUCCESS; do + case_dir=$(make_case "github-pending-rerun-$prior") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED "$prior" 2026-01-01T00:00:01Z)" \ + "$(check_run ci IN_PROGRESS - -)" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/92 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "github-pending-rerun-$prior: an unfinished re-run must refuse" + assert_grep "check 'ci' is not green" "$case_dir/stderr" \ + "github-pending-rerun-$prior: the pending check was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "github-pending-rerun-$prior: gh pr merge ran with a re-run still in flight" + done + pass "fm-pr-merge keeps a check red while its re-run is still in flight" +} + +# Supersession is scoped to one check name, which is also the name --allow-red +# matches, so a newer passing check never clears a different check's failure. +test_supersession_never_crosses_check_names() { + local case_dir rc head + head=ffffffffffffffffffffffffffffffffffffffff + case_dir=$(make_case github-cross-name) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run lint COMPLETED FAILURE 2026-01-01T00:00:01Z)" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:09Z)" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/93 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "github-cross-name: another check passing must not clear this failure" + assert_grep "check 'lint' is not green" "$case_dir/stderr" \ + "github-cross-name: the red check was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "github-cross-name: gh pr merge ran on a red check of a different name" + pass "fm-pr-merge never lets one check's pass clear another check's failure" +} + +# Supersession has to be proven from the forge's own start timestamps, so a run +# GitHub dated in any other way is treated as undated and clears nothing. +test_undated_runs_never_supersede() { + local case_dir rc spec label older newer + local head=0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a + set -- \ + 'undated-failure|-|2026-01-01T00:00:09Z' \ + 'undated-pass|2026-01-01T00:00:01Z|-' \ + 'fractional-pass|2026-01-01T00:00:01Z|2026-01-01T00:00:09.500Z' \ + 'offset-pass|2026-01-01T00:00:01Z|2026-01-01T00:00:09+00:00' + for spec in "$@"; do + label=${spec%%|*} + older=${spec#*|} + older=${older%%|*} + newer=${spec##*|} + case_dir=$(make_case "github-undated-$label") + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED FAILURE "$older")" \ + "$(check_run ci COMPLETED SUCCESS "$newer")" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/94 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "github-undated-$label: an unproven supersession must refuse" + assert_grep "check 'ci' is not green" "$case_dir/stderr" \ + "github-undated-$label: the red check was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "github-undated-$label: gh pr merge ran on an unproven supersession" + done + pass "fm-pr-merge clears a failure only on a proven later pass of the same check" +} + +# A superseded failure changes nothing about the waiver: --allow-red still covers +# exactly the named check, still needs every other check green, and the merge is +# still bound to the verified head. +test_allow_red_still_waives_only_the_current_failure() { + local case_dir rc head + head=0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b + case_dir=$(make_case github-superseded-allow-red-wrong-name) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED FAILURE 2026-01-01T00:00:01Z)" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:09Z)" \ + "$(check_run lint COMPLETED FAILURE 2026-01-01T00:00:09Z)" + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/95 \ + --allow-red ci > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 1 "$rc" "superseded-allow-red-wrong-name: waiving the green check must not merge" + assert_grep "check 'lint' is not green" "$case_dir/stderr" \ + "superseded-allow-red-wrong-name: the unwaived red check was not named" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "superseded-allow-red-wrong-name: gh pr merge ran with an unwaived red check" + + case_dir=$(make_case github-superseded-allow-red-named) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_github_rollup_json "$case_dir" "$head" \ + "$(check_run ci COMPLETED FAILURE 2026-01-01T00:00:01Z)" \ + "$(check_run ci COMPLETED SUCCESS 2026-01-01T00:00:09Z)" \ + "$(check_run lint COMPLETED FAILURE 2026-01-01T00:00:09Z)" + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/96 \ + --allow-red lint > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "superseded-allow-red-named: the named waiver should merge"$'\n'"$(cat "$case_dir/stderr")" + assert_logged_gh_merge "$case_dir" 96 example/repo --squash + pass "fm-pr-merge keeps --allow-red scoped to its named check beside a superseded failure" +} + test_allow_red_is_refused_while_away() { local case_dir rc head head=abababababababababababababababababababab @@ -2499,6 +2774,15 @@ test_untraversable_user_backend_config_directory_refuses_the_merge test_absent_user_backend_config_directory_and_backlog_still_merge test_backend_override_bypasses_unreadable_user_config test_github_red_checks_refuse_and_allow_red_waives_named +test_superseded_failed_check_run_no_longer_refuses +test_check_runs_never_supersede_status_contexts +test_current_failed_check_run_still_refuses +test_late_finishing_old_success_does_not_hide_current_failure +test_late_finishing_old_cancellation_is_superseded +test_unfinished_rerun_keeps_a_check_red +test_supersession_never_crosses_check_names +test_undated_runs_never_supersede +test_allow_red_still_waives_only_the_current_failure test_allow_red_is_refused_while_away test_allow_red_requires_one_separate_name test_away_grant_and_yolo_and_hold_for_return From de00521b9e671675aaf0a7481c51eac5e74ec868 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 12 Sep 2026 00:43:41 -0700 Subject: [PATCH 003/254] fix(bin): persist merge authority for poll-detected outcomes (#4266) * fix(merge): persist the merge authority on poll-detected merge outcomes The merge ledger tags a merge with the authority that permitted it while the away-posture record existed, but only the direct attended merge in bin/fm-pr-merge.sh recorded it. A merge the forge queued, or one the merge poll detected after the fact, published an untagged row, so exactly the merges no agent watched were the least auditable. bin/fm-merge-authority-lib.sh now owns that answer, read from the same structured sources the merge gate already used: the task's recorded yolo posture and the away-posture record's mechanical grant list, never prose. bin/fm-pr-merge.sh keeps its own refusal wording and gates on that answer; bin/fm-watch.sh only records it on the row its poll publishes, so reading the authority never becomes a second path to a merge. An unresolved answer records an untagged row rather than dropping the outcome or inventing an authority. * no-mistakes(review): Persist canonical merge authority for queued poll outcomes * no-mistakes(review): Harden merge authority persistence against lifecycle races * no-mistakes(review): Serialize poll authority publication with teardown * no-mistakes(document): Clarify persisted merge authority lifecycle * no-mistakes(ci): Added targeted SC2034 suppressions for the two public result assignments in bin/fm-merge-authority-lib.sh. Verified successfully with `CI=true bin/fm-lint.sh` --- AGENTS.md | 1 + bin/fm-merge-authority-lib.sh | 201 ++++++++++++++++++++ bin/fm-merge-outcome-lib.sh | 13 +- bin/fm-pr-merge.sh | 83 +++++--- bin/fm-teardown.sh | 10 +- bin/fm-watch.sh | 37 +++- docs/architecture.md | 3 + docs/scripts.md | 1 + tests/fm-pr-check-security.test.sh | 296 ++++++++++++++++++++++++++++- 9 files changed, 602 insertions(+), 43 deletions(-) create mode 100755 bin/fm-merge-authority-lib.sh diff --git a/AGENTS.md b/AGENTS.md index 3abd8348a28..0c7cf568b99 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -113,6 +113,7 @@ state/ runtime records and signals; gitignored .pr-poll private validated data sidecar for the byte-static PR merge poll .pr-poll-registration private transactional provenance record binding the task, canonical metadata identity, sidecar, and static poll publication .pr-poll-retirement private identity-bound crash-recovery receipt for one exact validated merged result; removed after its poll artifacts retire + .merge-authority private canonical-PR-bound authority persisted after firstmate's forge merge request is accepted and consumed by a later merged poll; bin/fm-merge-authority-lib.sh owns its format and lifecycle .pr-poll-merge-notified canonical PR identity of the last merge outcome delivered for this task; bin/fm-pr-lib.sh owns the marker format and identity mechanics, while bin/fm-merge-outcome-lib.sh owns locked publication, duplicate suppression, and replacement branch-outcomes.jsonl .branch-outcomes-cursor .branch-outcomes-processed ..branch-outcome-index .branch-outcome-index-ready Pi supervision-branch durable outcome store, its read cursor, main's processed marker, bounded latest per-task status-coverage caches, and their recovery marker; bin/fm-branch-outcome.sh owns the formats branch-session/ .branch-session .branch-mirror-cursor the branch's per-main-session conversations, the pointer to the current one, and the dialog-mirror cursor; extension-owned (docs/pi-supervision-branch.md) diff --git a/bin/fm-merge-authority-lib.sh b/bin/fm-merge-authority-lib.sh new file mode 100755 index 00000000000..9dbbadda2b1 --- /dev/null +++ b/bin/fm-merge-authority-lib.sh @@ -0,0 +1,201 @@ +#!/usr/bin/env bash +# Durable ownership of the authority under which a task's merge was accepted. +# +# The away-posture record (state/.afk-contract) and the task's recorded yolo +# posture are resolved only at the merge gate. After a forge accepts the merge, +# bin/fm-pr-merge.sh persists that answer as: +# state/.merge-authority +# fm-merge-authority-v1 +# +# +# +# +# yolo | away-grant | attended +# The identity comes from the merge run's immutable canonical URL parse; +# persistence revalidates the task's current pr= metadata under its metadata +# and lifecycle locks and refuses a mismatch. The file is atomically published, +# mode 0600, single-link, and on the state filesystem. A poll consumes it only +# when all identity fields match its own validated snapshot. Missing, malformed, +# or mismatched state means external; it is never resolved again from a later +# away-posture record. +# +# Resolution authorizes nothing by itself. bin/fm-pr-merge.sh owns the merge +# gate and persists only after a forge command succeeds, before releasing the +# task lifecycle lock. After observing a landed merge, bin/fm-watch.sh acquires +# that same lock, revalidates the poll, publishes its durable outcome, and +# retires only the exact authority record it read. Teardown uses the same lock, +# so it cannot interleave with that consumption transaction, and removes any +# remaining record. +# +# Sourced by those scripts and by tests. No side effects on source beyond its +# sourced libraries. + +_FM_MERGE_AUTHORITY_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=bin/fm-pr-lib.sh +. "$_FM_MERGE_AUTHORITY_LIB_DIR/fm-pr-lib.sh" +# shellcheck source=bin/fm-afk-contract.sh +. "$_FM_MERGE_AUTHORITY_LIB_DIR/fm-afk-contract.sh" + +# shellcheck disable=SC2034 # Public results consumed by sourcing callers. +FM_MERGE_AUTHORITY= +# shellcheck disable=SC2034 # Public results consumed by sourcing callers. +FM_MERGE_AUTHORITY_REASON= +# shellcheck disable=SC2034 # Public results consumed by sourcing callers. +FM_MERGE_AUTHORITY_RECORD_IDENTITY= + +fm_merge_authority_resolve() { # + local home=${1-} state=${2-} meta=${3-} id=${4-} + local yolo='' grants grant + FM_MERGE_AUTHORITY= + FM_MERGE_AUTHORITY_REASON='invalid' + [ -n "$home" ] && [ -n "$state" ] && [ -n "$meta" ] && [ -n "$id" ] || return 1 + + if ! fm_afk_contract_present "$state"; then + FM_MERGE_AUTHORITY='attended' + FM_MERGE_AUTHORITY_REASON='attended' + return 0 + fi + if ! FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ + "$_FM_MERGE_AUTHORITY_LIB_DIR/fm-afk-contract.sh" validate >/dev/null 2>&1; then + FM_MERGE_AUTHORITY_REASON='record-unreadable' + return 1 + fi + if [ -f "$meta" ]; then + yolo=$(grep '^yolo=' "$meta" | tail -1 | cut -d= -f2- || true) + fi + if [ "$yolo" = on ]; then + FM_MERGE_AUTHORITY='yolo' + FM_MERGE_AUTHORITY_REASON='granted' + return 0 + fi + grants=$(FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ + "$_FM_MERGE_AUTHORITY_LIB_DIR/fm-afk-contract.sh" grants 2>/dev/null) || { + FM_MERGE_AUTHORITY_REASON='grants-unreadable' + return 1 + } + while IFS= read -r grant; do + [ "$grant" = "$id" ] || continue + FM_MERGE_AUTHORITY='away-grant' + FM_MERGE_AUTHORITY_REASON='granted' + return 0 + done < + local record=$1 device=$2 expected_provider=$3 expected_host=$4 expected_path=$5 expected_number=$6 + local version provider host path number authority + fm_pr_private_file_valid "$record" 600 "$device" || return 1 + exec 8< "$record" || return 1 + IFS= read -r version <&8 || { exec 8<&-; return 1; } + IFS= read -r provider <&8 || { exec 8<&-; return 1; } + IFS= read -r host <&8 || { exec 8<&-; return 1; } + IFS= read -r path <&8 || { exec 8<&-; return 1; } + IFS= read -r number <&8 || { exec 8<&-; return 1; } + IFS= read -r authority <&8 || { exec 8<&-; return 1; } + if IFS= read -r _extra <&8; then + exec 8<&- + return 1 + fi + exec 8<&- + case "$authority" in yolo|away-grant|attended) ;; *) return 1 ;; esac + [ "$version" = fm-merge-authority-v1 ] \ + && [ "$provider" = "$expected_provider" ] \ + && [ "$host" = "$expected_host" ] \ + && [ "$path" = "$expected_path" ] \ + && [ "$number" = "$expected_number" ] || return 1 + FM_MERGE_AUTHORITY=$authority +} + +fm_merge_authority_persist() { # + local state=$1 id=$2 meta=$3 provider=$4 host=$5 path=$6 number=$7 authority=$8 + local record tmp='' state_device lock status=0 + fm_pr_task_id_valid "$id" || return 1 + case "$authority" in yolo|away-grant|attended) ;; *) return 1 ;; esac + [ -d "$state" ] && [ ! -L "$state" ] || return 1 + state_device=$(fm_pr_file_device "$state") || return 1 + fm_pr_metadata_identity_parse "$meta" || return 1 + [ "$FM_PR_META_PROVIDER" = "$provider" ] \ + && [ "$FM_PR_META_HOST" = "$host" ] \ + && [ "$FM_PR_META_PATH" = "$path" ] \ + && [ "$FM_PR_META_NUMBER" = "$number" ] || return 1 + record="$state/$id.merge-authority" + lock="$record.lock" + fm_lock_acquire_wait "$lock" || return 1 + fm_pr_regular_destination_on_device_or_absent "$record" "$state_device" || status=1 + if [ "$status" -eq 0 ]; then + umask 077 + tmp=$(mktemp "$state/.fm-merge-authority.XXXXXX") || status=1 + fi + if [ "$status" -eq 0 ]; then + printf '%s\n%s\n%s\n%s\n%s\n%s\n' \ + fm-merge-authority-v1 "$provider" "$host" "$path" "$number" "$authority" > "$tmp" \ + || status=1 + fi + if [ "$status" -eq 0 ]; then + chmod 0600 "$tmp" \ + && fm_merge_authority_record_matches "$tmp" "$state_device" \ + "$provider" "$host" "$path" "$number" \ + && fm_pr_regular_destination_on_device_or_absent "$record" "$state_device" \ + && mv -f -- "$tmp" "$record" \ + && fm_merge_authority_record_matches "$record" "$state_device" \ + "$provider" "$host" "$path" "$number" \ + || status=1 + fi + [ "$status" -eq 0 ] || rm -f -- "$tmp" + fm_lock_release "$lock" || status=1 + return "$status" +} + +fm_merge_authority_read() { # + local state=$1 id=$2 provider=$3 host=$4 path=$5 number=$6 + local record state_device lock status=0 + FM_MERGE_AUTHORITY='external' + FM_MERGE_AUTHORITY_RECORD_IDENTITY= + fm_pr_task_id_valid "$id" || return 1 + [ -d "$state" ] && [ ! -L "$state" ] || return 1 + state_device=$(fm_pr_file_device "$state") || return 1 + record="$state/$id.merge-authority" + lock="$record.lock" + fm_lock_acquire_wait "$lock" || return 1 + if fm_merge_authority_record_matches "$record" "$state_device" \ + "$provider" "$host" "$path" "$number"; then + # shellcheck disable=SC2034 # Public results consumed by sourcing callers. + FM_MERGE_AUTHORITY_RECORD_IDENTITY=$(fm_pr_file_identity "$record") || status=1 + else + FM_MERGE_AUTHORITY='external' + status=1 + fi + fm_lock_release "$lock" || status=1 + return "$status" +} + +fm_merge_authority_remove_if_matches() { # + local state=$1 id=$2 provider=$3 host=$4 path=$5 number=$6 + local authority=$7 expected_file_identity=$8 record state_device lock current_file_identity status=0 + fm_pr_task_id_valid "$id" || return 1 + [ -d "$state" ] && [ ! -L "$state" ] || return 1 + state_device=$(fm_pr_file_device "$state") || return 1 + record="$state/$id.merge-authority" + lock="$record.lock" + fm_lock_acquire_wait "$lock" || return 1 + if [ -e "$record" ] || [ -L "$record" ]; then + if fm_merge_authority_record_matches "$record" "$state_device" \ + "$provider" "$host" "$path" "$number"; then + current_file_identity=$(fm_pr_file_identity "$record") || status=1 + if [ "$status" -eq 0 ] \ + && [ "$FM_MERGE_AUTHORITY" = "$authority" ] \ + && [ "$current_file_identity" = "$expected_file_identity" ]; then + rm -f -- "$record" || status=1 + fi + elif ! fm_pr_private_file_valid "$record" 600 "$state_device"; then + status=1 + fi + fi + fm_lock_release "$lock" || status=1 + return "$status" +} diff --git a/bin/fm-merge-outcome-lib.sh b/bin/fm-merge-outcome-lib.sh index bf1f26c9c17..db279351145 100755 --- a/bin/fm-merge-outcome-lib.sh +++ b/bin/fm-merge-outcome-lib.sh @@ -42,10 +42,11 @@ FM_MERGE_OUTCOME_ALREADY_RECORDED=false # self - this home performed the merge. # poll - this home's merge poll detected the merge, so the canonical outcome # also wakes this home after any upward hop needed by a secondmate. -# Optional is yolo or away-grant when the merge ran while the -# away-posture record existed; it is appended to the ledger line. Known audit -# gap: queued merges and a poll that wins direct-merge deduplication publish an -# untagged row because the poll path does not persist merge authority. +# Optional is yolo, away-grant, attended, or external. Yolo, +# away-grant, and external are appended to the ledger line; attended remains +# untagged. The merge entrypoint supplies its authority after forge acceptance, +# while the poll supplies the persisted identity-bound value or external when +# no matching record proves that this home authorized the merge. # # Returns 0 when the outcome is recorded (or already was), 2 on an invalid # request, 3 when this home's own role or parent binding cannot be read well @@ -62,8 +63,8 @@ fm_merge_outcome_report() { # [autho FM_MERGE_OUTCOME_ALREADY_RECORDED=false case "$origin" in self|poll) ;; *) return 2 ;; esac case "$authority" in - yolo|away-grant) suffix=" $authority" ;; - '') ;; + yolo|away-grant|external) suffix=" $authority" ;; + attended|'') ;; *) return 2 ;; esac fm_pr_task_id_valid "$id" || return 2 diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 0b8aa79aeb2..85319a13777 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -73,8 +73,10 @@ # held for the captain return. An unreadable record refuses rather than being # skipped. Neither posture releases a captain hold, and the grant lapses when # the record is archived. -# The lock ends when the local forge command returns; docs/captain-hold-lifecycle.md owns -# the accepted asynchronous-landing and merge-to-cleanup residuals. +# A failed forge command releases the lock after it returns. A successful one +# retains the lock until the accepted merge authority is persisted against the +# still-matching task metadata; docs/captain-hold-lifecycle.md owns the accepted +# asynchronous-landing and merge-to-cleanup residuals. # # Extra args must not include --repo or -R in any form, including a bundled # short-option cluster such as -yR, because the repository comes only from the @@ -109,6 +111,8 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" . "$SCRIPT_DIR/fm-backlog-transition-lib.sh" # shellcheck source=bin/fm-merge-outcome-lib.sh . "$SCRIPT_DIR/fm-merge-outcome-lib.sh" +# shellcheck source=bin/fm-merge-authority-lib.sh +. "$SCRIPT_DIR/fm-merge-authority-lib.sh" # shellcheck source=bin/fm-afk-contract.sh . "$SCRIPT_DIR/fm-afk-contract.sh" @@ -124,6 +128,8 @@ if ! fm_pr_task_id_valid "$ID" || ! fm_pr_url_parse "$RAW_URL"; then fi URL=$FM_PR_URL PROVIDER=$FM_PR_PROVIDER +PR_HOST=$FM_PR_HOST +PR_PATH=$FM_PR_PATH PR_OWNER=$FM_PR_OWNER PR_REPO=$FM_PR_REPO PR_NUMBER=$FM_PR_NUMBER @@ -295,7 +301,9 @@ fi MERGE_EXPECTED_SPAWN_GEN=$FM_BACKLOG_META_SPAWN_GEN MERGE_CONTROL_LOCK= +MERGE_META_LOCK= merge_control_cleanup() { + [ -z "$MERGE_META_LOCK" ] || fm_lock_release "$MERGE_META_LOCK" || true [ -z "$MERGE_CONTROL_LOCK" ] || fm_lock_release "$MERGE_CONTROL_LOCK" || true } trap merge_control_cleanup EXIT @@ -825,33 +833,27 @@ require_released_captain_hold() { } FM_PR_MERGE_AUTHORITY= +# The gate on top of the shared authority read. bin/fm-merge-authority-lib.sh +# owns what the away-posture record and the task's recorded yolo posture say; +# this function owns what a merge run may do about it, so the answer the merge +# poll later tags its ledger row with is the same answer gated here. require_away_merge_grant() { - local yolo grants grant FM_PR_MERGE_AUTHORITY= - fm_afk_contract_present "$STATE" || return 0 - if ! FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" \ - "$SCRIPT_DIR/fm-afk-contract.sh" validate >/dev/null 2>&1; then - echo "error: PR merge refused - the away-posture record could not be read; nothing was merged" >&2 - return 1 - fi - yolo=$(grep '^yolo=' "$META" | tail -1 | cut -d= -f2- || true) - if [ "$yolo" = on ]; then - FM_PR_MERGE_AUTHORITY=yolo + if fm_merge_authority_resolve "$FM_HOME" "$STATE" "$META" "$ID"; then + FM_PR_MERGE_AUTHORITY=$FM_MERGE_AUTHORITY return 0 fi - grants=$(FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" \ - "$SCRIPT_DIR/fm-afk-contract.sh" grants 2>/dev/null) || { - echo "error: PR merge refused - the away-posture record's grants could not be read; nothing was merged" >&2 - return 1 - } - while IFS= read -r grant; do - [ "$grant" = "$ID" ] || continue - FM_PR_MERGE_AUTHORITY=away-grant - return 0 - done <&2 + case "$FM_MERGE_AUTHORITY_REASON" in + record-unreadable) + echo "error: PR merge refused - the away-posture record could not be read; nothing was merged" >&2 + ;; + grants-unreadable) + echo "error: PR merge refused - the away-posture record's grants could not be read; nothing was merged" >&2 + ;; + *) + echo "error: task $ID is held for the captain return" >&2 + ;; + esac return 1 } @@ -863,6 +865,23 @@ require_current_away_authority() { fi } +persist_accepted_merge_authority() { + local status=0 + MERGE_META_LOCK=$(fm_meta_lock_path "$META") || return 1 + fm_lock_acquire_wait "$MERGE_META_LOCK" || return 1 + fm_merge_authority_persist "$STATE" "$ID" "$META" \ + "$PROVIDER" "$PR_HOST" "$PR_PATH" "$PR_NUMBER" "$FM_PR_MERGE_AUTHORITY" \ + || status=1 + fm_lock_release "$MERGE_META_LOCK" || status=1 + MERGE_META_LOCK= + if [ "$status" -eq 0 ]; then + return 0 + fi + printf 'actionable: the forge accepted the merge request for %s but its merge authority could not be persisted; the merge poll remains armed\n' \ + "$URL" >&2 + return 1 +} + require_recorded_pr_identity() { local existing existing=$(grep '^pr=' "$META" | tail -1 | cut -d= -f2- || true) @@ -1024,11 +1043,14 @@ case "$PROVIDER" in merge_output=$(gh pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ --match-head-commit "$FM_PR_MERGE_HEAD" \ "${merge_args[@]+"${merge_args[@]}"}" "$@" 2>&1) || merge_status=$? - fm_lock_release "$MERGE_CONTROL_LOCK" || true - MERGE_CONTROL_LOCK= if [ "$merge_status" -eq 0 ]; then FM_PR_GITHUB_MERGE_ACCEPTED=true + persist_accepted_merge_authority || exit 1 + fm_lock_release "$MERGE_CONTROL_LOCK" || true + MERGE_CONTROL_LOCK= else + fm_lock_release "$MERGE_CONTROL_LOCK" || true + MERGE_CONTROL_LOCK= [ -z "$merge_output" ] || printf '%s\n' "$merge_output" >&2 if github_read_outcome; then if [ "$FM_PR_GITHUB_MERGED" != true ] && [ "$FM_PR_GITHUB_QUEUED" != true ]; then @@ -1071,9 +1093,14 @@ case "$PROVIDER" in merge_status=0 GITLAB_HOST="$FM_PR_HOST" glab mr merge "$PR_NUMBER" -R "$PROJECT_URL" \ --sha "$FM_PR_MERGE_HEAD" --yes "$@" || merge_status=$? + if [ "$merge_status" -ne 0 ]; then + fm_lock_release "$MERGE_CONTROL_LOCK" || true + MERGE_CONTROL_LOCK= + exit "$merge_status" + fi + persist_accepted_merge_authority || exit 1 fm_lock_release "$MERGE_CONTROL_LOCK" || true MERGE_CONTROL_LOCK= - [ "$merge_status" -eq 0 ] || exit "$merge_status" gitlab_confirm_rc=0 gitlab_confirm_merged || gitlab_confirm_rc=$? [ "$gitlab_confirm_rc" -eq 0 ] || exit 0 diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 7eaabe22d5a..1c94623f8dc 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -1207,7 +1207,7 @@ validate_pr_poll_cleanup() { fm_task_id_path_safe "$id" || return 0 for artifact in "$state_dir/$id.check.sh" "$state_dir/$id.pr-poll" \ "$state_dir/$id.pr-poll-registration" "$state_dir/$id.pr-poll-retirement" \ - "$state_dir/$id.check-trust"; do + "$state_dir/$id.merge-authority" "$state_dir/$id.check-trust"; do [ -e "$artifact" ] || [ -L "$artifact" ] || continue has_artifact=1 done @@ -1216,11 +1216,13 @@ validate_pr_poll_cleanup() { state_device=$(fm_pr_file_device "$state_dir") || return 1 for artifact in "$state_dir/$id.check.sh" "$state_dir/$id.pr-poll" \ "$state_dir/$id.pr-poll-registration" "$state_dir/$id.pr-poll-retirement" \ - "$state_dir/$id.check-trust"; do + "$state_dir/$id.merge-authority" "$state_dir/$id.check-trust"; do [ -e "$artifact" ] || [ -L "$artifact" ] || continue if [ ! -f "$artifact" ] || [ -L "$artifact" ] \ || [ "$(fm_pr_file_device "$artifact")" != "$state_device" ] \ - || [ "$(fm_pr_file_link_count "$artifact")" != 1 ]; then + || [ "$(fm_pr_file_link_count "$artifact")" != 1 ] \ + || { [ "$artifact" = "$state_dir/$id.merge-authority" ] \ + && [ "$(fm_pr_file_mode "$artifact")" != 600 ]; }; then echo "REFUSED: unsafe task PR-check artifact; preserving task state." >&2 return 1 fi @@ -1241,7 +1243,7 @@ remove_pr_poll_artifacts() { fm_pr_poll_merge_notified_remove "$state_dir" "$id" || return 1 rm -f "$state_dir/$id.check.sh" "$state_dir/$id.pr-poll" \ "$state_dir/$id.pr-poll-registration" "$state_dir/$id.pr-poll-retirement" \ - "$state_dir/$id.check-trust" || return 1 + "$state_dir/$id.merge-authority" "$state_dir/$id.check-trust" || return 1 } # Resolve the PR number for a worktree branch via gh-axi. Echoes the number on a diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 2a8e02b735a..bdd720a800d 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -147,6 +147,10 @@ mkdir -p "$STATE" # worker while adding no uncovered file. # shellcheck source=/dev/null . "$SCRIPT_DIR/fm-merge-outcome-lib.sh" +# The durable merge-authority owner is shared with bin/fm-pr-merge.sh. The +# watcher consumes only its identity-bound record after a poll observes landing. +# shellcheck source=/dev/null +. "$SCRIPT_DIR/fm-merge-authority-lib.sh" # shellcheck source=bin/fm-x-lib.sh . "$SCRIPT_DIR/fm-x-lib.sh" # shellcheck source=bin/fm-check-lib.sh @@ -1841,8 +1845,16 @@ reconcile_requests_detached() { RECONCILE_REQUEST_PID=$! } +PR_POLL_CONTROL_LOCK= + +pr_poll_control_release() { + [ -z "$PR_POLL_CONTROL_LOCK" ] || fm_lock_release "$PR_POLL_CONTROL_LOCK" || return 1 + PR_POLL_CONTROL_LOCK= +} + watcher_cleanup() { local cleanup_status=0 owns_lock=0 transition=release-lock + pr_poll_control_release || cleanup_status=1 if [ "$(cat "$WATCH_LOCK/pid" 2>/dev/null || true)" = "${WATCHER_PID:-}" ]; then owns_lock=1 if [ "${WATCHER_RECOVERY_PENDING:-0}" -eq 1 ] \ @@ -2012,6 +2024,13 @@ while :; do host=$FM_PR_POLL_SNAPSHOT_HOST path=$FM_PR_POLL_SNAPSHOT_PATH number=$FM_PR_POLL_SNAPSHOT_NUMBER + PR_POLL_CONTROL_LOCK="$STATE/.control-$id.lock" + fm_lock_acquire_wait "$PR_POLL_CONTROL_LOCK" || exit 1 + if ! fm_pr_poll_snapshot_matches "$STATE" "$id" "$SCRIPT_DIR/fm-pr-poll.sh"; then + pr_poll_control_release || exit 1 + triage_log "PR poll for $id changed before its validated check; skipping the stale snapshot" + continue + fi run_check_capture "$SCRIPT_DIR/fm-pr-poll.sh" --validated \ "$provider" "$url" "$host" "$path" "$number" || exit 1 out=$FM_CHECK_RESULT @@ -2029,14 +2048,28 @@ while :; do if [ -n "$out" ]; then reason="check: $c: $out" if [ "$is_pr_poll" -eq 1 ] && [ "$out" = merged ]; then + if ! fm_merge_authority_read "$STATE" "$id" \ + "$provider" "$host" "$path" "$number"; then + triage_log "no matching persisted merge authority for $id; recording an external merge outcome" + fi + merge_authority=$FM_MERGE_AUTHORITY + merge_authority_record_identity=$FM_MERGE_AUTHORITY_RECORD_IDENTITY merge_outcome_rc=0 fm_merge_outcome_report "$FM_HOME" "$STATE" "$id" "$url" poll \ - || merge_outcome_rc=$? + "$merge_authority" || merge_outcome_rc=$? if [ "$merge_outcome_rc" -ne 0 ]; then triage_log "merge outcome for $id could not be recorded (rc=$merge_outcome_rc)" exit 1 fi + if [ -n "$merge_authority_record_identity" ] \ + && ! fm_merge_authority_remove_if_matches "$STATE" "$id" \ + "$provider" "$host" "$path" "$number" "$merge_authority" \ + "$merge_authority_record_identity"; then + triage_log "published merge outcome for $id but could not retire its authority record" + exit 1 + fi retire_merged_pr_poll "$id" + pr_poll_control_release || exit 1 touch "$STATE/.last-check" if [ "$FM_MERGE_OUTCOME_ALREADY_RECORDED" = true ]; then triage_log "absorbed duplicate merged PR poll result for $id" @@ -2044,10 +2077,12 @@ while :; do fi wake "$reason" fi + pr_poll_control_release || exit 1 fm_wake_append check "$c" "$reason" || exit 1 touch "$STATE/.last-check" wake "$reason" fi + pr_poll_control_release || exit 1 done if [ -n "$rejected_checks" ]; then reason="check: rejected unauthenticated state checks:$rejected_checks" diff --git a/docs/architecture.md b/docs/architecture.md index a02e4b9a1e5..ef1b1519c54 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -327,6 +327,9 @@ An auto-merge request is held to the same standard: `--auto` that leaves the pul Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. A confirmed merge leaves a durable role-routed outcome instead of living only in the merging agent's memory, and [`bin/fm-merge-outcome-lib.sh`](../bin/fm-merge-outcome-lib.sh)'s header owns its destination, shape, identity, normal-case deduplication, and at-least-once recovery. The same emitter handles a merge firstmate performed and one its poll detected, while the watcher immediately delivers the emitter's local actionable poll row. +After the forge accepts firstmate's merge request, the merge path persists the resolved yolo, away-grant, or attended authority bound to the task's canonical PR identity. +A later merged poll consumes only that matching persisted value; with no match it records the landing as external rather than consulting a live away-posture record that may have been archived or replaced. +[`bin/fm-merge-authority-lib.sh`](../bin/fm-merge-authority-lib.sh)'s header owns resolution, private atomic persistence, identity-checked consumption, and retirement, while only the merge path gates on the answer. Teardown is fail-closed for ship worktrees: dirty worktrees refuse, and committed work must be landed before the worktree is returned. A pool worktree is only returned after teardown passes the slot-ownership proof: a contradictory task record or a supported live endpoint refuses without touching either task, and no discard authority relaxes that. A slot's own owner claim, written by the spawn that takes it under the allocation lock and owned by [`bin/fm-wake-lib.sh`](../bin/fm-wake-lib.sh), covers a slot reassigned to a task that left no record the scan could reach: a claim naming a different task releases nothing - teardown warns, names the claimant, and finishes only the task's own cleanup - because Treehouse's own live process lease cannot answer ownership once the worker's exit releases it. diff --git a/docs/scripts.md b/docs/scripts.md index 3f0694bac8d..048a064aa3c 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -132,6 +132,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-pr-check.sh` | Record validated `pr=` and `pr_head=` values, then atomically arm a static merge poll | | `fm-pr-merge.sh` | Record PR metadata, merge a task's canonical full GitHub or GitLab URL, then refuse an outcome it cannot prove landed or queued | | `fm-merge-outcome-lib.sh` | Publish a confirmed merge's durable, role-routed supervision outcome | +| `fm-merge-authority-lib.sh` | Resolve merge authority at the gate, persist it against the accepted canonical PR, and identity-check its later poll consumption | | `fm-parent-channel-lib.sh` | Resolve a secondmate home's parent channel and append a captain-facing outcome line to it at most once | | `fm-promote.sh` | Promote a scout task in place to a protected ship task with an explicit delivery mode, and write the ship instructions carrying that mode's definition of done | | `fm-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed scout deliverables, retire secondmate homes | diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index 240f26f40ba..40d8438fcb4 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -138,9 +138,9 @@ printf '%s\n' "$*" >> "$FM_TEST_GH_LOG" case "${1:-} ${2:-}" in "api graphql") printf '%s\n' \ - 'state=MERGED' \ - 'merged=true' \ - 'queued=false' \ + "state=${FM_TEST_GH_GRAPHQL_STATE:-MERGED}" \ + "merged=${FM_TEST_GH_GRAPHQL_MERGED:-true}" \ + "queued=${FM_TEST_GH_GRAPHQL_QUEUED:-false}" \ 'base=main' exit 0 ;; @@ -152,11 +152,16 @@ case "${1:-} ${2:-}" in ;; esac ;; + "pr merge") + [ -z "${FM_TEST_GH_MERGE_HOOK:-}" ] || "$FM_TEST_GH_MERGE_HOOK" + exit 0 + ;; esac case " $* " in *" headRefOid "*) printf '%s\n' "${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}" ;; *" state "*) [ "${FM_TEST_GH_FAIL:-0}" = 0 ] || exit 1 + [ -z "${FM_TEST_GH_STATE_STARTED:-}" ] || : > "$FM_TEST_GH_STATE_STARTED" [ "${FM_TEST_GH_SLEEP:-0}" = 0 ] || sleep "$FM_TEST_GH_SLEEP" printf '%s\n' "${FM_TEST_GH_STATE:-OPEN}" ;; @@ -201,10 +206,14 @@ write_task_meta() { "mode=no-mistakes" } +# Extra "field=value" arguments are written before pr=, because +# fm_pr_metadata_identity_parse rejects an unrecognised line after it. write_poll_meta() { local state=$1 id=$2 url=$3 + shift 3 fm_write_meta "$state/$id.meta" \ "window=fm-$id" \ + "$@" \ "pr=$url" } @@ -624,9 +633,10 @@ SH run_watcher_bounded() { local home=$1 fakebin=$2 check_interval=${FM_TEST_CHECK_INTERVAL:-0} watch_root=${FM_TEST_WATCH_ROOT:-$ROOT} + local check_timeout=${FM_TEST_CHECK_TIMEOUT:-1} shift 2 perl -e 'my $pid=fork; die unless defined $pid; if (!$pid) { exec @ARGV } local $SIG{ALRM}=sub { kill "TERM", $pid; waitpid $pid, 0; exit 124 }; alarm 10; waitpid $pid, 0; alarm 0; exit($? >> 8)' \ - env FM_HOME="$home" FM_ROOT_OVERRIDE="$watch_root" FM_CHECK_INTERVAL="$check_interval" FM_CHECK_TIMEOUT=1 \ + env FM_HOME="$home" FM_ROOT_OVERRIDE="$watch_root" FM_CHECK_INTERVAL="$check_interval" FM_CHECK_TIMEOUT="$check_timeout" \ FM_POLL=0.02 FM_HEARTBEAT=999999 FM_SIGNAL_GRACE=0 PATH="$fakebin:$BASE_PATH" "$WATCH" "$@" } @@ -2135,12 +2145,290 @@ test_gitlab_merged_poll_retires() { pass "GitHub and GitLab exact merged results share one retirement path" } +# --- poll-path merge authority ---------------------------------------------- + +write_away_record() { # [...] + local dir=$1 + shift + FM_HOME="$dir/home" FM_STATE_OVERRIDE="$dir/home/state" \ + "$ROOT/bin/fm-afk-contract.sh" propose "$@" >/dev/null \ + || fail "could not propose an away-posture record" + FM_HOME="$dir/home" FM_STATE_OVERRIDE="$dir/home/state" \ + "$ROOT/bin/fm-afk-contract.sh" confirm >/dev/null \ + || fail "could not confirm an away-posture record" +} + +archive_away_record() { # + FM_HOME="$1/home" FM_STATE_OVERRIDE="$1/home/state" \ + "$ROOT/bin/fm-afk-contract.sh" archive >/dev/null \ + || fail "could not archive the away-posture record" +} + +# The durable queue is TSV (epoch, sequence, kind, key, payload). +merged_ledger_row() { # + awk -F'\t' -v prefix="check: merge landed: $2 " \ + 'index($5, prefix) == 1 { print $5 }' "$1/.wake-queue" +} + +run_merged_poll_cycle() { # + local dir=$1 rc=0 + add_stop_custom_check "$dir" + set +e + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + > "$dir/watch.out" 2> "$dir/watch.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "merged poll watcher failed: $(cat "$dir/watch.err")" +} + +queue_merge() { # + local dir=$1 url=$2 rc=0 + set +e + FM_TEST_GH_GRAPHQL_STATE=OPEN FM_TEST_GH_GRAPHQL_MERGED=false \ + FM_TEST_GH_GRAPHQL_QUEUED=true \ + run_merge_entry "$dir" task-a "$url" > "$dir/merge.out" 2> "$dir/merge.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "queued merge failed: $(cat "$dir/merge.err")" + assert_grep "is queued" "$dir/merge.out" "the forge did not queue the merge" + [ -f "$dir/home/state/task-a.merge-authority" ] \ + || fail "the accepted queued merge did not persist its authority" +} + +test_merged_poll_row_carries_the_merge_authority() { + local dir state url expected posture + url=https://github.com/o/r/pull/1 + + for posture in yolo grant; do + dir=$(make_case "queued-merge-authority-$posture") + state="$dir/home/state" + write_task_meta "$dir" task-a + if [ "$posture" = yolo ]; then + printf 'yolo=on\n' >> "$state/task-a.meta" + write_away_record "$dir" + expected=yolo + else + write_away_record "$dir" --grant task-a + expected=away-grant + fi + run_check_entry "$dir" task-a "$url" >/dev/null 2> "$dir/seed.err" \ + || fail "$posture: could not arm the merge poll" + queue_merge "$dir" "$url" + archive_away_record "$dir" + run_merged_poll_cycle "$dir" + [ "$(merged_ledger_row "$state" task-a)" = "check: merge landed: task-a $url $expected" ] \ + || fail "$posture: archived posture lost persisted authority: $(merged_ledger_row "$state" task-a)" + [ ! -e "$state/task-a.merge-authority" ] \ + || fail "$posture: published merge left its authority record behind" + done + + pass "queued merges retain yolo and away-grant after captain return" +} + +test_merged_poll_row_names_no_authority_when_no_record_grants_one() { + local dir state url + url=https://github.com/o/r/pull/1 + + dir=$(make_case queued-merge-authority-attended) + state="$dir/home/state" + write_task_meta "$dir" task-a + run_check_entry "$dir" task-a "$url" >/dev/null 2> "$dir/seed.err" \ + || fail "attended: could not arm the merge poll" + queue_merge "$dir" "$url" + run_merged_poll_cycle "$dir" + [ "$(merged_ledger_row "$state" task-a)" = "check: merge landed: task-a $url" ] \ + || fail "attended queued merge was tagged: $(merged_ledger_row "$state" task-a)" + + dir=$(make_case merged-poll-authority-external) + state="$dir/home/state" + write_poll_meta "$state" task-a "$url" yolo=on + write_away_record "$dir" + seed_canonical_poll "$dir" task-a "$url" + run_merged_poll_cycle "$dir" + [ "$(merged_ledger_row "$state" task-a)" = "check: merge landed: task-a $url external" ] \ + || fail "external merge was attributed from live away posture: $(merged_ledger_row "$state" task-a)" + assert_poll_absent "$state" task-a + + pass "poll distinguishes attended authorization from external landing" +} + +test_authority_persistence_refuses_rebound_metadata() { + local dir state url_a url_b rc + url_a=https://github.com/o/r/pull/1 + url_b=https://github.com/o/r/pull/2 + dir=$(make_case merge-authority-rebound-metadata) + state="$dir/home/state" + write_task_meta "$dir" task-a + run_check_entry "$dir" task-a "$url_a" >/dev/null 2> "$dir/seed.err" \ + || fail "rebind: could not arm the original poll" + cat > "$dir/rebind.sh" </dev/null +SH + chmod +x "$dir/rebind.sh" + set +e + FM_TEST_GH_MERGE_HOOK="$dir/rebind.sh" \ + FM_TEST_GH_GRAPHQL_STATE=OPEN FM_TEST_GH_GRAPHQL_MERGED=false \ + FM_TEST_GH_GRAPHQL_QUEUED=true \ + run_merge_entry "$dir" task-a "$url_a" > "$dir/merge.out" 2> "$dir/merge.err" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "rebind: accepted merge persisted against rebound metadata" + grep -qxF "pr=$url_b" "$state/task-a.meta" \ + || fail "rebind: merge hook did not replace the canonical identity" + [ ! -e "$state/task-a.merge-authority" ] \ + || fail "rebind: authority was published for the wrong canonical identity" + pass "accepted merge authority refuses rebound task metadata" +} + +test_authority_persists_before_control_unlock() { + local dir state url + url=https://github.com/o/r/pull/1 + dir=$(make_case merge-authority-control-lock) + state="$dir/home/state" + write_task_meta "$dir" task-a + run_check_entry "$dir" task-a "$url" >/dev/null 2> "$dir/seed.err" \ + || fail "control lock: could not arm the merge poll" + cat > "$dir/fakebin/mv" <<'SH' +#!/usr/bin/env bash +case " $* " in + *"task-a.merge-authority "*) + [ -d "$FM_TEST_CONTROL_LOCK" ] || exit 91 + ;; +esac +exec "$FM_TEST_REAL_MV" "$@" +SH + chmod +x "$dir/fakebin/mv" + FM_TEST_CONTROL_LOCK="$state/.control-task-a.lock" FM_TEST_REAL_MV="$REAL_MV" \ + queue_merge "$dir" "$url" + pass "accepted merge authority persists under the lifecycle lock" +} + +test_teardown_cannot_race_authority_consumption() { + local dir state url watcher_pid rc i + url=https://github.com/o/r/pull/1 + dir=$(make_case merge-authority-teardown-race) + state="$dir/home/state" + fm_write_meta "$state/task-a.meta" \ + 'window=firstmate:fm-task-a' \ + 'endpoint_task_id=task-a' \ + "worktree=$dir/wt" \ + "project=$dir/project" \ + 'kind=ship' \ + 'mode=local-only' \ + 'yolo=on' + write_away_record "$dir" + run_check_entry "$dir" task-a "$url" >/dev/null 2> "$dir/seed.err" \ + || fail "teardown race: could not arm the merge poll" + queue_merge "$dir" "$url" + archive_away_record "$dir" + FM_TEST_GH_STATE_STARTED="$dir/poll-started" FM_TEST_GH_STATE=MERGED \ + FM_TEST_GH_SLEEP=0.5 FM_TEST_CHECK_TIMEOUT=3 \ + run_watcher_bounded "$dir/home" "$dir/fakebin" \ + > "$dir/watch.out" 2> "$dir/watch.err" & + watcher_pid=$! + i=0 + while [ ! -e "$dir/poll-started" ]; do + sleep 0.01 + i=$((i + 1)) + if [ "$i" -ge 500 ]; then + kill "$watcher_pid" 2>/dev/null || true + wait "$watcher_pid" 2>/dev/null || true + fail "teardown race: watcher did not begin its validated poll" + fi + done + set +e + FM_HOME="$dir/home" FM_ROOT_OVERRIDE="$ROOT" PATH="$dir/fakebin:$BASE_PATH" \ + "$TEARDOWN" task-a --force > "$dir/teardown.out" 2> "$dir/teardown.err" + rc=$? + set -e + [ "$rc" -ne 0 ] || fail "teardown race: cleanup crossed the active poll transaction" + [ -f "$state/task-a.merge-authority" ] \ + || fail "teardown race: refused cleanup removed persisted authority" + rc=0 + wait "$watcher_pid" || rc=$? + [ "$rc" -eq 0 ] || fail "teardown race: watcher failed with $rc: $(cat "$dir/watch.err")" + [ "$(merged_ledger_row "$state" task-a)" = "check: merge landed: task-a $url yolo" ] \ + || fail "teardown race: concurrent cleanup downgraded the merge authority" + pass "teardown cannot race merged-poll authority consumption" +} + +test_authority_retirement_preserves_replacement() { + local dir state url_a url_b rc i + url_a=https://github.com/o/r/pull/1 + url_b=https://github.com/o/r/pull/2 + dir=$(make_case merge-authority-retirement-replacement) + state="$dir/home/state" + write_task_meta "$dir" task-a + run_check_entry "$dir" task-a "$url_a" >/dev/null 2> "$dir/seed.err" \ + || fail "replacement: could not arm the original poll" + queue_merge "$dir" "$url_a" + cat > "$dir/replace-authority.sh" </dev/null +( + FM_TEST_GH_GRAPHQL_STATE=OPEN FM_TEST_GH_GRAPHQL_MERGED=false \\ + FM_TEST_GH_GRAPHQL_QUEUED=true \\ + "$PR_MERGE" task-a "$url_b" > "$dir/replacement-merge.out" 2> "$dir/replacement-merge.err" + printf '%s\n' \$? > "$dir/replacement-merge.rc" +) & +SH + chmod +x "$dir/replace-authority.sh" + cat > "$dir/fakebin/mv" <<'SH' +#!/usr/bin/env bash +"$FM_TEST_REAL_MV" "$@" || exit $? +case " $* " in + *"task-a.pr-poll-merge-notified "*) + if [ ! -e "$FM_TEST_REPLACEMENT_RAN" ]; then + : > "$FM_TEST_REPLACEMENT_RAN" + "$FM_TEST_REPLACEMENT_SCRIPT" + fi + ;; +esac +SH + chmod +x "$dir/fakebin/mv" + add_stop_custom_check "$dir" + set +e + FM_TEST_REAL_MV="$REAL_MV" FM_TEST_REPLACEMENT_RAN="$dir/replacement-ran" \ + FM_TEST_REPLACEMENT_SCRIPT="$dir/replace-authority.sh" \ + FM_TEST_GH_STATE=MERGED run_watcher_bounded "$dir/home" "$dir/fakebin" \ + > "$dir/watch-a.out" 2> "$dir/watch-a.err" + rc=$? + set -e + [ "$rc" -eq 0 ] || fail "replacement: original poll failed: $(cat "$dir/watch-a.err")" + i=0 + while [ ! -e "$dir/replacement-merge.rc" ]; do + sleep 0.01 + i=$((i + 1)) + [ "$i" -lt 200 ] || fail "replacement: serialized replacement merge did not finish" + done + [ "$(cat "$dir/replacement-merge.rc")" -eq 0 ] \ + || fail "replacement: serialized replacement merge failed: $(cat "$dir/replacement-merge.err")" + [ -f "$state/task-a.merge-authority" ] \ + || fail "replacement: original poll retirement deleted the replacement authority" + grep -qxF "pr=$url_b" "$state/task-a.meta" \ + || fail "replacement: replacement poll was not armed" + ack_watcher_cycle "$state" || fail "replacement: could not acknowledge the original wake" + rm -f "$dir/fakebin/mv" "$state/.last-check" + run_merged_poll_cycle "$dir" + awk -F'\t' -v expected="check: merge landed: task-a $url_b" \ + '$5 == expected { found=1 } END { exit !found }' "$state/.wake-queue" \ + || fail "replacement: replacement merge lost its attended authority" + pass "poll retirement preserves a replacement authority record" +} + test_parser_matrix test_gitlab_merge_watch test_merged_poll_retires_once test_merged_poll_reregistration_after_notification_is_absorbed test_merged_poll_retries_a_failed_upward_report test_self_merge_and_poll_publish_one_outcome +test_merged_poll_row_carries_the_merge_authority +test_merged_poll_row_names_no_authority_when_no_record_grants_one +test_authority_persistence_refuses_rebound_metadata +test_authority_persists_before_control_unlock +test_teardown_cannot_race_authority_consumption +test_authority_retirement_preserves_replacement test_merged_poll_reports_upward_from_a_secondmate_home_once test_different_merged_pr_for_same_task_is_not_absorbed test_persistent_secondmate_retirement_is_poll_only From 83c63cc6aa79d1cf913bb5ea988a75f867ffa322 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 12 Sep 2026 01:05:15 -0700 Subject: [PATCH 004/254] ci: supersede superseded PR CI and bound unbounded jobs (#4281) The 2026-09-12 Actions starvation incident found firstmate CI with no concurrency deduplication, so every superseded PR head kept its full 13-job fan-out, and four jobs with no timeout at all. Add per-PR supersession keyed on the PR number for pull_request events and on the unique run id for push events, cancelling only pull_request runs, so a new PR head replaces its own in-flight CI while every main push keeps its own group and is never cancelled. Add hang tripwires to the four previously unbounded jobs: 25 minutes for lint (measured at 14-16 minutes) and 5 minutes each for the coverage guard, the timing aggregate, and the repo invariants. Measured lane bounds are unchanged. tests/fm-ci-workflow.test.sh resolves the workflow's concurrency expressions against simulated pull_request and push contexts and holds every job's finite timeout. --- .github/workflows/ci.yml | 22 +++++ CONTRIBUTING.md | 1 + tests/fm-ci-workflow.test.sh | 159 +++++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+) create mode 100755 tests/fm-ci-workflow.test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5dcac6dff5a..24ade79a846 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,10 +9,26 @@ on: permissions: contents: read +# Per-PR supersession: a new push to the same PR replaces that PR's in-flight +# CI instead of letting superseded heads keep 13 jobs of hosted-runner work. +# The group uses the PR number for pull_request events, so every run of one PR +# shares a group, and falls back to the unique run id for push events, so each +# main push gets its own group and is never cancelled. Cancellation is likewise +# limited to pull_request events. Evidence and rationale: the September 12 +# Actions starvation report, section 3 "Workflow mechanics to ship first". +# The compliance workflow deliberately keeps its own event-specific groups; do +# not collapse it onto this simpler shape. +concurrency: + group: ci-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: lint: name: Lint runs-on: ubuntu-latest + # Hang tripwire only: lint executions measured at 14-16 minutes in the + # September 12 starvation report, so this leaves deliberate margin. + timeout-minutes: 25 steps: - uses: actions/checkout@v6 - name: Install pinned ShellCheck @@ -37,6 +53,8 @@ jobs: test-coverage: name: Test coverage guard runs-on: ubuntu-latest + # Hang tripwire: the coverage guard is a seconds-long local computation. + timeout-minutes: 5 steps: - uses: actions/checkout@v6 - name: Prove complete regression partition @@ -335,6 +353,8 @@ jobs: tests-timing-aggregate: name: Behavior timing aggregate runs-on: ubuntu-latest + # Hang tripwire: aggregation is seconds of work over lane artifacts. + timeout-minutes: 5 needs: - tests-portable-parallel-1 - tests-portable-parallel-2 @@ -433,6 +453,8 @@ jobs: invariants: name: Repo invariants runs-on: ubuntu-latest + # Hang tripwire: the invariant checks are seconds-long file comparisons. + timeout-minutes: 5 steps: - uses: actions/checkout@v6 - name: Compatibility pointers must stay intact diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c1c3d15ee91..87317bd5c45 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -107,6 +107,7 @@ Its header and `--help` own the flags, family labels, lanes, and changed-file ma Portable shard balance evidence lives in `docs/fm-test-portable-shards.md`. Family selection is the ordinary local path; `--all` is deliberate full regression only. CI owns broad regression across required portable parallel shards, the portable serial lane's separate-runner shards, the Herdr lane, lint, invariants, the coverage guard, and stock macOS Bash compatibility in [`.github/workflows/ci.yml`](.github/workflows/ci.yml). +Pushing a new head to a pull request cancels that pull request's still-running CI so only the current head is validated; pushes to `main` are never cancelled, and the workflow owns that contract and its rationale. Use `bin/fm-test-run.sh --list-lanes` for exact lane names and `--help` for `--jobs` rules and required gate-skip flags when reproducing a lane locally. Leave the `sleep 0.1` cadence in the suites' bounded condition waits alone. Those sleeps look like recoverable overhead - `fm-watch-triage.test.sh` alone issues about 1,900 of them, each paying a flat ~100ms scheduler wake-up penalty on macOS - but they are not overhead added to the clock; they are how a test waits for a subject that only moves on `fm-watch.sh`'s own one-second `FM_POLL` cadence. diff --git a/tests/fm-ci-workflow.test.sh b/tests/fm-ci-workflow.test.sh new file mode 100755 index 00000000000..fd2f7918493 --- /dev/null +++ b/tests/fm-ci-workflow.test.sh @@ -0,0 +1,159 @@ +#!/usr/bin/env bash +# Contract tests for .github/workflows/ci.yml's runner-spend safeguards. +# +# Origin: the 2026-09-12 GitHub Actions starvation incident. firstmate CI had no +# concurrency deduplication, so every superseded PR head kept its full job +# fan-out, and four jobs carried no timeout at all. These tests hold both +# safeguards: PR runs supersede within one PR while main pushes are never +# cancelled, and every CI job carries a finite hang tripwire. +# +# The workflow is parsed as YAML and its concurrency expressions are resolved +# against simulated pull_request and push contexts, so the assertions describe +# what GitHub would do, not how the file happens to be spelled. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +CI_WORKFLOW="$ROOT/.github/workflows/ci.yml" + +assert_present "$CI_WORKFLOW" ".github/workflows/ci.yml is missing" +command -v ruby >/dev/null 2>&1 \ + || fail "ruby is required to parse .github/workflows/ci.yml as YAML" + +# Resolve the workflow's concurrency contract under one simulated event and +# print "". Only the two expression constructs +# this workflow uses are resolved: an `a || b` fallback and an `==` comparison. +resolve_concurrency() { + local event=$1 pr_number=$2 run_id=$3 + ruby -ryaml -e ' +doc = YAML.load_file(ARGV[0]) +concurrency = doc.fetch("concurrency") +context = { + "github.workflow" => doc.fetch("name"), + "github.event_name" => ARGV[1], + "github.event.pull_request.number" => ARGV[2], + "github.run_id" => ARGV[3], +} + +value = lambda do |token| + token = token.strip + next token[1..-2] if token.start_with?("\x27") && token.end_with?("\x27") + raise "unresolvable context reference: #{token}" unless context.key?(token) + context.fetch(token) +end + +evaluate = lambda do |expression| + expression = expression.strip + if expression.include?("==") + left, right = expression.split("==", 2) + next value.call(left) == value.call(right) ? "true" : "false" + end + resolved = expression.split("||").map { |token| value.call(token) }.find { |v| !v.empty? } + resolved.to_s +end + +interpolate = lambda do |raw| + raw.to_s.gsub(/\$\{\{(.+?)\}\}/) { evaluate.call(Regexp.last_match(1)) } +end + +puts [interpolate.call(concurrency.fetch("group")), + interpolate.call(concurrency.fetch("cancel-in-progress"))].join("\t") +' "$CI_WORKFLOW" "$event" "$pr_number" "$run_id" +} + +job_timeout() { + ruby -ryaml -e ' +puts YAML.load_file(ARGV[0]).fetch("jobs").fetch(ARGV[1]).fetch("timeout-minutes", "none") +' "$CI_WORKFLOW" "$1" +} + +group_of() { printf '%s\n' "$1" | cut -f1; } +cancel_of() { printf '%s\n' "$1" | cut -f2; } + +test_pr_pushes_supersede_within_one_pr() { + local first second + first=$(resolve_concurrency pull_request 108 900001) || fail "could not resolve PR concurrency" + second=$(resolve_concurrency pull_request 108 900002) || fail "could not resolve PR concurrency" + [ "$(group_of "$first")" = "$(group_of "$second")" ] \ + || fail "two runs of one PR must share a concurrency group, got $(group_of "$first") and $(group_of "$second")" + [ "$(cancel_of "$first")" = true ] \ + || fail "PR runs must cancel the in-progress run, got $(cancel_of "$first")" + pass "a newer push to one PR supersedes that PR's in-flight CI" +} + +test_separate_prs_do_not_cancel_each_other() { + local one two + one=$(resolve_concurrency pull_request 108 900001) || fail "could not resolve PR concurrency" + two=$(resolve_concurrency pull_request 109 900003) || fail "could not resolve PR concurrency" + [ "$(group_of "$one")" != "$(group_of "$two")" ] \ + || fail "distinct PRs must not share a concurrency group ($(group_of "$one"))" + pass "distinct PRs get distinct concurrency groups" +} + +test_main_pushes_are_never_cancelled() { + local first second + first=$(resolve_concurrency push '' 900010) || fail "could not resolve push concurrency" + second=$(resolve_concurrency push '' 900011) || fail "could not resolve push concurrency" + [ "$(group_of "$first")" != "$(group_of "$second")" ] \ + || fail "each main push must get its own concurrency group, got $(group_of "$first") twice" + [ "$(cancel_of "$first")" = false ] \ + || fail "push runs must never cancel an in-progress run, got $(cancel_of "$first")" + pass "every main push keeps its own group and is never cancelled" +} + +test_every_job_has_a_finite_timeout() { + local reported + reported=$(ruby -ryaml -e ' +YAML.load_file(ARGV[0]).fetch("jobs").each do |name, job| + timeout = job["timeout-minutes"] + next if timeout.is_a?(Integer) && timeout > 0 + puts "#{name}: #{timeout.inspect}" +end +' "$CI_WORKFLOW") || fail "could not read job timeouts from ci.yml" + [ -z "$reported" ] || fail "these CI jobs have no finite hang tripwire:"$'\n'"$reported" + pass "every ci.yml job carries a finite timeout" +} + +# The four jobs the incident found unbounded, at the report's recommended caps. +test_previously_unbounded_jobs_keep_their_caps() { + local job expected actual + while read -r job expected; do + [ -n "$job" ] || continue + actual=$(job_timeout "$job") || fail "could not read the $job timeout" + [ "$actual" = "$expected" ] \ + || fail "$job timeout must stay $expected minutes, got $actual" + done <<'CAPS' +lint 25 +test-coverage 5 +tests-timing-aggregate 5 +invariants 5 +CAPS + pass "the incident's unbounded jobs keep their recommended caps" +} + +# Cancellation makes an undersized cap costlier: a falsely tripped job now also +# discards a run nobody replaced. These bounds were measured, not guessed. +test_measured_lanes_keep_their_existing_bounds() { + local job expected actual + while read -r job expected; do + [ -n "$job" ] || continue + actual=$(job_timeout "$job") || fail "could not read the $job timeout" + [ "$actual" = "$expected" ] \ + || fail "$job timeout must stay $expected minutes, got $actual" + done <<'CAPS' +tests-portable-parallel-1 10 +tests-portable-parallel-2 10 +tests-portable-serial 30 +tests-herdr 75 +macos-stock-bash 10 +CAPS + pass "the already-measured lane bounds are unchanged" +} + +test_pr_pushes_supersede_within_one_pr +test_separate_prs_do_not_cancel_each_other +test_main_pushes_are_never_cancelled +test_every_job_has_a_finite_timeout +test_previously_unbounded_jobs_keep_their_caps +test_measured_lanes_keep_their_existing_bounds From fa65b5df10745515b4c1a897e8cc1dbfd95309b0 Mon Sep 17 00:00:00 2001 From: nateliuroberts Date: Sat, 12 Sep 2026 03:41:12 -0700 Subject: [PATCH 005/254] test(watch): gate backlog-hold away-record fixture on tasks-axi (#4288) Every other make_hold_home caller in this file skips when tasks-axi is absent; this test was the one unguarded call, so hosts without tasks-axi hard-fail the fixture build instead of skipping. --- tests/fm-watch-triage.test.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index c4f3bd428d0..8093b733c7d 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -4665,6 +4665,8 @@ test_live_captain_held_first_sight_silenced_by_away_record() { test_backlog_hold_never_rechecked_while_away_record_exists() { local dir out capture wakes + command -v tasks-axi >/dev/null 2>&1 \ + || { echo "skip: tasks-axi not found (away-record backlog hold)"; return 0; } dir=$(make_hold_home away-record-backlog-hold 'done: PR https://example.test/pr/9 checks green' hold) \ || fail "could not build the backlog-hold fixture" out="$dir/watch.out"; capture="$dir/pane.txt" From fb19dd9a75f2a9ec0b4c6573e45a0172b4f20f68 Mon Sep 17 00:00:00 2001 From: Jon Roosevelt Date: Sat, 12 Sep 2026 14:21:09 -0400 Subject: [PATCH 006/254] fix(backlog): bound per-item backlog row reads so a wedged backend cannot blind a session start (#4027) * fix(bin): bound each backlog row read so one wedged backend cannot blind a session start bin/fm-bootstrap.sh's reconcile and close-replay sweeps read the backlog backend once per item through fm_backlog_row_show, and that read was unbounded. A single wedged `tasks-axi show` therefore consumed the whole FM_SESSION_START_TIMEOUT and truncated the digest before the wake queue, supervision instructions, fleet state, and context sections ever printed, leaving the fleet unsupervised with no live watcher. The harm was a blind startup, not a slow one. Bound the read with the existing shared timeout primitive (bin/fm-timeout-lib.sh), so a wedged backend degrades to a loud partial reconcile: the sweep's existing BACKLOG_RECONCILE diagnostic names the item it could not read and the loop continues to the next one. The first bound hit also latches FM_BACKLOG_ROW_SHOW_WEDGED, so a sweep over many items pays one bound rather than one per item and still names every item it skipped, which is what keeps the digest whole on a home carrying a large fleet. The bound holds regardless of any particular tasks-axi install, so it does not depend on the 0.2.5 `show` hang being resolved separately. * fix(bin): set the wedged-backend latch where it survives, and prove it The latch added with the read bound was inert. fm_backlog_row_show runs inside a command substitution in both of its status-capturing callers, so the subshell read the inherited value correctly but its write died with the subshell. Every item still paid a full bound and reported `exceeded`, never `skipped`, which left the large-fleet case the latch existed to cover completely uncovered. Move the write to the two callers that capture the read's status and own the surviving shell, and leave fm_backlog_row_show reading the latch only. Correct the comments that claimed an ownership the function never had. The test that was supposed to cover this asserted only that the second read finished under a generous ceiling, which is true whether or not the latch works. Assert instead that a latched read is strictly faster than one bound and that it reports its own item as skipped, so an inert latch fails the test. * test: cover every item the wedged-backend latch skips The latch assertion exercised a single skipped item, so "every skipped item is still named" was inferred rather than tested. Probe three items instead and assert each skipped one names itself and costs less than a bound. Verified as a real guard by removing both latch writes: the suite then fails on the first skipped item instead of passing. * no-mistakes(review): distinguish backlog read-bound hits from absent rows * no-mistakes(review): preserve read-bound status through the captain verify gates * no-mistakes(review): Preserve backlog read-bound hits through resolve_entry and reconcile instead of spending them as absent rows * no-mistakes(review): Preserve backlog read-bound 124 through migrated-prefix scan and remaining task_show call sites * no-mistakes(document): Document bounded backlog row reads and FM_BACKLOG_ROW_TIMEOUT_SECS * no-mistakes(ci): Fixed all four failing CI checks with one root-cause fix plus one test-heredity fix. (1) bin/fm-captain-hold.sh: task_show carries the row in TASK_SHOW_OUTPUT and emits no stdout, but four call sites still used the stale command-substitution convention show=$(task_show ...), leaving show empty: task_show_or_fail (every captain hold failed with 'did not retain its hold-set stamp' - broke fm-captain-hold-lifecycle in parallel 1 and fm-bearings-board in serial 3), resolve_migrated_entry (migrated-prefix resolution could never match), reconcile-requests (existing rows were refused as absent), and command_open --identity (printed a constant '#0' identity, so fm-watch-triage's re-held captain call inherited the previous call's silence in serial 1). This is also the Greptile P1. Fixed by invoking task_show in the current shell and reading show=$TASK_SHOW_OUTPUT, the convention the other eight call sites already use; read-bound hits still stop loudly by name. (2) tests/fm-backlog-read-bound.test.sh (serial 4, unclassified family): the new e2e half implicitly relied on the author's process tree containing a harness process so fm-lock.sh would grant the fleet lock; on CI runners the lock is refused, the reconcile sweep is skipped, and the final BACKLOG_RECONCILE assertion fails. Reproduced by simulating a CI ancestry via a ps shim, fixed by pinning the lock evidence with the established fake-ps harness fixture pattern from tests/fm-session-start.test.sh. Verified: shellcheck clean; parallel-1, serial-3, and serial-4 lanes fully green locally (failed=0); serial-1 lane green except fm-gemini-harness, which fails only under local Node v26 (comm=node-MainThread); CI's default Node 22 reports comm=node, the branch that test passes on, so it is not a CI failure * no-mistakes(document): Verified bounded backlog read docs accurate across branch --- bin/fm-backlog-transition-lib.sh | 65 ++++- bin/fm-captain-hold.sh | 135 ++++++--- docs/configuration.md | 1 + docs/sessionstart-nudge.md | 3 +- tests/fm-backlog-read-bound.test.sh | 430 ++++++++++++++++++++++++++++ 5 files changed, 594 insertions(+), 40 deletions(-) create mode 100755 tests/fm-backlog-read-bound.test.sh diff --git a/bin/fm-backlog-transition-lib.sh b/bin/fm-backlog-transition-lib.sh index 4442ec3c64f..c116d016b0e 100644 --- a/bin/fm-backlog-transition-lib.sh +++ b/bin/fm-backlog-transition-lib.sh @@ -73,6 +73,17 @@ FM_BACKLOG_ROW_HOLD_KIND= # shellcheck disable=SC2034 # Output global, read by the sourcing caller. FM_BACKLOG_CLOSE_REPLAY_RESULT= +# Bounded execution is fm-timeout-lib.sh's alone; source it rather than +# re-deriving a deadline here. It is stateless, so the memoisation reason this +# library does not source fm-tasks-axi-lib.sh does not apply. +# shellcheck source=bin/fm-timeout-lib.sh disable=SC1091 +. "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/fm-timeout-lib.sh" + +# Latched when a row read hits its bound. fm_backlog_row_show runs inside a +# command substitution, so the subshell can READ this latch but cannot set it; +# the callers that capture its status own the write. +FM_BACKLOG_ROW_SHOW_WEDGED=0 + # Emit each byte of a value as a decimal number, locale-independently. # Deliberately perl rather than od: the spawn and teardown lifecycle runs under a # curated PATH (tests/fm-teardown.test.sh make_path_without_lsof pins that set) @@ -382,22 +393,64 @@ fm_tasks_axi() { exit 127 } -# Print one row's `tasks-axi show` output (plus stderr); the exit status is -# tasks-axi's. Extra flags (such as --full) are passed through. +# Print one row's `tasks-axi show` output (plus stderr) from the addressing +# fm_backlog_tasks_axi_addressing resolved, with `--file` only for the markdown +# backend. Addressing or backend-resolution errors return before tasks-axi runs; +# otherwise its exit status is preserved. Extra flags (--full) pass through. +# +# Every read is bounded, because a wedged backend read here is what blinds a +# whole session start: bin/fm-bootstrap.sh's reconcile and close-replay sweeps +# call this once per item, and one unbounded read consumes the entire +# FM_SESSION_START_TIMEOUT and truncates the digest before the wake queue, +# supervision instructions, fleet state and context sections ever print. The +# bound turns that into a loud partial reconcile: the caller reports the item it +# could not read and moves to the next one. +# +# A per-item bound alone is not enough on a home carrying a large fleet, because +# N wedged items still cost N bounds and the digest is truncated anyway. So the +# first bound hit latches FM_BACKLOG_ROW_SHOW_WEDGED and every later read in the +# same sweep returns immediately, still naming its own item so nothing is +# silently skipped. This function only READS that latch: it runs inside a +# command substitution, and a write here would die with the subshell, so the +# callers that capture its status set it. The latch is deliberately +# process-wide because these scripts are short-lived and a backend that wedged +# once will wedge again within the same run. fm_backlog_row_show() { # [flag...] - local data=$1 id=$2 addressing_status + local data=$1 id=$2 out status addressing_status secs=${FM_BACKLOG_ROW_TIMEOUT_SECS:-10} shift 2 + # A non-positive bound is not a bound (fm-timeout-lib.sh), and a padded zero + # such as 00 is still zero, so the digits test alone would let the very read + # this bound exists to prevent back in. Compare arithmetically, tolerating a + # value too large for the shell to compare at all. + case "$secs" in ''|*[!0-9]*) secs=10 ;; esac + [ "$secs" -gt 0 ] 2>/dev/null || secs=10 fm_backlog_tasks_axi_addressing "$data" addressing_status=$? if [ "$addressing_status" -ne 0 ]; then [ -z "${FM_BACKLOG_TRANSITION_ERROR:-}" ] || printf '%s\n' "$FM_BACKLOG_TRANSITION_ERROR" >&2 return "$addressing_status" fi + if [ "$FM_BACKLOG_ROW_SHOW_WEDGED" = 1 ]; then + printf 'tasks-axi show %s skipped: the backlog backend already exceeded its %ss read bound\n' "$id" "$secs" + return 124 + fi if [ -n "$FM_BACKLOG_AXI_FILE" ]; then - (cd "$FM_BACKLOG_AXI_ROOT" 2>/dev/null && fm_tasks_axi show "$id" "$@" --file "$FM_BACKLOG_AXI_FILE" 2>&1) + set -- "$@" --file "$FM_BACKLOG_AXI_FILE" + fi + # shellcheck disable=SC2016 # Expansion is deliberately deferred to the child shell. + out=$(fm_run_timed "$secs" bash -c 'cd "$1" 2>/dev/null || exit 1; shift; exec tasks-axi show "$@"' \ + _ "$FM_BACKLOG_AXI_ROOT" "$id" "$@" 2>&1) + status=$? + # A backend that wrote a header or a progress line before wedging leaves that + # fragment as the first output line, and every caller reads the first line as + # the failure reason. Whatever a timed-out read managed to emit is incomplete + # by definition, so the bound speaks for it instead. + if [ "$status" -eq 124 ]; then + printf 'tasks-axi show %s exceeded its %ss backlog read bound\n' "$id" "$secs" else - (cd "$FM_BACKLOG_AXI_ROOT" 2>/dev/null && fm_tasks_axi show "$id" "$@" 2>&1) + printf '%s\n' "$out" fi + return "$status" } fm_backlog_row_list() { # [flag...] @@ -436,6 +489,7 @@ fm_backlog_row_probe() { # fi out=$(fm_backlog_row_show "$data" "$id") command_status=$? + [ "$command_status" -ne 124 ] || FM_BACKLOG_ROW_SHOW_WEDGED=1 if [ "$command_status" -ne 0 ]; then if printf '%s\n' "$out" | grep -q '^code: NOT_FOUND$'; then FM_BACKLOG_ROW_RESULT=not_found @@ -559,6 +613,7 @@ fm_backlog_retain() { # [flag...] if [ -n "$deliverable" ]; then out=$(fm_backlog_row_show "$data" "$id" --full) command_status=$? + [ "$command_status" -ne 124 ] || FM_BACKLOG_ROW_SHOW_WEDGED=1 if [ "$command_status" -ne 0 ]; then FM_BACKLOG_TRANSITION_ERROR=$(printf '%s\n' "$out" | sed -n '1p') [ -n "$FM_BACKLOG_TRANSITION_ERROR" ] \ diff --git a/bin/fm-captain-hold.sh b/bin/fm-captain-hold.sh index a9ef07b132a..c3d3a98f67e 100755 --- a/bin/fm-captain-hold.sh +++ b/bin/fm-captain-hold.sh @@ -350,10 +350,38 @@ require_tasks_axi() { || fail "tasks-axi does not expose the captain-hold contract" } -task_show() { # - local data +# Read one row into TASK_SHOW_OUTPUT; a non-zero return means the row is +# absent. A read that could not finish inside its bound is NOT absence, and +# every caller below would otherwise spend it as one - minting a duplicate task, +# skipping a keyed answer, or reporting a task that exists as missing. So the +# bound's own status stops the command instead, loudly and by name, and it +# leaves 124 intact rather than collapsing to fail's 1 so a caller running this +# inside a command substitution can still tell a wedged backend from a +# genuinely unknown id. +TASK_SHOW_OUTPUT= +task_show() { # ; sets TASK_SHOW_OUTPUT + local data status=0 reason data=$(fm_backlog_data_absolute "$DATA") || fail "data directory cannot be resolved: $DATA" - fm_backlog_row_show "$data" "$1" --full 2>/dev/null + TASK_SHOW_OUTPUT=$(fm_backlog_row_show "$data" "$1" --full 2>/dev/null) || status=$? + if [ "$status" -eq 124 ]; then + reason=${TASK_SHOW_OUTPUT%%$'\n'*} + printf 'fm-captain-hold: %s\n' \ + "${reason:-tasks-axi show $1 exceeded its backlog read bound}" >&2 + exit 124 + fi + return "$status" +} + +# Read one row into `show`, failing with only when the read +# genuinely failed; a read-bound hit (124) stops the command by name instead. +# task_show must be called in THIS shell, not inside a command substitution: +# it carries the row in TASK_SHOW_OUTPUT, which a subshell cannot hand back. +task_show_or_fail() { # ; sets show + task_show "$1" || { + [ "$?" -ne 124 ] || fail "the backlog backend exceeded its read bound reading $1" + fail "$2" + } + show=$TASK_SHOW_OUTPUT } show_field() { # @@ -388,7 +416,7 @@ show_field_value() { # origin_exists_here() { # [ -f "$STATE/$1.meta" ] && return 0 [ -f "$DATA/$1/report.md" ] && return 0 - task_show "$1" >/dev/null 2>&1 + task_show "$1" } list_has_key() { # @@ -493,7 +521,8 @@ resolution_block() { # # surviving even when a date gate has expired) or a recorded captain answer. verify_hold_durable() { # local id=$1 show state hold_kind body - show=$(task_show "$id") || fail "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" + task_show "$id" || fail "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" + show=$TASK_SHOW_OUTPUT state=$(show_field "$show" state) hold_kind=$(show_field_value "$show" hold_kind) body=$(show_field "$show" body) @@ -678,7 +707,13 @@ resolve_migrated_entry() { # *-) prefixed="$prefix$candidate" ;; *) prefixed="$prefix-$candidate" ;; esac - show=$(task_show "$prefixed" 2>/dev/null) || continue + # Same shell rule as task_show_or_fail: the row is read out of + # TASK_SHOW_OUTPUT, so the read cannot sit inside a command substitution. + task_show "$prefixed" 2>/dev/null || { + [ "$?" -ne 124 ] || return 124 + continue + } + show=$TASK_SHOW_OUTPUT [ "$(show_field_value "$show" hold_kind)" = captain ] || continue prefixed_matches="${prefixed_matches}${prefixed_matches:+$NL_SEP}$prefixed" done @@ -699,13 +734,13 @@ resolve_migrated_entry() { # # migrated-prefix, so a caller can record which evidence carried the attestation. resolve_entry() { # ; prints " " or fails local origin=$1 entry=$2 legacy migrated rc - if task_show "$entry" >/dev/null 2>&1; then + if task_show "$entry"; then printf '%s exact' "$entry" return 0 fi if [ -n "$origin" ] && [ "$origin" != "$BINDING_ANY" ]; then legacy=$(legacy_hold_id "$origin" "$entry") - if task_show "$legacy" >/dev/null 2>&1; then + if task_show "$legacy"; then printf '%s legacy' "$legacy" return 0 fi @@ -715,6 +750,7 @@ resolve_entry() { # ; prints " " or fails case "$rc" in 0) printf '%s' "$migrated"; return 0 ;; 2) return 2 ;; + 124) return 124 ;; esac if [ -n "$origin" ] && [ "$origin" != "$BINDING_ANY" ]; then legacy=$(legacy_hold_id "$origin" "$entry") @@ -763,6 +799,24 @@ write_hold_set_stamp() { # " so the caller can keep the +# attestation evidence. +verify_entry_durable() { # ; prints " " + local origin=$1 entry=$2 resolved resolve_status=0 + resolved=$(resolve_entry "$origin" "$entry") || resolve_status=$? + if [ "$resolve_status" -ne 0 ]; then + [ "$resolve_status" -ne 124 ] \ + || fail "the backlog backend exceeded its read bound resolving $entry" + exit "$resolve_status" + fi + printf '%s\n' "$resolved" + verify_hold_durable "${resolved%% *}" +} + command_hold() { local id=${1:-} title='' reason='' repo='' origin='' until='' show state existing_title body='' hold_kind hold_set occurrence local existing_hold_kind='' existing_held='' preserve_hold_set=0 @@ -798,7 +852,8 @@ command_hold() { esac acquire_task_control_lock "$id" require_tasks_axi - if show=$(task_show "$id"); then + if task_show "$id"; then + show=$TASK_SHOW_OUTPUT state=$(show_field "$show" state) [ "$state" != "done" ] \ || fail "task $id is already closed; a new captain call needs its own task" @@ -833,9 +888,9 @@ command_hold() { # Publish the timestamp before the captain-hold annotation. A concurrent # snapshot may see the harmless stamp by itself, but can never see a newly # held task without the timestamp that defines this hold lifecycle's age. - show=$(task_show "$id") || fail "task $id disappeared before recording its hold-set stamp" + task_show_or_fail "$id" "task $id disappeared before recording its hold-set stamp" write_hold_set_stamp "$id" "$(show_field "$show" body)" "$hold_set" "$preserve_hold_set" - show=$(task_show "$id") || fail "task $id disappeared while recording its hold-set stamp" + task_show_or_fail "$id" "task $id disappeared while recording its hold-set stamp" [ -n "$(body_hold_set_timestamp "$(show_field_value "$show" body)")" ] \ || fail "task $id did not retain its hold-set stamp" if [ -n "$until" ]; then @@ -845,7 +900,8 @@ command_hold() { tasks_axi hold "$id" --reason "$reason" --kind captain >/dev/null \ || fail "could not hold task $id for the captain" fi - show=$(task_show "$id") || fail "task $id disappeared while holding it" + task_show "$id" || fail "task $id disappeared while holding it" + show=$TASK_SHOW_OUTPUT hold_kind=$(show_field_value "$show" hold_kind) [ "$hold_kind" = captain ] || fail "task $id did not retain its captain hold" occurrence=$(( $(resolution_record_count "$(show_field "$show" body)") + 1 )) @@ -922,7 +978,7 @@ close_answered() { # remove_interrupted_answer_stamp() { # local id=$1 show body existing tmp - show=$(task_show "$id") || fail "task $id disappeared after closing" + task_show_or_fail "$id" "task $id disappeared after closing" body=$(decode_shown_value "$(show_field "$show" body)") \ || fail "could not decode the closed body for $id" existing=$(body_hold_set_timestamp "$body") @@ -958,7 +1014,8 @@ command_answer() { load_decision "$decision_file" acquire_task_control_lock "$id" require_tasks_axi - show=$(task_show "$id") || fail "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" + task_show "$id" || fail "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" + show=$TASK_SHOW_OUTPUT state=$(show_field "$show" state) hold_kind=$(show_field_value "$show" hold_kind) body=$(show_field "$show" body) @@ -994,7 +1051,8 @@ command_answer() { || fail "task $id was never held for the captain; nothing to record an answer on" write_resolution_record "$id" repaired "$body" remove_interrupted_answer_stamp "$id" - show=$(task_show "$id") || fail "task $id disappeared while recording the answer" + task_show "$id" || fail "task $id disappeared while recording the answer" + show=$TASK_SHOW_OUTPUT [ "$(show_field "$show" state)" = "done" ] || fail "recording the answer reopened closed task $id" body_has_resolution_record "$(show_field "$show" body)" \ || fail "captain-held task $id did not retain its durable resolution record" @@ -1031,7 +1089,8 @@ command_answer() { fail "could not close answered captain-held task $id" fi remove_interrupted_answer_stamp "$id" - show=$(task_show "$id") || fail "task $id disappeared after closing" + task_show "$id" || fail "task $id disappeared after closing" + show=$TASK_SHOW_OUTPUT body_has_resolution_record "$(show_field "$show" body)" \ || fail "captain-held task $id did not retain its durable resolution record" publish_parent_resolution_then_retire "$id" "$occurrence" "$outcome" @@ -1152,6 +1211,7 @@ sanitize_reconcile_provenance() { command_answers() { local origin='' source='' row rest key answer label mode id show state hold_kind body digest legacy_digest legacy_key local recorded_digest recorded_mode occurrence tmp err closed=0 skipped=0 reason release_flag tab=$'\t' + local resolve_rc while [ "$#" -gt 0 ]; do case "$1" in --source) shift; source=${1:-} ;; @@ -1212,6 +1272,12 @@ command_answers() { continue fi if [ "$resolve_rc" -ne 0 ]; then + # resolve_entry runs in a command substitution, so task_show's exit + # cannot stop this loop; only its status crosses back. 124 means the + # backend never answered, which is not the same as an unknown key and + # must not be spent as a skip. + [ "$resolve_rc" -ne 124 ] \ + || fail "the backlog backend exceeded its read bound resolving $key" printf 'skipped: %s (no captain-held task with that id)\n' "$key" skipped=$((skipped + 1)) continue @@ -1231,7 +1297,8 @@ command_answers() { if [ -n "$legacy_key" ]; then legacy_digest=$(sha256_text "$(legacy_keyed_decision_text "$source" "$legacy_key" "$answer" "$label")") fi - show=$(task_show "$id") || { printf 'skipped: %s (absent)\n' "$id"; skipped=$((skipped + 1)); continue; } + task_show "$id" || { printf 'skipped: %s (absent)\n' "$id"; skipped=$((skipped + 1)); continue; } + show=$TASK_SHOW_OUTPUT state=$(show_field "$show" state) hold_kind=$(show_field_value "$show" hold_kind) body=$(show_field "$show" body) @@ -1347,7 +1414,7 @@ publish_parent_resolution_then_retire() { # } command_reconcile_requests() { - local source_id='' source='' origin row id note provenance show created=0 skipped=0 tab=$'\t' + local source_id='' source='' origin row id note provenance show show_status=0 created=0 skipped=0 tab=$'\t' while [ "$#" -gt 0 ]; do case "$1" in --source-id) shift; source_id=${1:-} ;; @@ -1372,7 +1439,13 @@ command_reconcile_requests() { [ "${#id}" -le 128 ] \ || { printf 'refused: %s (task id is too long)\n' "$id"; skipped=$((skipped + 1)); continue; } acquire_task_control_lock "$id" - show=$(task_show "$id") || true + show_status=0 + show='' + task_show "$id" || show_status=$? + [ "$show_status" -ne 0 ] || show=$TASK_SHOW_OUTPUT + if [ "$show_status" -eq 124 ]; then + fail "the backlog backend exceeded its read bound reading $id" + fi if [ -z "$show" ]; then printf 'refused: %s (absent)\n' "$id" skipped=$((skipped + 1)) @@ -1447,7 +1520,7 @@ reconcile_close() { reconcile_request_read "$id" \ || fail "task $id has no pending board-created reconcile request" require_tasks_axi - show=$(task_show "$id") || fail "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" + task_show_or_fail "$id" "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" state=$(show_field "$show" state) hold_kind=$(show_field_value "$show" hold_kind) body=$(show_field "$show" body) @@ -1483,7 +1556,7 @@ reconcile_close() { fi close_answered "$id" 0 || fail "could not close reconciled captain-held task $id" remove_interrupted_answer_stamp "$id" - show=$(task_show "$id") || fail "task $id disappeared after closing" + task_show_or_fail "$id" "task $id disappeared after closing" body_has_resolution_record "$(show_field "$show" body)" \ || fail "captain-held task $id did not retain its durable resolution record" publish_parent_hold "$id" "$occurrence" resolved reconciled @@ -1519,7 +1592,7 @@ reconcile_note() { require_tasks_axi command_open "$id" \ || fail "task $id is not an open captain call; a note cannot keep a closed call open" - show=$(task_show "$id") || fail "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" + task_show_or_fail "$id" "captain-held task $id is absent from this home's configured backlog (data directory $DATA)" body=$(decode_shown_value "$(show_field "$show" body)") \ || fail "could not decode the existing body for $id" note_digest=$(sha256_text "$note") @@ -1584,13 +1657,9 @@ command_complete() { if [ -n "$keys" ]; then while IFS= read -r entry; do [ -n "$entry" ] || continue - if ! resolved=$(resolve_entry "$origin" "$entry"); then - # resolve_entry has already refused on stderr naming the entry. - exit 1 - fi + resolved=$(verify_entry_durable "$origin" "$entry") || exit $? resolved_how=${resolved##* } resolved=${resolved%% *} - verify_hold_durable "$resolved" if [ "$resolved_how" = migrated-prefix ]; then attested_by_prefix="${attested_by_prefix}${attested_by_prefix:+ }$entry=$resolved" fi @@ -1648,11 +1717,7 @@ command_verify() { if [ -n "$keys" ]; then while IFS= read -r entry; do [ -n "$entry" ] || continue - if ! resolved=$(resolve_entry "$origin" "$entry"); then - # resolve_entry has already refused on stderr naming the entry. - exit 1 - fi - verify_hold_durable "${resolved%% *}" + verify_entry_durable "$origin" "$entry" >/dev/null done < [--identity] [--distinguish-absent] state=${FM_BACKLOG_ROW_STATE%% *} if [ "$state" != "done" ] && [ "$FM_BACKLOG_ROW_HOLD_KIND" = captain ]; then if [ "$identity" -eq 1 ]; then - show=$(task_show "$id") || { + task_show "$id" || { printf 'fm-captain-hold: captain call %s is open but its record could not be read\n' "$id" >&2 exit 2 } + show=$TASK_SHOW_OUTPUT shown_body=$(show_field "$show" body) printf '%s#%s\n' \ "$(body_hold_set_timestamp "$(decode_shown_value "$shown_body")")" \ diff --git a/docs/configuration.md b/docs/configuration.md index 3a9ef6e0b43..b6ad202c6fa 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -976,6 +976,7 @@ FM_ZELLIJ_SESSION=firstmate # zellij-only: named session for normal backend ops CMUX_SOCKET_PASSWORD= # cmux-only: socket password fallback when config/cmux-socket-password is absent (docs/cmux-backend.md) FM_SESSION_START_STATUS_TAIL=5 # state/*.status lines printed per task in the session-start digest; each line is capped by bin/fm-line-cap-lib.sh FM_SESSION_START_QUEUED_LIMIT=20 # plain queued backlog rows in the session-start digest; in-flight, held, and blocked rows are never bounded and done rows are never listed +FM_BACKLOG_ROW_TIMEOUT_SECS=10 # seconds bounding each backlog row read (bin/fm-backlog-transition-lib.sh); nonpositive or invalid values fall back to 10; the first bound hit latches the sweep so later reads return immediately, each still naming its own item FM_BOOTSTRAP_DETECT_ONLY=0 # internal/read-only session-start mode: skip bootstrap's mutating sweeps and print advisory TANGLE wording FM_BOOTSTRAP_NETWORK=all # internal session-start phase split: all, skip (local steps only), or only (network steps only); see bin/fm-bootstrap.sh FM_STARTUP_NETWORK_TIMEOUT=120 # seconds bounding the deferred inactive-outcome scan plus network checks; hitting it prints an actionable NETWORK_CHECKS line diff --git a/docs/sessionstart-nudge.md b/docs/sessionstart-nudge.md index 68f9b9c4ecb..17d44c93c44 100644 --- a/docs/sessionstart-nudge.md +++ b/docs/sessionstart-nudge.md @@ -42,7 +42,8 @@ On a run-tier harness the nudge cannot also fire: `resume`, `reload`, and `fork` The run tier blocks either hook-driven session initialization or Pi's first provider preflight while the digest runs, so `bin/fm-session-start.sh` bounds itself rather than betting on an unbounded prerequisite. The digest makes no external-network call at all: every one it owes runs off the blocking path in the separately bounded deferred stage owned by `bin/fm-startup-network.sh`, so an unreachable host can no longer consume this budget. -What remains is still not individually bounded - tool version probes, the backlog listing, and the per-task endpoint reads are all local but unbounded subprocesses - so the whole digest runs as one bounded child, default 120s via `FM_SESSION_START_TIMEOUT`. +Tool version probes, the backlog listing, and the per-task endpoint reads remain local but unbounded subprocesses, so the whole digest still runs as one bounded child, default 120s via `FM_SESSION_START_TIMEOUT`. +The per-item backlog row reads inside bootstrap's reconcile and close-replay sweeps are the exception: each is bounded by `FM_BACKLOG_ROW_TIMEOUT_SECS` (default 10s) through `bin/fm-backlog-transition-lib.sh`, and the first bound hit latches the sweep so later reads return immediately while still naming their own item. The shared timeout owner falls back to a pure-Bash process-group watchdog when timeout, gtimeout, and perl are unavailable, so no supported host runs the digest unbounded. Because the child streams into the native transport as it runs, everything emitted before the bound was hit is retained for delivery; the parent then prints a `STARTUP TRUNCATED` banner naming the stage that did not finish and the stages that were therefore never emitted, and still exits 0. The registered hook timeouts sit above that budget so the harness never preempts the banner. diff --git a/tests/fm-backlog-read-bound.test.sh b/tests/fm-backlog-read-bound.test.sh new file mode 100755 index 00000000000..726ca052455 --- /dev/null +++ b/tests/fm-backlog-read-bound.test.sh @@ -0,0 +1,430 @@ +#!/usr/bin/env bash +# tests/fm-backlog-read-bound.test.sh - behavior tests for the per-item bound on +# bin/fm-backlog-transition-lib.sh's backlog row read. +# +# The defect this pins: bin/fm-bootstrap.sh's reconcile and close-replay sweeps +# read the backlog backend once per item, and an unbounded read of a wedged +# backend consumed the whole FM_SESSION_START_TIMEOUT. The digest was then +# truncated before the wake queue, supervision instructions, fleet state, and +# context sections printed, leaving a whole fleet unsupervised. +# +# Both halves are proved here: +# - a deliberately hanging `tasks-axi show` cannot exceed the per-item bound, +# and the failure names the item it could not read +# - a session start against that same wedged backend still completes end to +# end, with every digest section present and a loud partial reconcile +# +# The bound must hold on its own, independent of any particular tasks-axi +# install, so the fake here simply never returns. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +BASE_PATH=${FM_TEST_BASE_PATH:-/usr/bin:/bin:/usr/sbin:/sbin} +TMP_ROOT=$(fm_test_tmproot fm-backlog-read-bound-tests) +trap fm_test_cleanup EXIT + +BOUND_SECS=2 +# Generous enough that a slow CI box never flakes, far below the unbounded hang +# (300s per read) and below the session-start budget the defect consumed. +BOUND_CEILING=30 + +# A backend whose `show` never returns. Everything the compatibility gate and the +# startup listing need still answers promptly, so the only thing under test is +# the read that hangs. +make_hanging_tasks_axi() { # + local fakebin=$1 + cat > "$fakebin/tasks-axi" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + --version) printf '%s\n' '0.2.5'; exit 0 ;; + update) + [ "${2:-}" = --help ] || exit 0 + printf '%s\n' 'usage: tasks-axi update [flags]' ' --body-file ' ' --archive-body' + exit 0 + ;; + mv) + [ "${2:-}" = --help ] || exit 0 + printf '%s\n' 'usage: tasks-axi mv [...] --to ' + exit 0 + ;; + show) + # A real backend rejects an unusable id promptly instead of wedging, which + # is what makes a dropped 124 surface as "absent" rather than as a bound. + if [ -z "${2:-}" ]; then + printf 'code: NOT_FOUND\n' >&2 + exit 1 + fi + # The wedge under test: a read that never returns. + sleep 300 + exit 0 + ;; + hold) + [ "${2:-}" = --help ] || exit 0 + printf '%s\n' 'usage: tasks-axi hold [flags]' ' --kind captain' ' --until ' + exit 0 + ;; + add) + # Recorded, never silent: creating a row that already exists is the damage a + # timed-out read must never be spent on. + [ -z "${FM_TEST_TASKS_AXI_ADD_LOG:-}" ] || printf '%s\n' "$*" >> "$FM_TEST_TASKS_AXI_ADD_LOG" + exit 0 + ;; + list) + printf 'count: 0\n' + printf 'tasks[0]{id,state,kind,repo,title,blocked_by,hold_kind,hold_reason}:\n' + exit 0 + ;; +esac +exit 0 +SH + chmod +x "$fakebin/tasks-axi" +} + +elapsed_since() { # + local now + now=$(date +%s) + printf '%s\n' "$((now - $1))" +} + +# --- half one: the per-item bound holds ------------------------------------- + +UNIT="$TMP_ROOT/unit" +UNIT_FAKEBIN=$(fm_fakebin "$UNIT") +mkdir -p "$UNIT/data" +make_hanging_tasks_axi "$UNIT_FAKEBIN" +printf '# Backlog\n' > "$UNIT/data/backlog.md" + +# Three items, so "every skipped item is still named" is actually exercised +# rather than inferred from a single skip. +PROBE_OUT="$UNIT/probe.out" +PATH="$UNIT_FAKEBIN:$BASE_PATH" FM_BACKLOG_ROW_TIMEOUT_SECS="$BOUND_SECS" \ + bash -c ' + set -u + . "$1/bin/fm-tasks-axi-lib.sh" + . "$1/bin/fm-backlog-transition-lib.sh" + for id in wedged-one wedged-two wedged-three; do + start=$(date +%s) + fm_backlog_row_probe "$2" "$id" && printf "unexpected-success\n" + printf "elapsed:%s=%s\n" "$id" "$(( $(date +%s) - start ))" + printf "error:%s=%s\n" "$id" "$FM_BACKLOG_ROW_ERROR" + done + ' _ "$ROOT" "$UNIT/data" > "$PROBE_OUT" 2>&1 + +probe_elapsed() { # + sed -n "s/^elapsed:$1=//p" "$PROBE_OUT" +} + +probe_error() { # + sed -n "s/^error:$1=//p" "$PROBE_OUT" +} + +grep -q '^unexpected-success$' "$PROBE_OUT" \ + && fail "a hanging tasks-axi show must not report a successful row read: $(cat "$PROBE_OUT")" + +FIRST_ELAPSED=$(probe_elapsed wedged-one) +[ -n "$FIRST_ELAPSED" ] || fail "probe produced no timing: $(cat "$PROBE_OUT")" +[ "$FIRST_ELAPSED" -lt "$BOUND_CEILING" ] \ + || fail "bounded row read took ${FIRST_ELAPSED}s, over the ${BOUND_CEILING}s ceiling: $(cat "$PROBE_OUT")" +pass "a hanging tasks-axi show returns within the per-item bound instead of running unbounded" + +FIRST_ERROR=$(probe_error wedged-one) +case "$FIRST_ERROR" in + *wedged-one*bound*) ;; + *) fail "the timed-out read must name the item and its bound, got: $FIRST_ERROR" ;; +esac +pass "a timed-out row read reports one error naming the item that timed out" + +# The latch is what keeps a home carrying a large fleet from paying N bounds and +# losing the digest anyway, so assert it strictly: a latched read must be +# FASTER than one bound, not merely under the ceiling. A ceiling-only assertion +# passes whether or not the latch works, and fm_backlog_row_show runs inside a +# command substitution whose writes die with the subshell - the exact way this +# latch can silently become inert. +for SKIPPED in wedged-two wedged-three; do + SKIPPED_ERROR=$(probe_error "$SKIPPED") + SKIPPED_ELAPSED=$(probe_elapsed "$SKIPPED") + case "$SKIPPED_ERROR" in + *"$SKIPPED"*skipped*) ;; + *) fail "every skipped item must still be named as skipped, $SKIPPED got: $SKIPPED_ERROR" ;; + esac + [ -n "$SKIPPED_ELAPSED" ] && [ "$SKIPPED_ELAPSED" -lt "$BOUND_SECS" ] \ + || fail "the latch is inert: $SKIPPED paid ${SKIPPED_ELAPSED}s against a known-wedged backend" +done +pass "after the first bound hit the sweep continues and names every remaining item without paying the bound again" + +# A padded zero is still zero, and `timeout 0` / `alarm 0` disable the deadline +# outright, so a bound that only rejects the literal 0 silently restores the +# unbounded read this whole change exists to prevent. +PADDED_OUT="$UNIT/padded.out" +PADDED_START=$(date +%s) +PATH="$UNIT_FAKEBIN:$BASE_PATH" FM_BACKLOG_ROW_TIMEOUT_SECS=00 \ + bash -c ' + set -u + . "$1/bin/fm-tasks-axi-lib.sh" + . "$1/bin/fm-backlog-transition-lib.sh" + fm_backlog_row_probe "$2" padded-zero && printf "unexpected-success\n" + printf "error=%s\n" "$FM_BACKLOG_ROW_ERROR" + ' _ "$ROOT" "$UNIT/data" > "$PADDED_OUT" 2>&1 +PADDED_ELAPSED=$(elapsed_since "$PADDED_START") + +[ "$PADDED_ELAPSED" -lt "$BOUND_CEILING" ] \ + || fail "a padded-zero bound disabled the deadline: the read ran ${PADDED_ELAPSED}s" +case "$(sed -n 's/^error=//p' "$PADDED_OUT")" in + *padded-zero*bound*) ;; + *) fail "a padded-zero bound must fall back to the default bound and report it: $(cat "$PADDED_OUT")" ;; +esac +pass "a padded-zero bound falls back to the default instead of disabling the deadline" + +# --- a bound hit is not absence --------------------------------------------- +# +# Turning a hang into a fast 124 reaches every caller that reads a non-zero row +# status as "this row does not exist". bin/fm-captain-hold.sh's hold path is the +# one where that misreading corrupts: it would create a task that already +# exists. The bound must stop the command instead. + +CAPTAIN="$TMP_ROOT/captain" +CAPTAIN_FAKEBIN=$(fm_fakebin "$CAPTAIN") +mkdir -p "$CAPTAIN/data" "$CAPTAIN/state" "$CAPTAIN/config" +make_hanging_tasks_axi "$CAPTAIN_FAKEBIN" +cp "$ROOT/.tasks.toml" "$CAPTAIN/.tasks.toml" +printf '# Backlog\n' > "$CAPTAIN/data/backlog.md" + +ADD_LOG="$CAPTAIN/add.log" +HOLD_OUT="$CAPTAIN/hold.out" +HOLD_STATUS=0 +PATH="$CAPTAIN_FAKEBIN:$BASE_PATH" FM_HOME="$CAPTAIN" \ + FM_STATE_OVERRIDE="$CAPTAIN/state" FM_DATA_OVERRIDE="$CAPTAIN/data" \ + FM_CONFIG_OVERRIDE="$CAPTAIN/config" FM_BACKLOG_ROW_TIMEOUT_SECS="$BOUND_SECS" \ + FM_TEST_TASKS_AXI_ADD_LOG="$ADD_LOG" \ + "$ROOT/bin/fm-captain-hold.sh" hold wedged-hold --title 'Wedged hold' --reason 'backend wedged' \ + > "$HOLD_OUT" 2>&1 || HOLD_STATUS=$? + +[ "$HOLD_STATUS" -ne 0 ] \ + || fail "holding a task against a wedged backend must not report success: $(cat "$HOLD_OUT")" +[ ! -s "$ADD_LOG" ] \ + || fail "a timed-out read was spent as absence: tasks-axi add ran anyway: $(cat "$ADD_LOG")" +case "$(cat "$HOLD_OUT")" in + *wedged-hold*bound*) ;; + *) fail "the refusal must name the item and the bound it hit, got: $(cat "$HOLD_OUT")" ;; +esac +pass "a bound hit stops a captain hold loudly instead of being read as a missing task" + +# The teardown gate reaches a row read through the same resolver, so the bound +# hit has to survive the command substitution that carries the resolved id. +fm_write_meta "$CAPTAIN/state/wedged-origin.meta" \ + 'window=firstmate:fm-wedged-origin' \ + 'worktree=/nonexistent/wedged-origin' \ + 'project=alpha' \ + 'harness=claude' \ + 'decisions_reviewed=1' \ + 'decision_keys=wedged-entry' + +VERIFY_OUT="$CAPTAIN/verify.out" +VERIFY_STATUS=0 +PATH="$CAPTAIN_FAKEBIN:$BASE_PATH" FM_HOME="$CAPTAIN" \ + FM_STATE_OVERRIDE="$CAPTAIN/state" FM_DATA_OVERRIDE="$CAPTAIN/data" \ + FM_CONFIG_OVERRIDE="$CAPTAIN/config" FM_BACKLOG_ROW_TIMEOUT_SECS="$BOUND_SECS" \ + "$ROOT/bin/fm-captain-hold.sh" verify wedged-origin > "$VERIFY_OUT" 2>&1 || VERIFY_STATUS=$? + +[ "$VERIFY_STATUS" -ne 0 ] \ + || fail "verify must not attest an inventory it could not read: $(cat "$VERIFY_OUT")" +case "$(cat "$VERIFY_OUT")" in + *absent*) fail "a bound hit was reported as an absent task: $(cat "$VERIFY_OUT")" ;; +esac +case "$(cat "$VERIFY_OUT")" in + *wedged-entry*bound*) ;; + *) fail "verify must name the entry it could not read and the bound it hit, got: $(cat "$VERIFY_OUT")" ;; +esac +pass "the teardown verify gate reports a bound hit by name instead of as an absent inventory entry" + +# The reconcile-requests intake reads each row with task_show in this shell and +# must stop on a bound hit by name; spending the 124 as 'refused: +# (absent)' would let a wedged backend erase real rows from the reconcile +# sweep. +REQ="$TMP_ROOT/req" +REQ_FAKEBIN=$(fm_fakebin "$REQ") +mkdir -p "$REQ/data" "$REQ/state" "$REQ/config" "$REQ/state/decision-bindings" +make_hanging_tasks_axi "$REQ_FAKEBIN" +cp "$ROOT/.tasks.toml" "$REQ/.tasks.toml" +printf '# Backlog\n' > "$REQ/data/backlog.md" +printf 'schema=fm-decision-binding.v1\norigin=wedged-origin\n' \ + > "$REQ/state/decision-bindings/probe.origin" + +REQ_OUT="$REQ/req.out" +REQ_STATUS=0 +printf 'wedged-req\n' \ + | PATH="$REQ_FAKEBIN:$BASE_PATH" FM_HOME="$REQ" \ + FM_STATE_OVERRIDE="$REQ/state" FM_DATA_OVERRIDE="$REQ/data" \ + FM_CONFIG_OVERRIDE="$REQ/config" FM_BACKLOG_ROW_TIMEOUT_SECS="$BOUND_SECS" \ + "$ROOT/bin/fm-captain-hold.sh" reconcile-requests --source-id probe --source 'test capture' \ + > "$REQ_OUT" 2>&1 || REQ_STATUS=$? + +[ "$REQ_STATUS" -ne 0 ] \ + || fail "reconcile-requests must not report success against a wedged backend: $(cat "$REQ_OUT")" +case "$(cat "$REQ_OUT")" in + *absent*|*refused*) fail "the reconcile intake spent a bound hit as an absent row: $(cat "$REQ_OUT")" ;; +esac +case "$(cat "$REQ_OUT")" in + *wedged-req*bound*) ;; + *) fail "the reconcile intake must name the row and the bound it hit, got: $(cat "$REQ_OUT")" ;; +esac +pass "the reconcile-requests intake stops loudly on a bound hit instead of refusing the row as absent" + +# The migrated-prefix scan is the resolution path whose exact and legacy ids +# genuinely answer NOT_FOUND: only the prefixed migrated row wedges. A dropped +# 124 there falls through to 'no captain-held task $entry resolves to nothing' +# - the exact bound-hit-as-absence outcome the resolver's own 124 arm exists to +# prevent - so the bound must survive the prefixed scan to verify_entry_durable. +MIG="$TMP_ROOT/migrated" +MIG_FAKEBIN=$(fm_fakebin "$MIG") +mkdir -p "$MIG/data" "$MIG/state" "$MIG/config" +cat > "$MIG_FAKEBIN/tasks-axi" <<'SH' +#!/usr/bin/env bash +set -u +case "${1:-}" in + --version) printf '%s\n' '0.2.5'; exit 0 ;; + show) + [ -z "${2:-}" ] && { printf 'code: NOT_FOUND\n' >&2; exit 1; } + # Only the prefixed migrated candidates wedge; the exact and legacy ids + # answer NOT_FOUND promptly, the concrete path the prefix scan exists for. + case "$2" in $FM_TEST_PREFIXED_GLOB) sleep 300; exit 0 ;; esac + printf 'code: NOT_FOUND\n' >&2 + exit 1 + ;; + update) + [ "${2:-}" = --help ] || exit 0 + printf '%s\n' 'usage: tasks-axi update [flags]' ' --body-file ' ' --archive-body' + exit 0 + ;; + mv) + [ "${2:-}" = --help ] || exit 0 + printf '%s\n' 'usage: tasks-axi mv [...] --to ' + exit 0 + ;; + hold) + [ "${2:-}" = --help ] || exit 0 + printf '%s\n' 'usage: tasks-axi hold [flags]' ' --kind captain' ' --until ' + exit 0 + ;; + list) + printf 'count: 0\n' + printf 'tasks[0]{id,state,kind,repo,title,blocked_by,hold_kind,hold_reason}:\n' + exit 0 + ;; +esac +exit 0 +SH +chmod +x "$MIG_FAKEBIN/tasks-axi" +cat > "$MIG_FAKEBIN/bd" <<'SH' +#!/usr/bin/env bash +[ "${1:-}" = list ] && { printf '[]\n'; exit 0; } +exit 1 +SH +chmod +x "$MIG_FAKEBIN/bd" +cat > "$MIG/.tasks.toml" <<'TOML' +backend = "beads" + +[beads] +prefix = "bd" +path = "graph" +binary = "bd" +TOML +printf '# Backlog\n' > "$MIG/data/backlog.md" +fm_write_meta "$MIG/state/wedged-origin.meta" \ + 'window=firstmate:fm-wedged-origin' \ + 'worktree=/nonexistent/wedged-origin' \ + 'project=alpha' \ + 'harness=claude' \ + 'decisions_reviewed=1' \ + 'decision_keys=mig-entry' + +VERIFY_MIG_OUT="$MIG/verify.out" +VERIFY_MIG_STATUS=0 +PATH="$MIG_FAKEBIN:$BASE_PATH" FM_HOME="$MIG" \ + FM_STATE_OVERRIDE="$MIG/state" FM_DATA_OVERRIDE="$MIG/data" \ + FM_CONFIG_OVERRIDE="$MIG/config" FM_BACKLOG_ROW_TIMEOUT_SECS="$BOUND_SECS" \ + FM_TEST_PREFIXED_GLOB='bd-*' \ + "$ROOT/bin/fm-captain-hold.sh" verify wedged-origin > "$VERIFY_MIG_OUT" 2>&1 || VERIFY_MIG_STATUS=$? + +[ "$VERIFY_MIG_STATUS" -ne 0 ] \ + || fail "verify must not attest an inventory whose migrated-prefix read wedged: $(cat "$VERIFY_MIG_OUT")" +case "$(cat "$VERIFY_MIG_OUT")" in + *'no captain-held task'*|*absent*) + fail "the migrated-prefix bound hit was spent as an unresolved key: $(cat "$VERIFY_MIG_OUT")" ;; +esac +case "$(cat "$VERIFY_MIG_OUT")" in + *'exceeded its read bound resolving mig-entry') ;; + *) fail "verify must name the entry it could not read and the bound it hit, got: $(cat "$VERIFY_MIG_OUT")" ;; +esac +pass "a bound hit in the migrated-prefix scan stops verify by name instead of resolving to nothing" + +# --- half two: the digest still completes end to end ------------------------ + +E2E="$TMP_ROOT/e2e" +E2E_ROOT="$E2E/root" +E2E_HOME="$E2E/home" +E2E_FAKEBIN="$E2E/fakebin" +mkdir -p "$E2E_HOME/state" "$E2E_HOME/data" "$E2E_HOME/config" "$E2E_FAKEBIN" +git init -q -b main "$E2E_ROOT" +git -C "$E2E_ROOT" commit -q --allow-empty -m init + +make_hanging_tasks_axi "$E2E_FAKEBIN" +# The reconcile sweep this half asserts on runs only under a verified fleet +# lock, and fm-lock.sh finds its holder by walking the invoking process tree +# through `ps`. A CI runner's ancestry carries no harness process, so the lock +# would be refused there and the sweep silently skipped. Pin the lock evidence +# the same way tests/fm-session-start.test.sh's make_fake_ps_harness does: +# every queried pid reports a live `claude` harness, independent of whatever +# process tree the test itself was launched from. +cat > "$E2E_FAKEBIN/ps" <<'SH' +#!/usr/bin/env bash +set -u +case "$*" in + *"comm="*) printf '%s\n' '/usr/local/bin/claude'; exit 0 ;; + *"args="*) printf '%s\n' 'claude'; exit 0 ;; + *"ppid="*) exit 1 ;; +esac +exit 1 +SH +chmod +x "$E2E_FAKEBIN/ps" +fm_fake_exit0 "$E2E_FAKEBIN" tmux node chrome-devtools-axi gh treehouse +fm_fake_version_tool "$E2E_FAKEBIN" lavish-axi FM_FAKE_LAVISH_AXI_VERSION 0.1.46 +fm_fake_version_tool "$E2E_FAKEBIN" gh-axi FM_FAKE_GH_AXI_VERSION 0.1.29 +fm_fake_version_tool "$E2E_FAKEBIN" no-mistakes FM_FAKE_NO_MISTAKES_VERSION \ + 'no-mistakes version v1.46.0 (fake) 2026-06-27T00:02:18Z' + +printf '# Backlog\n' > "$E2E_HOME/data/backlog.md" +# One owned record, so the reconcile sweep actually reads the wedged backend. +fm_write_meta "$E2E_HOME/state/wedged-task.meta" \ + 'window=firstmate:fm-wedged-task' \ + 'worktree=/nonexistent/wedged-task' \ + 'project=alpha' \ + 'harness=claude' \ + 'mode=no-mistakes' \ + 'yolo=off' + +DIGEST="$E2E/digest.out" +DIGEST_START=$(date +%s) +env -u CLAUDECODE -u PI_CODING_AGENT -u FM_PI_HARNESS -u GROK_AGENT \ + FM_HOME="$E2E_HOME" FM_ROOT_OVERRIDE="$E2E_ROOT" PATH="$E2E_FAKEBIN:$BASE_PATH" \ + FM_BACKLOG_ROW_TIMEOUT_SECS="$BOUND_SECS" \ + "$ROOT/bin/fm-session-start.sh" > "$DIGEST" 2>&1 || true +DIGEST_ELAPSED=$(elapsed_since "$DIGEST_START") + +[ "$DIGEST_ELAPSED" -lt "$BOUND_CEILING" ] \ + || fail "session start took ${DIGEST_ELAPSED}s against a wedged backlog backend" + +for SECTION in 'WAKE QUEUE' 'SUPERVISION OPERATING INSTRUCTIONS' 'FLEET STATE' 'CONTEXT'; do + grep -q "$SECTION" "$DIGEST" \ + || fail "the digest lost its $SECTION section against a wedged backlog backend: $(cat "$DIGEST")" +done +pass "a wedged backlog backend still leaves a complete digest: wake queue, supervision instructions, fleet state, and context all print" + +grep -q '^BACKLOG_RECONCILE: wedged-task: ' "$DIGEST" \ + || fail "the wedged item must be reported by name as a partial reconcile: $(cat "$DIGEST")" +pass "an unreachable backlog backend degrades to a loud partial reconcile naming the item it could not read" + +echo "# fm-backlog-read-bound.test.sh: all assertions passed" From 76d44055d81060de6fb8fbe8e94d30291425c0d5 Mon Sep 17 00:00:00 2001 From: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:54:46 -0700 Subject: [PATCH 007/254] fix(merge): serialize away authority with synchronous merges (#4285) * fix(merge): serialize the away-authority check with a synchronous merge bin/fm-pr-merge.sh read the away-posture record for merge authority (the per-task merge grant and the yolo/away-grant decision) and handed the merge to the forge afterwards. An archive at the captain's return or a grant revoked by a replacement record could land in between, so a merge could proceed on away authority that no longer held. The away record now carries a cross-subsystem lock, built on the existing bounded lock primitive rather than a new lock format: the record-mutating subcommands hold it across their mutation, and the merge holds it across both its authority read and the forge command. Because a queued or auto merge returns before the pull request lands, and would therefore outlive the lock, an away merge is now refused whenever it could land asynchronously: a requested --auto, a base branch whose merge-queue state does not prove an immediate merge, and GitLab's asynchronous flags and configuration. What remains permitted while away is the synchronous merge that lands inside the lock. This closes the common away-record/merge race against a live lock owner. It does not make the merge atomic in every case, and two narrow races are accepted and documented at their sites rather than hidden, both confused-agent-grade in the sense bin/fm-lease-lib.sh already uses: - A merge-queue rule change or a PR base change in the window between the queue-free preflight and the forge call can still enqueue the merge, which can then land after its grant lapses. - Killing the lock-owning shell while its gh or glab child is still running lets stale-owner recovery reclaim the lock and the record be archived or replaced, after which the orphaned child can complete the merge on lapsed authority. Closing either one needs landing verification or an ownership handoff, which is deliberately out of scope here. No existing gate is relaxed. The lock is taken after the live green-at-head verify and the captain-hold check, the in-lock authority read is unchanged, and a lock that cannot be taken refuses the merge rather than proceeding unlocked. The away grant stays a structured field; no prose is parsed. * no-mistakes(review): Fix GitHub rollup fixture base branch * no-mistakes(document): Document atomic away-authority merge locking * no-mistakes(ci): Updated two executable GitHub API fixtures to include the required baseRefName. Both previously failing test suites now pass: fm-captain-hold-lifecycle.test.sh and fm-pr-check-security.test.sh. git diff --check also passes --- .agents/skills/afk/SKILL.md | 1 + AGENTS.md | 2 +- bin/fm-afk-contract.sh | 95 ++++++++- bin/fm-pr-merge.sh | 145 ++++++++++--- docs/architecture.md | 9 +- docs/captain-hold-lifecycle.md | 1 + docs/scripts.md | 2 +- tests/fm-afk-contract.test.sh | 60 ++++++ tests/fm-captain-hold-lifecycle.test.sh | 2 +- tests/fm-pr-check-security.test.sh | 2 +- tests/fm-pr-merge.test.sh | 264 +++++++++++++++++++++++- 11 files changed, 543 insertions(+), 40 deletions(-) diff --git a/.agents/skills/afk/SKILL.md b/.agents/skills/afk/SKILL.md index 5b2479e2997..1de10bfddec 100644 --- a/.agents/skills/afk/SKILL.md +++ b/.agents/skills/afk/SKILL.md @@ -86,6 +86,7 @@ A PR ready for merge keeps the merge authority from `AGENTS.md` section 7, and a While the away-posture record exists, a merge proceeds only when that task's recorded yolo posture is on or its id is in the record's merge-grant list; otherwise it is held for the captain's return. A merge grant never releases a captain hold, and it expires when the away record is archived. `--allow-red` remains attended-only and is refused while the record exists. +A merge under away authority must be synchronous; `fm-pr-merge.sh` refuses auto-merge and any GitHub queue state that cannot prove an immediate merge while the record exists. A mandate clause is the captain's explicit instruction given before leaving, recorded with its named object and condition; a clause is never inferred, never applied by analogy, and expires at return. Forbidden, destructive, irreversible, and security-sensitive actions are never pre-authorizable regardless of clause text, and no recorded clause is authority by itself. This release records clauses and does not execute them. diff --git a/AGENTS.md b/AGENTS.md index 0c7cf568b99..135831d62f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,7 +140,7 @@ state/ runtime records and signals; gitignored .watcher-down private generation-bound recovery state coupling watcher downtime, durable wake presentation, and post-handling acknowledgement; never touch ..open-decisions-cursor per-task byte cursor and folded open-decision set bounding the OPEN DECISIONS scan's cost to new status-log appends; written only by fm-classify-lib.sh's status_open_decisions_incremental, removed by teardown, safe to delete (forces one full re-fold) .status-presentation-cursor .status-presentation-lock fleet-wide per-task status identity plus independent annotation and outcome-backstop byte offsets, with a serialization lock preventing already-presented lines from replaying while preserving delayed signal annotations; owned by fm-classify-lib.sh, with each task's row retired by teardown - .afk-contract the away-posture record: the captain's verbatim away words, expected return, reach profile, spend cap, and structured mandate clauses; written only by bin/fm-afk-contract.sh after the captain confirms the read-back, archived under afk-contracts/ at return; its presence IS the away posture in every harness + .afk-contract the away-posture record: the captain's verbatim away words, expected return, reach profile, spend cap, and structured mandate clauses; written only by bin/fm-afk-contract.sh after the captain confirms the read-back, archived under afk-contracts/ at return; its presence IS the away posture in every harness; its sibling .afk-contract.lock serializes actions authorized by the live record (contract: bin/fm-afk-contract.sh) afk-contracts/ archived away-posture records: one final record per away window keyed by entry time, plus any superseded mandates from that window .afk durable away-mode daemon flag on the harnesses that still launch the daemon (never on Pi); present = sub-supervisor may inject escalations (set by the daemon entry, cleared on user return) .watch.lock .wake-queue.lock watcher singleton and queue serialization locks diff --git a/bin/fm-afk-contract.sh b/bin/fm-afk-contract.sh index 272e810846d..04f8197f9a6 100755 --- a/bin/fm-afk-contract.sh +++ b/bin/fm-afk-contract.sh @@ -112,9 +112,27 @@ # fm-afk-contract.sh archive move the record aside; print its path # fm-afk-contract.sh archived print that archived record's path # -# Sourceable: with the BASH_SOURCE guard, other scripts get the path and -# presence helpers (fm_afk_contract_path, fm_afk_contract_present, -# fm_afk_contract_proposal_path, fm_afk_contract_archive_dir) without running main. +# CROSS-SUBSYSTEM LOCK (state/.afk-contract.lock; this script is its one owner). +# This record is authority another subsystem reads and then ACTS on outside this +# script: bin/fm-pr-merge.sh reads the merge grants and afterwards hands a merge +# to the forge. A publication, replacement, or archive landing between that read +# and the forge handoff would land a merge on authority that no longer holds, so +# the two subsystems share one lock instead of each locking its own records: the +# record-mutating subcommands (confirm, archive) hold it across their mutation, +# and a reader that acts on the record holds it across both its read and that +# action (fm_afk_contract_lock_hold / fm_afk_contract_lock_release). The +# read-only subcommands never take it, so a holder can still read the record it +# locked. Neither side ever proceeds without it: the acquire is bounded, and a +# bound that is hit refuses and names the live holder rather than racing. That +# fixed bound is 120 seconds, sized so only a genuinely wedged holder trips it. +# A lock left by a killed process is reclaimed +# by the ordinary stale-owner recovery in bin/fm-wake-lib.sh, which owns the lock +# primitive itself. +# +# Sourceable: with the BASH_SOURCE guard, other scripts get the path, presence, +# and lock helpers (fm_afk_contract_path, fm_afk_contract_present, +# fm_afk_contract_proposal_path, fm_afk_contract_archive_dir, +# fm_afk_contract_lock_hold, fm_afk_contract_lock_release) without running main. set -u FM_AFK_CONTRACT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -129,6 +147,10 @@ FM_AFK_CONTRACT_VERSION=1 FM_AFK_CONTRACT_VERBS="merge land prerelease install rerun dispatch abort-run answer discard wake-me" FM_AFK_CONTRACT_REACH_ANNOUNCED='No phone channel is configured; anything that needs you waits for your return.' FM_AFK_CONTRACT_SPEND_DEFAULT=4 +# Generous against the longest legitimate holder, a merge waiting on the forge, +# so the bound only ever trips on something genuinely wedged. +_FM_AFK_CONTRACT_LOCK_TIMEOUT=120 +FM_AFK_CONTRACT_LOCK_HELD= fm_afk_contract_path() { # [state-dir] printf '%s/.afk-contract' "${1:-$FM_AFK_CONTRACT_STATE}" @@ -146,6 +168,54 @@ fm_afk_contract_present() { # [state-dir] [ -f "$(fm_afk_contract_path "${1:-$FM_AFK_CONTRACT_STATE}")" ] } +fm_afk_contract_lock_path() { # [state-dir] + printf '%s/.afk-contract.lock' "${1:-$FM_AFK_CONTRACT_STATE}" +} + +# Lazily reach the lock primitive. bin/fm-wake-lib.sh is a canonical lint root +# in its own right, so keep this an analysis boundary for the same reason +# bin/fm-lease-lib.sh's fm_lease_lock_helpers does. +fm_afk_contract_lock_helpers() { + command -v fm_lock_acquire_wait_bounded >/dev/null 2>&1 && return 0 + # shellcheck source=/dev/null + . "$FM_AFK_CONTRACT_DIR/fm-wake-lib.sh" +} + +# fm_afk_contract_lock_hold [state-dir]: take the cross-subsystem lock described +# in the header. The acquire is bounded so a wedged holder is refused instead of +# blocking a merge or a captain return forever, and returns 1 WITHOUT the lock so +# every caller refuses rather than proceeding unlocked. +fm_afk_contract_lock_hold() { # [state-dir] + local lock rc=0 STATE timeout + STATE=${1:-$FM_AFK_CONTRACT_STATE} + lock=$(fm_afk_contract_lock_path "$STATE") + timeout=${FM_TEST_AFK_CONTRACT_LOCK_TIMEOUT:-$_FM_AFK_CONTRACT_LOCK_TIMEOUT} + fm_afk_contract_lock_helpers || { + fm_afk_contract_log "could not load the lock primitive for $lock" + return 1 + } + fm_lock_acquire_wait_bounded "$lock" "$timeout" || rc=$? + if [ "$rc" -ne 0 ]; then + if [ "$rc" -eq 124 ] && [ -n "${FM_LOCK_HELD_PID:-}" ]; then + fm_afk_contract_log "the away-posture record is locked by live process $FM_LOCK_HELD_PID (an in-flight merge, or another change to this record); nothing was changed" + else + fm_afk_contract_log "could not take the away-posture record lock at $lock; nothing was changed" + fi + return 1 + fi + FM_AFK_CONTRACT_LOCK_HELD=$lock +} + +# Release the lock taken by fm_afk_contract_lock_hold. Idempotent, so callers can +# invoke it unconditionally from their own cleanup. +fm_afk_contract_lock_release() { + local lock=$FM_AFK_CONTRACT_LOCK_HELD + [ -n "$lock" ] || return 0 + FM_AFK_CONTRACT_LOCK_HELD= + fm_afk_contract_lock_helpers || return 1 + fm_lock_release "$lock" +} + fm_afk_contract_log() { printf 'fm-afk-contract: %s\n' "$*" >&2; } fm_afk_contract_usage() { @@ -874,13 +944,28 @@ fm_afk_contract_select_path() { # -> prints the record path chosen by printf '%s' "$path" } +# The record-mutating subcommands run inside the cross-subsystem lock, so no +# publication, replacement, or archive can land between another subsystem's +# authority read and the action it takes on that authority. +fm_afk_contract_locked_cmd() { # [args...] + local rc=0 + fm_afk_contract_lock_hold || return 1 + trap 'fm_afk_contract_lock_release || true' EXIT + "$@" || rc=$? + trap - EXIT + fm_afk_contract_lock_release || true + return "$rc" +} + fm_afk_contract_main() { local cmd=${1:-} path [ -n "$cmd" ] || { fm_afk_contract_usage >&2; return 2; } shift case "$cmd" in propose) fm_afk_contract_cmd_propose "$@" ;; - confirm) [ "$#" -eq 0 ] || { fm_afk_contract_usage >&2; return 2; }; fm_afk_contract_cmd_confirm ;; + confirm) + [ "$#" -eq 0 ] || { fm_afk_contract_usage >&2; return 2; } + fm_afk_contract_locked_cmd fm_afk_contract_cmd_confirm ;; readback) path=$(fm_afk_contract_select_path "$@") || { fm_afk_contract_usage >&2; return 2; } [ -f "$path" ] || { fm_afk_contract_log "no record at $path"; return 1; } @@ -917,7 +1002,7 @@ fm_afk_contract_main() { path=$(fm_afk_contract_select_path "$@") || { fm_afk_contract_usage >&2; return 2; } [ -f "$path" ] || { fm_afk_contract_log "no record at $path"; return 1; } fm_afk_contract_read_grants "$path" ;; - archive) fm_afk_contract_cmd_archive ;; + archive) fm_afk_contract_locked_cmd fm_afk_contract_cmd_archive ;; archived) [ "$#" -eq 1 ] || { fm_afk_contract_usage >&2; return 2; } path="$(fm_afk_contract_archive_dir)/$1.afk-contract" diff --git a/bin/fm-pr-merge.sh b/bin/fm-pr-merge.sh index 85319a13777..9ca7bfb2f9e 100755 --- a/bin/fm-pr-merge.sh +++ b/bin/fm-pr-merge.sh @@ -30,11 +30,13 @@ # refuses, reporting the failed gh read and naming both failed reads when the # gh-axi view could not prove the outcome either. # If the pull request remains open and the base branch has an effective -# merge_queue rule, the refusal names the queue's configured merge method and -# the exact --attended-override -- --auto -- retry flags, unless the caller already passed -# that method with --auto to a merge command that returned success, in which -# case it reports instead that the accepted request has not entered the queue -# and the queue state has to be re-checked. +# merge_queue rule, an attended refusal names the queue's configured merge +# method and exact --attended-override -- --auto -- retry flags. While +# the away-posture record exists, asynchronous merge requests are refused and +# queue retry flags are not offered because they would outlive away authority. +# An attended caller that already passed the configured method with --auto is +# told instead that the accepted request has not entered the queue and its queue +# state has to be re-checked. # No method is selected for the caller in any case. A rules response that names # no queue rule, one that could not be read, rules that disagree, and a method # this script does not recognise are four distinct outcomes and are reported @@ -73,10 +75,17 @@ # held for the captain return. An unreadable record refuses rather than being # skipped. Neither posture releases a captain hold, and the grant lapses when # the record is archived. +# The authority read and synchronous forge command share the away record's +# cross-subsystem lock, which bin/fm-afk-contract.sh owns, closing the common +# live-owner TOCTOU; failure to take it refuses before the forge call. Async and +# queued paths are refused while away. Two confused-agent-grade limitations are +# accepted rather than hidden: queue or base changes after GitHub's preflight can +# still enqueue, and killing this shell can orphan a forge child after stale-lock +# recovery. docs/architecture.md owns those away-merge limits, while +# docs/captain-hold-lifecycle.md owns the separate merge-to-cleanup residual. # A failed forge command releases the lock after it returns. A successful one # retains the lock until the accepted merge authority is persisted against the -# still-matching task metadata; docs/captain-hold-lifecycle.md owns the accepted -# asynchronous-landing and merge-to-cleanup residuals. +# still-matching task metadata. # # Extra args must not include --repo or -R in any form, including a bundled # short-option cluster such as -yR, because the repository comes only from the @@ -274,6 +283,26 @@ reject_repo_overrides "$@" || exit 1 reject_head_overrides "$@" || exit 1 reject_protected_forge_args "$@" || exit 1 +FM_PR_GITHUB_AUTO_REQUESTED=false +if [ "$PROVIDER" = github ] && caller_requested_auto_merge "$@"; then + FM_PR_GITHUB_AUTO_REQUESTED=true +fi +FM_PR_GITLAB_ASYNC_REQUESTED=false +if [ "$PROVIDER" = gitlab ]; then + for arg in "$@"; do + case "$arg" in + --auto-merge|--when-pipeline-succeeds) FM_PR_GITLAB_ASYNC_REQUESTED=true ;; + --auto-merge=*|--when-pipeline-succeeds=*) + case "${arg#*=}" in + [tT]|[tT][rR][uU][eE]|1) FM_PR_GITLAB_ASYNC_REQUESTED=true ;; + [fF]|[fF][aA][lL][sS][eE]|0) FM_PR_GITLAB_ASYNC_REQUESTED=false ;; + esac + ;; + esac + done +fi +FM_PR_AWAY_POSTURE=false + fm_backlog_directory_present "$STATE" "state directory" || { echo "error: PR merge refused: $FM_BACKLOG_TRANSITION_ERROR" >&2 exit 1 @@ -304,6 +333,7 @@ MERGE_CONTROL_LOCK= MERGE_META_LOCK= merge_control_cleanup() { [ -z "$MERGE_META_LOCK" ] || fm_lock_release "$MERGE_META_LOCK" || true + fm_afk_contract_lock_release || true [ -z "$MERGE_CONTROL_LOCK" ] || fm_lock_release "$MERGE_CONTROL_LOCK" || true } trap merge_control_cleanup EXIT @@ -355,11 +385,12 @@ fi # the merge request. Sets FM_PR_MERGE_HEAD to the verified head on success and # returns non-zero after reporting every condition that failed. FM_PR_MERGE_HEAD= +FM_PR_GITLAB_ASYNC_CONFIGURED=false gitlab_verify_mergeable() { local json fields line local total=0 named=0 refusals='' local state='' detail='' conflicts='' discussions='' - local live_head='' pipeline_sha='' pipeline_status='' + local live_head='' pipeline_sha='' pipeline_status='' async_configured='' # GITLAB_HOST is set to the same host the project URL already carries, so the # instance is taken from the parsed URL by both signals and never from the @@ -381,7 +412,8 @@ gitlab_verify_mergeable() { "discussions=" + (.blocking_discussions_resolved | tostring), "head=" + ((.sha // "") | tostring), "pipeline_sha=" + ((.head_pipeline.sha // "") | tostring), - "pipeline_status=" + ((.head_pipeline.status // "") | tostring) + "pipeline_status=" + ((.head_pipeline.status // "") | tostring), + "async_configured=" + (if .merge_when_pipeline_succeeds == true or (.merge_after != null) then "true" else "false" end) else error("merge request payload is not an object") end' 2>/dev/null); then @@ -398,6 +430,7 @@ gitlab_verify_mergeable() { head=*) live_head=${line#head=} ;; pipeline_sha=*) pipeline_sha=${line#pipeline_sha=} ;; pipeline_status=*) pipeline_status=${line#pipeline_status=} ;; + async_configured=*) async_configured=${line#async_configured=} ;; *) continue ;; esac named=$((named + 1)) @@ -407,7 +440,7 @@ FIELDS # Every field named exactly once and no unnamed line: a value carrying a # newline would split into a line no name matches, so it is refused here # rather than silently truncated into a value a check could accept. - if [ "$named" -ne 7 ] || [ "$total" -ne 7 ]; then + if [ "$named" -ne 8 ] || [ "$total" -ne 8 ]; then echo "error: could not read the GitLab merge request state before merging" >&2 return 1 fi @@ -450,6 +483,7 @@ FIELDS printf 'verified: %s is open and mergeable, with a successful pipeline at head %s\n' \ "$URL" "$live_head" >&2 FM_PR_MERGE_HEAD=$live_head + FM_PR_GITLAB_ASYNC_CONFIGURED=$async_configured } # Every GitHub check that is not green in the given live pull-request JSON, one @@ -535,9 +569,9 @@ github_checks_not_green() { github_verify_mergeable() { local json fields line red name covered local total=0 named=0 refusals='' - local state='' draft='' mergeable='' merge_state='' live_head='' + local state='' draft='' mergeable='' merge_state='' live_head='' base='' - if ! json=$(gh pr view "$URL" --json state,isDraft,mergeable,mergeStateStatus,headRefOid,statusCheckRollup 2>/dev/null) \ + if ! json=$(gh pr view "$URL" --json state,isDraft,mergeable,mergeStateStatus,headRefOid,baseRefName,statusCheckRollup 2>/dev/null) \ || [ -z "$json" ]; then echo "error: could not read the GitHub pull request state before merging" >&2 return 1 @@ -548,7 +582,8 @@ github_verify_mergeable() { "draft=" + (if (.isDraft | type) == "boolean" then (.isDraft | tostring) else "" end), "mergeable=" + ((.mergeable // "") | tostring), "merge_state=" + ((.mergeStateStatus // "") | tostring), - "head=" + ((.headRefOid // "") | tostring) + "head=" + ((.headRefOid // "") | tostring), + "base=" + ((.baseRefName // "") | tostring) else error("pull request payload is not an object") end' 2>/dev/null); then @@ -563,13 +598,14 @@ github_verify_mergeable() { mergeable=*) mergeable=${line#mergeable=} ;; merge_state=*) merge_state=${line#merge_state=} ;; head=*) live_head=${line#head=} ;; + base=*) base=${line#base=} ;; *) continue ;; esac named=$((named + 1)) done <&2 return 1 fi @@ -627,6 +663,7 @@ EOF printf 'verified: %s is open and mergeable, with every required check green at head %s\n' \ "$URL" "$live_head" >&2 FM_PR_MERGE_HEAD=$live_head + FM_PR_GITHUB_BASE=$base } # Read one live GitHub pull request view after gh returns. The selected @@ -857,9 +894,35 @@ require_away_merge_grant() { return 1 } +# Take the away record's own lock (bin/fm-afk-contract.sh owns it) so that +# record cannot be published, replaced, or archived between the authority read +# below and the forge command that acts on it. Refuses without the lock: a merge +# on authority nothing is holding still is exactly what this closes. This is the +# only path that holds both the per-task control lock and the away-record lock, +# and it always takes them in that order; the away-record side takes only its own +# lock, so the pair cannot deadlock. +hold_away_record_for_merge() { + fm_afk_contract_lock_hold "$STATE" && return 0 + echo "error: PR merge refused - the away-posture record could not be locked for the merge; nothing was merged" >&2 + return 1 +} + require_current_away_authority() { + FM_PR_AWAY_POSTURE=false + if fm_afk_contract_present "$STATE"; then + FM_PR_AWAY_POSTURE=true + if [ "$PROVIDER" = github ] && [ "$FM_PR_GITHUB_AUTO_REQUESTED" = true ]; then + echo "error: --auto is attended-only; while the away-posture record exists only a synchronous merge may run under its authority lock" >&2 + return 2 + fi + if [ "$PROVIDER" = gitlab ] \ + && { [ "$FM_PR_GITLAB_ASYNC_REQUESTED" = true ] || [ "$FM_PR_GITLAB_ASYNC_CONFIGURED" = true ]; }; then + echo "error: GitLab auto-merge is attended-only; while the away-posture record exists only an immediate merge may run under its authority lock" >&2 + return 2 + fi + fi require_away_merge_grant || return 1 - if fm_afk_contract_present "$STATE" && [ "${#ALLOW_RED[@]}" -gt 0 ]; then + if [ "$FM_PR_AWAY_POSTURE" = true ] && [ "${#ALLOW_RED[@]}" -gt 0 ]; then echo "error: --allow-red is attended-only; while the away-posture record exists the green check is absolute" >&2 return 2 fi @@ -882,6 +945,17 @@ persist_accepted_merge_authority() { return 1 } +refuse_github_queue_while_away() { + [ "$FM_PR_AWAY_POSTURE" = true ] || return 0 + # Accepted confused-agent-grade limitation, as in bin/fm-lease-lib.sh, not an + # oversight: a queue rule or PR base change after this preflight can still + # enqueue the merge, which can land after its away grant lapses. + github_read_queue_method + [ "$FM_PR_GITHUB_QUEUE_STATUS" = none ] && return 0 + echo "error: GitHub merge refused while away because the base branch's merge-queue state does not prove an immediate merge; nothing was handed to the forge" >&2 + return 2 +} + require_recorded_pr_identity() { local existing existing=$(grep '^pr=' "$META" | tail -1 | cut -d= -f2- || true) @@ -891,7 +965,6 @@ require_recorded_pr_identity() { return 1 } -FM_PR_GITHUB_AUTO_REQUESTED=false FM_PR_GITHUB_MERGE_ACCEPTED=false FM_PR_GITHUB_CALLER_METHOD= @@ -935,6 +1008,10 @@ github_caller_method_is() { github_report_queue_rules() { local queue_method methods_display + if [ "$FM_PR_AWAY_POSTURE" = true ]; then + printf 'error: the direct merge did not land while the away-posture record exists; merge-queue retry flags are unavailable because a queued merge would outlive its authority\n' >&2 + return 0 + fi github_read_queue_method case "$FM_PR_GITHUB_QUEUE_STATUS" in single) @@ -987,8 +1064,12 @@ github_report_unmerged_outcome() { fi fi if [ "$FM_PR_GITHUB_QUEUE_OBSERVED" != true ]; then - printf 'error: the merge queue could not be observed for %s because the queue-aware read was unavailable, so a pull request already in the merge queue cannot be told apart from one that never entered it; re-check the pull request'"'"'s merge queue state before retrying\n' \ - "$URL" >&2 + if [ "$FM_PR_AWAY_POSTURE" = true ]; then + printf 'error: the synchronous merge did not land while the away-posture record exists; no asynchronous merge or queue retry is available under away authority\n' >&2 + else + printf 'error: the merge queue could not be observed for %s because the queue-aware read was unavailable, so a pull request already in the merge queue cannot be told apart from one that never entered it; re-check the pull request'"'"'s merge queue state before retrying\n' \ + "$URL" >&2 + fi return 0 fi github_report_queue_rules @@ -1022,6 +1103,10 @@ require_recorded_pr_identity || exit 1 record_pr_metadata || exit 1 require_released_captain_hold || exit 1 +# Accepted confused-agent-grade limitation, as in bin/fm-lease-lib.sh, not an +# oversight: if this lock-owning shell dies while its gh or glab child lives, +# stale-owner recovery can release the record for archive or replacement and +# the orphaned forge child can still merge on the lapsed away authority. case "$PROVIDER" in github) merge_output= @@ -1029,16 +1114,15 @@ case "$PROVIDER" in if ! caller_has_merge_method "$@"; then merge_args=(--squash) fi - if caller_requested_auto_merge "$@"; then - FM_PR_GITHUB_AUTO_REQUESTED=true - fi FM_PR_GITHUB_CALLER_METHOD=$(caller_merge_method "$@") github_verify_mergeable || exit 1 - # This last presence and authority read narrows the publication race to the - # forge handoff; without a shared lock, a residual sub-second race remains. + # The away record is locked first, so this last presence and authority read + # and the forge command below share one live-owner critical section. + hold_away_record_for_merge || exit 1 away_status=0 require_current_away_authority || away_status=$? [ "$away_status" -eq 0 ] || exit "$away_status" + refuse_github_queue_while_away || exit 2 merge_status=0 merge_output=$(gh pr merge "$PR_NUMBER" --repo "$PR_OWNER/$PR_REPO" \ --match-head-commit "$FM_PR_MERGE_HEAD" \ @@ -1046,9 +1130,11 @@ case "$PROVIDER" in if [ "$merge_status" -eq 0 ]; then FM_PR_GITHUB_MERGE_ACCEPTED=true persist_accepted_merge_authority || exit 1 + fm_afk_contract_lock_release || true fm_lock_release "$MERGE_CONTROL_LOCK" || true MERGE_CONTROL_LOCK= else + fm_afk_contract_lock_release || true fm_lock_release "$MERGE_CONTROL_LOCK" || true MERGE_CONTROL_LOCK= [ -z "$merge_output" ] || printf '%s\n' "$merge_output" >&2 @@ -1085,20 +1171,27 @@ case "$PROVIDER" in # in between is refused by GitLab instead of merged unverified. --yes only # skips the interactive confirmation, which no supervised run can answer; # the conditions above are what authorize the merge. - # This last presence and authority read narrows the publication race to the - # forge handoff; without a shared lock, a residual sub-second race remains. + # The away record is locked first, so this last presence and authority read + # and the forge command below share one live-owner critical section. + hold_away_record_for_merge || exit 1 away_status=0 require_current_away_authority || away_status=$? [ "$away_status" -eq 0 ] || exit "$away_status" merge_status=0 + gitlab_merge_args=() + if [ "$FM_PR_AWAY_POSTURE" = true ]; then + gitlab_merge_args=(--auto-merge=false) + fi GITLAB_HOST="$FM_PR_HOST" glab mr merge "$PR_NUMBER" -R "$PROJECT_URL" \ - --sha "$FM_PR_MERGE_HEAD" --yes "$@" || merge_status=$? + --sha "$FM_PR_MERGE_HEAD" --yes "$@" "${gitlab_merge_args[@]+"${gitlab_merge_args[@]}"}" || merge_status=$? if [ "$merge_status" -ne 0 ]; then + fm_afk_contract_lock_release || true fm_lock_release "$MERGE_CONTROL_LOCK" || true MERGE_CONTROL_LOCK= exit "$merge_status" fi persist_accepted_merge_authority || exit 1 + fm_afk_contract_lock_release || true fm_lock_release "$MERGE_CONTROL_LOCK" || true MERGE_CONTROL_LOCK= gitlab_confirm_rc=0 diff --git a/docs/architecture.md b/docs/architecture.md index ef1b1519c54..1ccf3aa7c45 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -317,11 +317,18 @@ A `https://github.com///pull/` URL requires `gh` and `jq`, is me A check run is green when its current run is green, because GitHub leaves a cancelled run in the rollup beside the passing re-run it triggered when the base branch advanced; `bin/fm-pr-merge.sh`'s `github_checks_not_green` owns the rule, which uses `startedAt` to clear only an older completed check run that a passing run with the same name provably replaced, while unfinished check runs and non-green status contexts stay red. `--auto`, `--admin`, and branch-deletion flags are refused unless `--attended-override` is passed for an explicit captain instruction; that override never skips the live green check, the away-grant check, or a captain hold. An attended `--allow-red ` may appear once, waives only GitHub checks with that exact name, and is refused while the away-posture record exists. +Because away merge authority is read from that record and then acted on by the forge, the authority read and synchronous forge command share the record's cross-subsystem lock, closing the common live-owner TOCTOU. +A lock that cannot be taken refuses the merge. +While the record exists, GitHub auto-merge and any base whose rules cannot prove the absence of a merge queue are refused before submission, and GitLab auto-merge flags or scheduled state are refused while an immediate merge is forced with a final `--auto-merge=false`. +This is deliberately confused-agent-grade, as `bin/fm-lease-lib.sh` defines that grade, rather than fully atomic. +A GitHub queue-rule or PR-base change after the queue-free preflight can still enqueue a merge that lands after its away grant lapses, and killing the lock-owning shell while its forge child survives lets stale-owner recovery admit archive or replacement before that child completes. +These are accepted limitations, not oversights; durable authority, landing re-verification, and child-lock handoff are outside this boundary. +`bin/fm-afk-contract.sh` owns the lock contract, while `tests/fm-afk-contract.test.sh` and `tests/fm-pr-merge.test.sh` pin the serialization and fail-closed merge behavior. A `https:////-/merge_requests/` URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) invokes `glab mr merge -R https:///`, so the instance comes from the URL, and adds no merge-method flag because the project's own merge method applies. That path merges only after one live read of the merge request confirms it is open, mergeable, conflict-free, with blocking discussions resolved and a successful pipeline at the current head, and it binds the merge to that verified head; recorded metadata is never the authority for those conditions because a rebase leaves it stale. After either forge command returns, the script confirms the PR or MR actually landed, and only a confirmed landing records a landed outcome; a queued or unconfirmed request records none and leaves its poll armed. On GitLab an auto-merge-queued or unconfirmed request is reported without failing the run. -On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and a base branch that requires the merge queue is refused with the concrete `--attended-override -- --auto --` retry flags its configured method requires rather than having a merge method chosen on the caller's behalf. +On GitHub an outcome that is neither merged nor queued is refused loudly and non-zero, naming the observed state, and in attended posture a base branch that requires the merge queue is refused with the concrete `--attended-override -- --auto --` retry flags its configured method requires rather than having a merge method chosen on the caller's behalf. When the forge already accepted exactly those flags and the pull request still has not entered the queue, that refusal points at the queue state to re-check instead of echoing back the flags the caller just ran. An auto-merge request is held to the same standard: `--auto` that leaves the pull request neither merged nor queued is refused rather than reported as success. Every GitHub refusal states what it could not observe as plainly as what it did, so an unreadable branch-rule response, an unrecognised queue method, and a merge queue no available read can see are each named rather than left to look like a base branch with no queue at all. diff --git a/docs/captain-hold-lifecycle.md b/docs/captain-hold-lifecycle.md index 2de7b260a89..88da8e585f0 100644 --- a/docs/captain-hold-lifecycle.md +++ b/docs/captain-hold-lifecycle.md @@ -154,6 +154,7 @@ The window between a merge landing and cleanup is an accepted structural residua That local window is normally only seconds wide and requires re-holding a task whose merge has just landed. A re-hold inside the window makes cleanup retain the row rather than publish it, so the delivery is omitted until the stale hold is cleared from that row. Queued forge merges cannot be covered locally because the forge performs the merge asynchronously after the local command has returned, when no lock this code could hold would still be held. +The away-posture restriction on queued merges and its residual limits are owned by [architecture.md](architecture.md#delivery-modes-are-explicit-per-task). ## Record divergence diff --git a/docs/scripts.md b/docs/scripts.md index 048a064aa3c..09a27089aae 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -87,7 +87,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-watch-checkpoint.sh` | Run one bounded foreground watcher checkpoint for Codex-style supervision | | `fm-watch.sh` | Singleton-safe watcher: absorb benign wakes, detect stalled local-secondmate wake queues, and exit on actionable ones | | `fm-inactive-reconcile.sh` | Reconcile long-inactive direct crewmate terminal outcomes without forge access | -| `fm-afk-contract.sh` | Own the away-posture record: schema, mandate-clause fields and never-set scan, refusal naming the missing part, read-back, entry announcement, archive | +| `fm-afk-contract.sh` | Own the away-posture record: schema, mandate-clause fields and never-set scan, refusal naming the missing part, read-back, entry announcement, archive, and cross-subsystem authority lock | | `fm-afk-start.sh` | Run the common sourceable away-mode daemon entry in the foreground | | `fm-afk-launch.sh` | Own away-mode entry (read-back, confirm, record), exit, rollback, and any backend terminal lifecycle | | `fm-afk-return.sh` | Own deterministic return shutdown, the return brief, catch-up evidence, and the firstmate-actionable blocker gate | diff --git a/tests/fm-afk-contract.test.sh b/tests/fm-afk-contract.test.sh index 8a6ba9fb580..5ccacb6b58e 100755 --- a/tests/fm-afk-contract.test.sh +++ b/tests/fm-afk-contract.test.sh @@ -617,6 +617,65 @@ test_archive_drops_live_grants() { pass "archive removes live grants so archived copies are not consulted" } +# The record-mutating commands share one lock with the subsystems that read this +# record's authority and then act on it (bin/fm-pr-merge.sh reads the grants and +# merges). While a reader holds that lock, confirm and archive must refuse and +# change nothing, so no publication, replacement, or archive can land inside the +# window between that read and the action it authorized. +test_record_changes_refuse_while_a_reader_holds_the_lock() { + local home lock holder_pid i rc out before + home=$(make_home lock-contended) + contract "$home" propose --grant task-x1 >/dev/null || fail "lock-contended: proposal failed" + contract "$home" confirm >/dev/null || fail "lock-contended: confirm failed" + before=$(cat "$home/state/.afk-contract") + lock="$home/state/.afk-contract.lock" + + FM_STATE_OVERRIDE="$home/state" bash -c ' + . "$1" + fm_lock_acquire_wait "$2" || exit 10 + printf "ready\n" > "$3" + while [ ! -e "$4" ]; do sleep 0.05; done + fm_lock_release "$2" + ' _ "$ROOT/bin/fm-wake-lib.sh" "$lock" "$home/holder.ready" "$home/release" & + holder_pid=$! + i=0 + while [ "$i" -lt 100 ] && [ ! -s "$home/holder.ready" ]; do + sleep 0.05 + i=$((i + 1)) + done + [ -s "$home/holder.ready" ] \ + || { kill "$holder_pid" 2>/dev/null || true; fail "lock-contended: the fixture never took the lock"; } + + set +e + out=$(FM_TEST_AFK_CONTRACT_LOCK_TIMEOUT=1 contract "$home" archive 2>&1) + rc=$? + set -e + [ "$rc" -ne 0 ] || { kill "$holder_pid" 2>/dev/null || true; fail "lock-contended: archive ran while the record was locked"; } + assert_contains "$out" 'locked by live process' "lock-contended: the archive refusal did not name the live holder" + [ -f "$home/state/.afk-contract" ] \ + || { kill "$holder_pid" 2>/dev/null || true; fail "lock-contended: the refused archive still moved the record"; } + + contract "$home" propose --grant task-other >/dev/null || fail "lock-contended: replacement proposal failed" + set +e + out=$(FM_TEST_AFK_CONTRACT_LOCK_TIMEOUT=1 contract "$home" confirm 2>&1) + rc=$? + set -e + [ "$rc" -ne 0 ] || { kill "$holder_pid" 2>/dev/null || true; fail "lock-contended: confirm replaced the record while it was locked"; } + assert_contains "$out" 'locked by live process' "lock-contended: the confirm refusal did not name the live holder" + [ "$(cat "$home/state/.afk-contract")" = "$before" ] \ + || { kill "$holder_pid" 2>/dev/null || true; fail "lock-contended: the refused confirm changed the standing record"; } + [ "$(contract "$home" grants)" = task-x1 ] \ + || { kill "$holder_pid" 2>/dev/null || true; fail "lock-contended: a read subcommand did not see the unchanged grants"; } + + : > "$home/release" + wait "$holder_pid" || fail "lock-contended: the fixture holder did not release cleanly" + contract "$home" confirm >/dev/null 2>&1 || fail "lock-contended: confirm failed once the lock cleared" + [ "$(contract "$home" grants)" = task-other ] \ + || fail "lock-contended: the released replacement did not take effect" + contract "$home" archive >/dev/null || fail "lock-contended: archive failed once the lock cleared" + pass "confirm and archive refuse while the record is locked, and proceed once it clears" +} + test_fields_refuse_each_missing_part_by_name test_omitted_stop_confirms_as_no_stop test_never_set_flags_without_refusing_and_never_over_matches @@ -641,4 +700,5 @@ test_merge_grants_empty_form_and_usage_errors test_legacy_record_without_merge_grants_reads_empty test_malformed_merge_grants_refuse_validation test_archive_drops_live_grants +test_record_changes_refuse_while_a_reader_holds_the_lock diff --git a/tests/fm-captain-hold-lifecycle.test.sh b/tests/fm-captain-hold-lifecycle.test.sh index dae536d8684..91ad77d9eca 100755 --- a/tests/fm-captain-hold-lifecycle.test.sh +++ b/tests/fm-captain-hold-lifecycle.test.sh @@ -106,7 +106,7 @@ case "${1:-} ${2:-}" in "pr view") case " $* " in *statusCheckRollup*) - printf '%s\n' '{"state":"OPEN","isDraft":false,"mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"1111111111111111111111111111111111111111","statusCheckRollup":[{"__typename":"CheckRun","name":"ci","status":"COMPLETED","conclusion":"SUCCESS"}]}' + printf '%s\n' '{"state":"OPEN","isDraft":false,"mergeable":"MERGEABLE","mergeStateStatus":"CLEAN","headRefOid":"1111111111111111111111111111111111111111","baseRefName":"main","statusCheckRollup":[{"__typename":"CheckRun","name":"ci","status":"COMPLETED","conclusion":"SUCCESS"}]}' ;; *headRefOid*) printf '%s\n' 1111111111111111111111111111111111111111 ;; esac diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index 40d8438fcb4..b38435781bc 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -147,7 +147,7 @@ case "${1:-} ${2:-}" in "pr view") case " $* " in *statusCheckRollup*) - printf '%s\n' "{\"state\":\"OPEN\",\"isDraft\":false,\"mergeable\":\"MERGEABLE\",\"mergeStateStatus\":\"CLEAN\",\"headRefOid\":\"${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}\",\"statusCheckRollup\":[{\"__typename\":\"CheckRun\",\"name\":\"ci\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\"}]}" + printf '%s\n' "{\"state\":\"OPEN\",\"isDraft\":false,\"mergeable\":\"MERGEABLE\",\"mergeStateStatus\":\"CLEAN\",\"headRefOid\":\"${FM_TEST_GH_HEAD:-0123456789abcdef0123456789abcdef01234567}\",\"baseRefName\":\"main\",\"statusCheckRollup\":[{\"__typename\":\"CheckRun\",\"name\":\"ci\",\"status\":\"COMPLETED\",\"conclusion\":\"SUCCESS\"}]}" exit 0 ;; esac diff --git a/tests/fm-pr-merge.test.sh b/tests/fm-pr-merge.test.sh index 5f4122b001e..b90c0468117 100755 --- a/tests/fm-pr-merge.test.sh +++ b/tests/fm-pr-merge.test.sh @@ -65,7 +65,7 @@ write_github_live_json() { local case_dir=$1 head=$2 printf '%s\n' "$head" > "$case_dir/github-head" cat > "$case_dir/github-view.json" < "$case_dir/github-head" cat > "$case_dir/github-view.json" < "$case_dir/github-head" cat > "$case_dir/github-view.json" < "$FM_TEST_META_AT_MERGE" fi + # The forge call runs inside the merge's critical section, so a real + # away-record change attempted from here is the TOCTOU itself: whatever + # happens to it happens between the authority read and the merge. + if [ -x "${FM_TEST_AWAY_MUTATE_AT_MERGE:-}" ]; then + away_rc=0 + "$FM_TEST_AWAY_MUTATE_AT_MERGE" > "$FM_TEST_AWAY_MUTATE_OUT" 2>&1 || away_rc=$? + printf '%s\n' "$away_rc" > "$FM_TEST_AWAY_MUTATE_RC" + "$FM_TEST_ROOT/bin/fm-afk-contract.sh" grants \ + > "$FM_TEST_AWAY_GRANTS_AT_MERGE" 2>/dev/null \ + || printf 'no-live-record\n' > "$FM_TEST_AWAY_GRANTS_AT_MERGE" + fi if [ -n "${FM_TEST_GH_MERGE_OUTPUT:-}" ]; then printf '%s\n' "$FM_TEST_GH_MERGE_OUTPUT" else @@ -278,6 +289,7 @@ write_mr_json() { local file=$1 kv key value local state=opened detail=mergeable conflicts=false discussions=true local head=$MR_HEAD pipeline_sha=$MR_HEAD pipeline_status=success pipeline=present + local merge_when_pipeline_succeeds=false merge_after=null shift for kv in "$@"; do key=${kv%%=*} @@ -291,6 +303,8 @@ write_mr_json() { pipeline_sha) pipeline_sha=$value ;; pipeline_status) pipeline_status=$value ;; pipeline) pipeline=$value ;; + merge_when_pipeline_succeeds) merge_when_pipeline_succeeds=$value ;; + merge_after) merge_after=$value ;; *) fail "write_mr_json: unknown field '$key'" ;; esac done @@ -299,8 +313,10 @@ write_mr_json() { fi printf '{"iid":7,"state":"%s","detailed_merge_status":"%s","has_conflicts":%s,' \ "$state" "$detail" "$conflicts" > "$file" - printf '"blocking_discussions_resolved":%s,"sha":"%s","head_pipeline":%s}\n' \ + printf '"blocking_discussions_resolved":%s,"sha":"%s","head_pipeline":%s,' \ "$discussions" "$head" "$pipeline" >> "$file" + printf '"merge_when_pipeline_succeeds":%s,"merge_after":%s}\n' \ + "$merge_when_pipeline_succeeds" "$merge_after" >> "$file" } # make_gitlab_case [= ...]: a case dir with both forge @@ -367,6 +383,11 @@ run_pr_merge() { FM_TEST_GH_RULES_FAIL="$case_dir/github-rules-fail" \ FM_TEST_META_AT_MERGE="$case_dir/meta-at-merge" \ FM_TEST_AWAY_RECORD_AFTER_VIEW="$case_dir/away-record-after-view" \ + FM_TEST_ROOT="$ROOT" \ + FM_TEST_AWAY_MUTATE_AT_MERGE="${FM_TEST_AWAY_MUTATE_AT_MERGE:-}" \ + FM_TEST_AWAY_MUTATE_OUT="$case_dir/away-mutate-output" \ + FM_TEST_AWAY_MUTATE_RC="$case_dir/away-mutate-rc" \ + FM_TEST_AWAY_GRANTS_AT_MERGE="$case_dir/away-grants-at-merge" \ FM_TEST_REAL_MV="$REAL_MV" \ FM_TEST_GLAB_LOG="$case_dir/glab.log" \ FM_TEST_GLAB_JSON="$case_dir/mr.json" \ @@ -2686,6 +2707,79 @@ test_away_grant_and_yolo_and_hold_for_return() { pass "away merges require yolo or a grant, and --attended-override does not skip that" } +test_away_posture_refuses_asynchronous_merge_paths() { + local case_dir rc url head merge_line + head=abababababababababababababababababababab + url=https://github.com/example/repo/pull/89 + + case_dir=$(make_case away-auto-refused) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + write_away_record "$case_dir" --grant task-x1 + set +e + run_pr_merge "$case_dir" task-x1 "$url" --attended-override -- --auto --merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 2 "$rc" "away-auto-refused: auto-merge must be attended-only" + assert_grep '--auto is attended-only' "$case_dir/stderr" \ + "away-auto-refused: refusal did not name the asynchronous flag" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "away-auto-refused: gh pr merge ran for an away auto-merge request" + + case_dir=$(make_case away-queue-refused) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" "$head" + printf 'merge_method=MERGE\n' > "$case_dir/github-rules" + write_away_record "$case_dir" --grant task-x1 + set +e + run_pr_merge "$case_dir" task-x1 "$url" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 2 "$rc" "away-queue-refused: a required merge queue must refuse before submission" + assert_grep 'merge-queue state does not prove an immediate merge' "$case_dir/stderr" \ + "away-queue-refused: refusal did not explain the away restriction" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "away-queue-refused: gh received a merge that could enter its queue" + + case_dir=$(make_gitlab_case away-gitlab-auto) + write_away_record "$case_dir" --grant task-x1 + set +e + run_pr_merge "$case_dir" task-x1 "$MR_URL" --attended-override -- --auto-merge \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 2 "$rc" "away-gitlab-auto: GitLab auto-merge must refuse" + assert_grep 'GitLab auto-merge is attended-only' "$case_dir/stderr" \ + "away-gitlab-auto: refusal did not name auto-merge" + [ -z "$(glab_merge_line "$case_dir/glab.log")" ] \ + || fail "away-gitlab-auto: glab received an asynchronous merge" + + case_dir=$(make_gitlab_case away-gitlab-configured merge_when_pipeline_succeeds=true) + write_away_record "$case_dir" --grant task-x1 + set +e + run_pr_merge "$case_dir" task-x1 "$MR_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + expect_code 2 "$rc" "away-gitlab-configured: configured auto-merge must refuse" + [ -z "$(glab_merge_line "$case_dir/glab.log")" ] \ + || fail "away-gitlab-configured: glab received a configured asynchronous merge" + + case_dir=$(make_gitlab_case away-gitlab-sync) + write_away_record "$case_dir" --grant task-x1 + run_pr_merge "$case_dir" task-x1 "$MR_URL" \ + > "$case_dir/stdout" 2> "$case_dir/stderr" \ + || fail "away-gitlab-sync: an immediate granted merge should succeed" + merge_line=$(glab_merge_line "$case_dir/glab.log") + case "$merge_line" in + *" --auto-merge=false") ;; + *) fail "away-gitlab-sync: the final glab flag did not force an immediate merge: '$merge_line'" ;; + esac + pass "away posture permits immediate merges but refuses every asynchronous path" +} + test_away_grant_does_not_bypass_red_or_identity() { local case_dir rc head head=adadadadadadadadadadadadadadadadadadadad @@ -2739,6 +2833,164 @@ test_unreadable_away_record_refuses_merge() { pass "an unreadable away-posture record refuses the merge instead of skipping the grant" } +# The race this closes: the away record is read for merge authority and the +# forge is called afterwards, so an archive (the captain's return) or a grant +# revocation landing in between would merge on authority that no longer holds. +# away_change_script writes the change the gh mock attempts from inside the +# forge call, which IS that window. Its body drives the real away-record +# commands /afk and the return use, never a file edit, and takes a one-second +# lock bound so a contended case refuses quickly instead of waiting. +away_change_script() { # ; script body on stdin + local case_dir=$1 name=$2 path + path="$case_dir/$name" + { + printf '#!/usr/bin/env bash\n' + printf 'set -eu\n' + printf 'export FM_TEST_AFK_CONTRACT_LOCK_TIMEOUT=1\n' + printf 'CONTRACT="%s/bin/fm-afk-contract.sh"\n' "$ROOT" + cat + } > "$path" + chmod +x "$path" + printf '%s\n' "$path" +} + +# Two away-record changes, each attempted from inside the merge's critical +# section: the archive a captain return performs, and the replacement that +# revokes a grant. Neither may land there, and the merge must still complete on +# the authority it read. +test_away_record_cannot_change_between_the_authority_read_and_the_merge() { + local case_dir rc mutate + case_dir=$(make_case away-archive-at-merge) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b1b + write_away_record "$case_dir" --grant task-x1 + mutate=$(away_change_script "$case_dir" archive-at-merge <<'SH' +"$CONTRACT" archive +SH + ) + + export FM_TEST_AWAY_MUTATE_AT_MERGE="$mutate" + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/71 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + unset FM_TEST_AWAY_MUTATE_AT_MERGE + + expect_code 0 "$rc" "away-archive-at-merge: the granted green merge should still land" + [ -s "$case_dir/away-mutate-rc" ] \ + || fail "away-archive-at-merge: the archive was never attempted inside the merge" + [ "$(cat "$case_dir/away-mutate-rc")" != 0 ] \ + || fail "away-archive-at-merge: the archive landed inside the merge's critical section" + assert_grep 'locked by live process' "$case_dir/away-mutate-output" \ + "away-archive-at-merge: the refused archive did not name the live holder" + assert_equals task-x1 "$(cat "$case_dir/away-grants-at-merge" 2>/dev/null || true)" \ + "away-archive-at-merge: the grant this merge read was not still standing at the forge call" + assert_grep "merge landed: task-x1 https://github.com/example/repo/pull/71 away-grant" \ + "$case_dir/state/.wake-queue" \ + "away-archive-at-merge: the landed merge was not recorded under the grant it read" + # The lock goes with the merge rather than leaking: the captain's return + # archives the record on its first try once the merge is done. + FM_HOME="$case_dir/home" FM_STATE_OVERRIDE="$case_dir/state" \ + "$ROOT/bin/fm-afk-contract.sh" archive >/dev/null \ + || fail "away-archive-at-merge: the record stayed locked after the merge" + + case_dir=$(make_case away-revoke-at-merge) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c2c + write_away_record "$case_dir" --grant task-x1 + mutate=$(away_change_script "$case_dir" revoke-at-merge <<'SH' +"$CONTRACT" propose --grant task-other +"$CONTRACT" confirm +SH + ) + export FM_TEST_AWAY_MUTATE_AT_MERGE="$mutate" + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/72 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + unset FM_TEST_AWAY_MUTATE_AT_MERGE + + expect_code 0 "$rc" "away-revoke-at-merge: the granted green merge should still land" + [ "$(cat "$case_dir/away-mutate-rc" 2>/dev/null || true)" != 0 ] \ + || fail "away-revoke-at-merge: the replacement landed inside the critical section" + assert_equals task-x1 "$(cat "$case_dir/away-grants-at-merge" 2>/dev/null || true)" \ + "away-revoke-at-merge: the grant was revoked inside the merge's critical section" + pass "no away-record archive or grant revocation lands between the authority read and the merge" +} + +# The same serialization from the other side. A revocation that wins the race +# lands BEFORE the in-lock authority read, and the merge then refuses: the lock +# decides an order, it never lets a stale grant through. +test_a_grant_revoked_before_the_merge_refuses_it() { + local case_dir rc + case_dir=$(make_case away-revoked-before-merge) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 3d3d3d3d3d3d3d3d3d3d3d3d3d3d3d3d3d3d3d3d + write_away_record "$case_dir" + mv "$case_dir/state/.afk-contract" "$case_dir/away-record-after-view" + write_away_record "$case_dir" --grant task-x1 + + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/73 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + + expect_code 1 "$rc" "away-revoked-before-merge: a revoked grant must refuse" + assert_grep 'held for the captain return' "$case_dir/stderr" \ + "away-revoked-before-merge: refusal did not name hold-for-return" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "away-revoked-before-merge: gh pr merge ran on a revoked grant" + pass "a grant revoked before the merge's own authority read refuses the merge" +} + +# Fail closed. The lock is what makes the authority read and the merge one +# action, so a merge that cannot take it has no locked window to merge in and +# refuses - including on this attended case, where the record is absent and +# there is no grant to check at all. +test_merge_refuses_when_the_away_record_cannot_be_locked() { + local case_dir rc holder_pid i lock + case_dir=$(make_case away-lock-unavailable) + mkdir -p "$case_dir/wt" + add_gh_mocks "$case_dir" 4e4e4e4e4e4e4e4e4e4e4e4e4e4e4e4e4e4e4e4e + lock="$case_dir/state/.afk-contract.lock" + + FM_STATE_OVERRIDE="$case_dir/state" bash -c ' + . "$1" + fm_lock_acquire_wait "$2" || exit 10 + printf "ready\n" > "$3" + while [ ! -e "$4" ]; do sleep 0.05; done + fm_lock_release "$2" + ' _ "$ROOT/bin/fm-wake-lib.sh" "$lock" "$case_dir/holder.ready" "$case_dir/release-holder" & + holder_pid=$! + i=0 + while [ "$i" -lt 100 ] && [ ! -s "$case_dir/holder.ready" ]; do + sleep 0.05 + i=$((i + 1)) + done + [ -s "$case_dir/holder.ready" ] \ + || { kill "$holder_pid" 2>/dev/null || true; fail "away-lock-unavailable: the fixture never took the record lock"; } + + export FM_TEST_AFK_CONTRACT_LOCK_TIMEOUT=1 + set +e + run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/74 \ + > "$case_dir/stdout" 2> "$case_dir/stderr" + rc=$? + set -e + unset FM_TEST_AFK_CONTRACT_LOCK_TIMEOUT + : > "$case_dir/release-holder" + wait "$holder_pid" || fail "away-lock-unavailable: the fixture holder did not release cleanly" + + expect_code 1 "$rc" "away-lock-unavailable: an unlockable away record must refuse the merge" + assert_grep 'could not be locked for the merge' "$case_dir/stderr" \ + "away-lock-unavailable: refusal did not name the lock it could not take" + assert_no_grep 'pr merge' "$case_dir/gh.log" \ + "away-lock-unavailable: gh pr merge ran without the away-record lock" + pass "a merge that cannot lock the away record refuses instead of merging unlocked" +} + test_allow_red_refused_on_gitlab() { local case_dir rc case_dir=$(make_gitlab_case gitlab-allow-red) @@ -2786,6 +3038,10 @@ test_allow_red_still_waives_only_the_current_failure test_allow_red_is_refused_while_away test_allow_red_requires_one_separate_name test_away_grant_and_yolo_and_hold_for_return +test_away_posture_refuses_asynchronous_merge_paths test_away_grant_does_not_bypass_red_or_identity test_unreadable_away_record_refuses_merge +test_away_record_cannot_change_between_the_authority_read_and_the_merge +test_a_grant_revoked_before_the_merge_refuses_it +test_merge_refuses_when_the_away_record_cannot_be_locked test_allow_red_refused_on_gitlab From 35761484e9f0a692ebec86532ff4dd252b224cf9 Mon Sep 17 00:00:00 2001 From: AnPod Date: Sun, 13 Sep 2026 00:50:23 +0200 Subject: [PATCH 008/254] feat(bin): add Antigravity CLI (agy) as third worker/scout adapter (#4200) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(agy): verify Antigravity CLI as third worker/scout adapter Detection by anchored ancestry in fm-harness.sh (no marker of its own); bootstrap harness and effort validation; launch template with model and effort mapping plus reachable-catalog model validation; rendered-tail busy fallback in fm-busy-lib.sh with delivery footer in fm-composer-lib.sh; control mechanics with crewmate/scout-only refusal; tmux liveness naming; router entry with concise adapter reference; dated verification record; portable regression plus opt-in live drift guard. Verified live on agy 1.2.0: supervised spawn, durable steering, same-copy relaunch, and exit, with Herdr-native busy agreement. * no-mistakes(review): bound agy model probe, gate trust dialog, narrow busy signature * no-mistakes(review): pre-register agy workspace trust, make readiness gate strict * no-mistakes(review): Close Orca terminal on gate failure; isolate live-guard HOME; tighten agy matching * no-mistakes(document): Document agy adapter in stale harness enumerations * no-mistakes(review): Clamp non-positive FM_AGY_MODELS_TIMEOUT to the default bound * no-mistakes(document): Fix stale test-shard snapshots after agy lane additions * no-mistakes(ci): Fixed ci-3 (tests/fm-agy-harness.test.sh:519). Root cause: the agy spawn fixture's default base PATH (/usr/bin:/bin:/usr/sbin:/sbin) omits node's directory, but the spawn drives the real bin/fm-agy-trust.sh (which hard-requires node to record trust) and the fixture's fake tmux trust lookup (node -e) under that PATH. On the ubuntu-latest CI runner node lives in the toolcache (/usr/local/bin), so trust pre-registration failed on portable serial 2; on typical Arch hosts node is in /usr/bin, masking the defect. Fix (smallest, following the existing tests/fm-kimi-harness.test.sh precedent of carrying the interpreter's resolved directory): resolve node from the invoking environment (failing the test with 'test needs node' if absent, as kimi does for python3) and prepend its directory to the fixture's default base PATH; the FM_TEST_BASE_PATH override contract is untouched. Verified locally: (1) pre-fix reproduction with a CI-shaped base PATH (system bins minus node) produced exactly the reported failure — 'node is required to record workspace trust and was not found on PATH' plus the fake tmux 'node: command not found'; (2) post-fix, all 29 tests in the file pass both with node available only via a leading non-standard dir in the base PATH (CI's shape) and with the default base PATH on this host. bash -n clean; ShellCheck is not installed in this worktree (previously recorded as environmental) * no-mistakes(test): Give agy typed sends a longer submit-confirm budget * no-mistakes(document): Document agy send budget, trust gate, and control coverage * no-mistakes(document): Document agy busy fallback inventory and send-timing evidence --- .agents/skills/harness-adapters/SKILL.md | 7 +- .../references/common/control-and-recovery.md | 1 + .../references/harness/agy.md | 55 ++ AGENTS.md | 2 +- CONTRIBUTING.md | 2 +- bin/fm-agent-process-lib.sh | 5 + bin/fm-agy-trust.sh | 185 ++++ bin/fm-bootstrap.sh | 3 +- bin/fm-busy-lib.sh | 55 +- bin/fm-composer-lib.sh | 17 +- bin/fm-control-lib.sh | 26 +- bin/fm-harness.sh | 11 +- bin/fm-send.sh | 25 +- bin/fm-spawn.sh | 203 +++- bin/fm-test-run.sh | 6 +- docs/agent-control.md | 6 +- docs/architecture.md | 4 +- docs/configuration.md | 3 +- docs/documentation-audiences.json | 8 + docs/fm-test-portable-shards.md | 18 +- docs/tmux-backend.md | 3 +- docs/trace-context.md | 2 +- docs/verification/agy.md | 170 ++++ tests/fm-agy-harness.test.sh | 905 ++++++++++++++++++ tests/fm-agy-signals-live-e2e.test.sh | 193 ++++ tests/fm-bootstrap.test.sh | 4 + tests/fm-send-agy-confirm.test.sh | 165 ++++ 27 files changed, 2012 insertions(+), 72 deletions(-) create mode 100644 .agents/skills/harness-adapters/references/harness/agy.md create mode 100755 bin/fm-agy-trust.sh create mode 100644 docs/verification/agy.md create mode 100755 tests/fm-agy-harness.test.sh create mode 100755 tests/fm-agy-signals-live-e2e.test.sh create mode 100755 tests/fm-send-agy-confirm.test.sh diff --git a/.agents/skills/harness-adapters/SKILL.md b/.agents/skills/harness-adapters/SKILL.md index be447a41d7a..0c3a0353411 100644 --- a/.agents/skills/harness-adapters/SKILL.md +++ b/.agents/skills/harness-adapters/SKILL.md @@ -3,7 +3,7 @@ name: harness-adapters description: >- Agent-only reference for firstmate harness operations. Use before spawning or recovering a crewmate or secondmate, handling a trust dialog, sending a harness-specific skill invocation, interrupting or exiting an agent, resuming an exited agent, or verifying a new harness adapter. - Contains verified facts for claude, codex, opencode, pi, pi-signed, grok, kimi, cursor, gemini, muse, rovo, and omp. + Contains verified facts for claude, codex, opencode, pi, pi-signed, grok, kimi, cursor, gemini, muse, rovo, omp, and agy. user-invocable: false metadata: internal: true @@ -35,7 +35,7 @@ For recovery and control, use the exact `harness=` in `state/.meta`; never i Deliver lifecycle actions only through `../../../bin/fm-control.sh interrupt|exit|relaunch`. Never type an interrupt key or exit command through `fm-send`, where routing-marked lifecycle text becomes chat. Trust handling is complete only when inspection proves the target started processing its instructions; delivery success alone is not proof. -Muse and Gemini are verified only for crewmate and scout work, never a secondmate or primary. +Muse, Gemini, and AGY are verified only for crewmate and scout work, never a secondmate or primary. ## Detection @@ -93,7 +93,8 @@ A new tool remains undispatchable until the `verify` plan, its harness entry, ev "gemini": "references/harness/gemini.md", "muse": "references/harness/muse.md", "rovo": "references/harness/rovo.md", - "omp": "references/harness/omp.md" + "omp": "references/harness/omp.md", + "agy": "references/harness/agy.md" } } ``` diff --git a/.agents/skills/harness-adapters/references/common/control-and-recovery.md b/.agents/skills/harness-adapters/references/common/control-and-recovery.md index 78115e47174..4223b63b895 100644 --- a/.agents/skills/harness-adapters/references/common/control-and-recovery.md +++ b/.agents/skills/harness-adapters/references/common/control-and-recovery.md @@ -18,6 +18,7 @@ No observed dialog proves only that launch. Each supported harness handles its folder-trust gate differently, and the tool reference owns the detail. For Claude, load `references/harness/claude.md`; its workspace-trust section owns the non-key-answerable gate and spawn-time pre-registration for every spawn kind. +agy gates every fresh worktree too; the spawn pre-registers it in agy's own store the same way, and a strict post-launch gate answers any dialog that still renders before the spawn reports success. Cursor suppresses its dialog with launch-time `--trust`, and Muse suppresses its own with `--yolo`. Grok dodges its gate instead of granting trust, because its project picker appears only outside a project and the spawn starts in the isolated git root. Pi gates the fresh-worktree case too, but unlike Claude its dialog is answered with Enter, and `references/harness/pi.md` owns that recipe and where the decision persists. diff --git a/.agents/skills/harness-adapters/references/harness/agy.md b/.agents/skills/harness-adapters/references/harness/agy.md new file mode 100644 index 00000000000..0f38ca16ba6 --- /dev/null +++ b/.agents/skills/harness-adapters/references/harness/agy.md @@ -0,0 +1,55 @@ +# Antigravity CLI + +Antigravity's `agy` TUI, verified end to end on 2026-09-10 with agy 1.2.0 on Linux through the Herdr backend. +Verified as a CREWMATE and SCOUT adapter only; `../../../../../bin/fm-spawn.sh` refuses a secondmate launch on it because `../../../../../docs/supervision-protocols/` carries no agy wake protocol. +`../../../../../docs/verification/agy.md` owns how every fact below was established and what is still unproven. + +## Operating facts + +| Fact | Value | +|---|---| +| Binary | Absolute `agy` from `PATH`, refused if absent; a Go-compiled single binary, so the live process name is exactly `agy` with `argv[0]=agy`. | +| Launch | `agy --prompt-interactive "" --model --effort --dangerously-skip-permissions`, with the resolved absolute binary; the brief auto-submits with no extra Enter. The spawn pre-registers the worktree in agy's trust store first, then waits for a busy turn (answering the folder-trust dialog if it renders anyway) before reporting success. | +| Busy state | No hook or plugin writer, so nothing is armed and no record is seeded; on Herdr the native `working` status classifies busy, and everywhere else the `agy-regex` rendered-tail fallback in `../../../../../bin/fm-busy-lib.sh` does. | +| Rendered tail | Busy status row carries `esc to cancel` on the left; the idle row shows `? for shortcuts` instead. The `Generating...` word beside the braille spinner is free-floating output and is not a signal. | +| Turn end | No turn-end hook or notification touch exists; completion arrives through the worker status protocol and, on Herdr, the native return to `idle`. | +| Exit | `/quit`, one Enter; the process exits. | +| Interrupt | Single `Escape`, which prints the Interrupted row and leaves an idle composer with no repollution, so no clear key follows. | +| Skill | No verified slash-skill form; use natural language. | +| Autonomy | `--dangerously-skip-permissions` auto-approves tool calls for the run. | +| Marker | None; a live TUI carries no `AGY_*` or `ANTIGRAVITY_*` variable. | +| Resume | `--continue` and `--conversation` exist but carry no verified pane-resume contract; use deterministic relaunch. | +| Model | `--model ` with the bare catalog id from `agy models` (for example `gemini-3.8-flash-high`); `bin/fm-spawn.sh` refuses a requested id a reachable listing omits. The listing is a remote fetch, so the probe runs stdin-detached under the shared hard bound and an unreachable or hung listing launches unvalidated with a notice. | +| Effort | `--effort low\|medium\|high`; `xhigh` and `max` stay in task metadata under the record-and-omit contract. | +| Composer | Borderless bare `>` row, which the shared classifier reads as `unknown` under the dead-shell rule, never `empty`; steering confirms delivery through native agent-state and the delivery footer instead, the cursor precedent. | + +## Trust, and where the decision persists + +Every task worktree is a path agy has never seen, so an unregistered launch stops on `Do you trust the contents of this project?` with the safe choice `Yes, I trust this folder` preselected, and an unanswered dialog sends the turn into agy's scratch directory instead of the worktree. +There is no launch flag that suppresses the dialog, but agy honours a `trustedWorkspaces` entry in the captain's own `~/.gemini/antigravity-cli/settings.json` written ahead of launch (verified live), so `../../../../../bin/fm-spawn.sh` pre-registers the worktree through `../../../../../bin/fm-agy-trust.sh` before launch, the claude shape: the helper refuses anything but a linked worktree of the spawning project, records both the logical pane path and its resolved form because agy compares the logical cwd, and preserves every other key in the store. +The post-launch readiness gate is the backstop: it answers a dialog that renders anyway with a single Enter, then requires a busy verdict (Herdr's native `working` status or the pinned `esc to cancel` row) before the spawn reports success, and on a path that was not pre-registered it never counts a busy verdict as ready until the dialog has been answered, because Herdr's native verdict can precede the dialog. +A pane whose brief cannot be confirmed to run in the worktree fails the spawn, records the failure in the task status, and closes the endpoint. +Never steer into a pane still showing the dialog; a spawn that reported success has already cleared it. + +## Credential precondition + +A verified agy worker ran under a signed-in Google account with no key export and no dialog. +The unauthenticated failure mode was not observed, so treat any auth prompt or refusal as a credential blocker under `../../../../../AGENTS.md` section 9, fix the environment, and retire the endpoint rather than typing into it. + +## Detection + +Detected by ancestry alone: `../../../../../bin/fm-harness.sh` matches the anchored process name `agy`, never `*agy*`. +No environment marker is promoted: `AGENT=1` observed on a live TUI is an inherited launcher value, not an agy identity, and agy does not clear an inherited `CLAUDECODE`, so the spawn clears foreign markers at the launch boundary and the ancestry arm decides. +agy is deliberately absent from the session-lock name vocabulary in `../../../../../bin/fm-session-lock-lib.sh`, where muse, gemini, and rovo are also absent: a crewmate-only adapter must never own a home session lock. + +## Worker busy state and turn end + +`../../../../../bin/fm-spawn.sh` arms no busy generation for agy and writes no sidecar, exactly because no writer could ever clear a seeded record. +`fm_busy_agy_tail_busy` matches the pinned `esc to cancel` status row alone, hardcoded with no environment override, and `fm_busy_classify` reports `unknown agy-regex` rather than idle when it is absent, because a long turn can scroll the marker out of the captured tail. +Teardown removes nothing agy-specific because the spawn leaves nothing behind. + +## Primary integration + +Unsupported and unverified. +`../../../../../docs/supervision-protocols/` carries no agy protocol, no turn-end guard adapter exists for it, and this adapter verified only the crewmate-side launch, busy state, interrupt, and exit. +`references/common/primary-hooks.md`'s unsupported-boundary rule applies: never invent a wake protocol from a similar TUI. diff --git a/AGENTS.md b/AGENTS.md index 135831d62f5..7d58297bb07 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -209,7 +209,7 @@ A silent bootstrap section needs no action; for any printed actionable diagnosti ## 4. Harness and runtime dispatch Load `harness-adapters` before every spawn or recovery and before trust handling, skill invocation, interrupt, exit, resume, or adapter verification. -The verified harnesses are `claude`, `codex`, `opencode`, `pi`, `pi-signed`, `grok`, `kimi`, `cursor`, and `omp`, plus `muse`, `gemini`, and `rovo` for crewmates and scouts only; never dispatch on an unverified adapter. +The verified harnesses are `claude`, `codex`, `opencode`, `pi`, `pi-signed`, `grok`, `kimi`, `cursor`, and `omp`, plus `muse`, `gemini`, `rovo`, and `agy` for crewmates and scouts only; never dispatch on an unverified adapter. If static `config/crew-harness` or `config/secondmate-harness` names an unverified adapter, report it and fall back only to a verified adapter rather than launching it. `docs/configuration.md` owns dispatch-profile and runtime-backend schemas, `bin/fm-harness.sh` owns static resolution, and `bin/fm-spawn.sh` owns launch flags and fail-closed validation. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87317bd5c45..de4cc759a35 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -52,7 +52,7 @@ See the [no-mistakes quick start](https://kunchenguid.github.io/no-mistakes/star It pins one exact shellcheck version and one exact actionlint version and refuses to run under any other. Print the shellcheck pin with `bin/fm-lint.sh --required-version` and the actionlint pin with `bin/fm-lint-workflows.sh --required-version`. Use `bin/fm-install-shellcheck.sh` and `bin/fm-install-actionlint.sh` to install those exact builds locally; each installer's header owns its destination usage and supported platforms. -- Harness-adapter ownership spans detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, spawn-time Claude workspace-trust pre-registration in `bin/fm-claude-trust.sh`, semantic busy sources and trust gates in `bin/fm-busy-lib.sh`, delivery-only rendered guards in `bin/fm-composer-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in the skill tree rooted at `.agents/skills/harness-adapters/SKILL.md`; the `firstmate-coding-guidelines` skill owns the validation policy for checks that depend on those harnesses. +- Harness-adapter ownership spans detection in `bin/fm-harness.sh`, launch and hook mechanics in `bin/fm-spawn.sh`, spawn-time workspace-trust pre-registration in `bin/fm-claude-trust.sh` and `bin/fm-agy-trust.sh`, semantic busy sources and trust gates in `bin/fm-busy-lib.sh`, delivery-only rendered guards in `bin/fm-composer-lib.sh`, cleanup in `bin/fm-teardown.sh`, and facts in the skill tree rooted at `.agents/skills/harness-adapters/SKILL.md`; the `firstmate-coding-guidelines` skill owns the validation policy for checks that depend on those harnesses. - Changes to runtime session backends (`bin/fm-backend.sh`, `bin/backends/`, and the scripts that dispatch through them) keep current setup and limits in the relevant backend guide and active empirical evidence in [`docs/verification/runtime-backends.md`](docs/verification/runtime-backends.md). - [`docs/documentation-audiences.md`](docs/documentation-audiences.md) and its machine-consumed inventory own prose classification; run `bin/fm-doc-audience-check.sh` after documentation changes. - In Markdown, put each full sentence on its own line. diff --git a/bin/fm-agent-process-lib.sh b/bin/fm-agent-process-lib.sh index 5943f1b2749..dcf4b59ff4e 100644 --- a/bin/fm-agent-process-lib.sh +++ b/bin/fm-agent-process-lib.sh @@ -41,6 +41,11 @@ fm_agent_process_classify_name() { # [argv0] -> agent|shell|other # name is the bare word `omp` (verified, omp 18.1.11) and a glob would claim # unrelated commands such as ompd or comp. *claude*|*codex*|*opencode*|*grok*|*kimi*|*rovo*|pi|pi-signed|pi-launcher|Pi|omp) printf 'agent' ;; + # agy (Antigravity CLI) is anchored for the same reason as muse and omp: its + # live process name is the bare word `agy` (verified, agy 1.2.0: a Go-compiled + # single binary, comm=agy with argv[0]=agy), and a glob would claim + # unrelated commands containing that fragment. + agy) printf 'agent' ;; zsh|bash|sh|dash|ash|ksh|mksh|tcsh|csh|fish) printf 'shell' ;; *) if fm_harness_path_name "$path" >/dev/null || fm_harness_path_name "$argv0" >/dev/null; then diff --git a/bin/fm-agy-trust.sh b/bin/fm-agy-trust.sh new file mode 100755 index 00000000000..627d892da2f --- /dev/null +++ b/bin/fm-agy-trust.sh @@ -0,0 +1,185 @@ +#!/usr/bin/env bash +# Pre-register Antigravity CLI's workspace trust for the isolated task worktree +# a ship/scout spawn is about to launch an agy crewmate into, so the worker +# reaches its brief in the worktree instead of parking on the folder-trust +# dialog and running its turn in agy's own scratch directory. +# +# Usage: fm-agy-trust.sh +# the isolated task worktree this spawn launches into +# the primary checkout that worktree belongs to +# Prints one line naming what it registered; refuses loudly on anything else. +# +# WHY THIS EXISTS. agy 1.2.0 gates a folder it has never seen behind +# "Do you trust the contents of this project?" and no launch flag suppresses +# it (`agy --help` lists none). Answering appends the folder to the +# `trustedWorkspaces` array of ${HOME}/.gemini/antigravity-cli/settings.json, +# and agy honours an entry written there ahead of launch: verified live under a +# throwaway HOME, a pre-registered folder launched straight into its turn while +# an unregistered sibling parked on the dialog (docs/verification/agy.md). agy +# compares the pane's LOGICAL working directory, not its resolved path (a +# symlinked cwd with only the real path registered still parked), so both the +# logical path and its resolved form are recorded when they differ. +# +# bin/fm-spawn.sh keeps a post-launch gate as the backstop: it answers the +# dialog if one renders anyway and never counts a busy turn as ready on a path +# that was neither pre-registered here nor answered there. +# +# THE SCOPE TEST IS THE SAFETY PROPERTY and mirrors bin/fm-claude-trust.sh: +# must be a LINKED git worktree - its own git dir, sharing +# 's common dir - whose top level is exactly the resolved argument. A +# primary checkout, a worktree of an unrelated repo, a subdirectory of a +# worktree, a plain directory, and a home directory are each refused with a +# non-zero exit, never a warning and never a silent skip. Only the launching +# user's own store is written, it must be a regular file this uid owns, every +# unrelated key and entry is preserved, and the replacement is atomic. +set -u +unset CDPATH \ + GIT_DIR GIT_WORK_TREE GIT_COMMON_DIR GIT_OBJECT_DIRECTORY GIT_INDEX_FILE \ + GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_CEILING_DIRECTORIES GIT_NAMESPACE \ + GIT_DISCOVERY_ACROSS_FILESYSTEM GIT_CONFIG GIT_CONFIG_GLOBAL \ + GIT_CONFIG_SYSTEM GIT_CONFIG_NOSYSTEM GIT_CONFIG_COUNT + +[ "$#" -eq 2 ] || { echo "usage: fm-agy-trust.sh " >&2; exit 2; } +WT_ARG=$1 +PROJ_ARG=$2 + +refuse() { echo "error: refusing to pre-register agy trust: $1" >&2; exit 1; } + +real_dir() { (cd -P -- "$1" 2>/dev/null && pwd -P); } +logical_dir() { (cd -- "$1" 2>/dev/null && pwd -L); } +real_file() { node -e 'process.stdout.write(require("node:fs").realpathSync(process.argv[1]))' "$1" 2>/dev/null; } + +common_dir_of() { + local dir=$1 common + common=$(git -C "$dir" rev-parse --git-common-dir 2>/dev/null) || return 1 + (cd -P -- "$dir" && real_dir "$common") +} + +WT_REAL=$(real_dir "$WT_ARG") || true +[ -n "$WT_REAL" ] || refuse "worktree '$WT_ARG' is not an accessible directory" +WT_LOGICAL=$(logical_dir "$WT_ARG") || true +[ -n "$WT_LOGICAL" ] || WT_LOGICAL=$WT_REAL +PROJ_REAL=$(real_dir "$PROJ_ARG") || true +[ -n "$PROJ_REAL" ] || refuse "project '$PROJ_ARG' is not an accessible directory" + +[ -n "${HOME:-}" ] || refuse "HOME is not set, so agy's settings store cannot be located" +HOME_REAL=$(real_dir "$HOME") || true +[ -n "$HOME_REAL" ] || refuse "HOME '$HOME' is not an accessible directory" +[ "$WT_REAL" != "$HOME_REAL" ] || refuse "'$WT_REAL' is the home directory, not a task worktree" + +WT_TOP=$(git -C "$WT_REAL" rev-parse --show-toplevel 2>/dev/null) || true +[ -n "$WT_TOP" ] || refuse "'$WT_REAL' is not inside a git repository" +WT_TOP_REAL=$(real_dir "$WT_TOP") || true +[ "$WT_TOP_REAL" = "$WT_REAL" ] || refuse "'$WT_REAL' is not a worktree root (its root is '${WT_TOP_REAL:-unresolvable}')" + +WT_GIT_DIR=$(git -C "$WT_REAL" rev-parse --absolute-git-dir 2>/dev/null) || true +[ -n "$WT_GIT_DIR" ] || refuse "'$WT_REAL' has no resolvable git directory" +WT_GIT_DIR=$(real_dir "$WT_GIT_DIR") || true +[ -n "$WT_GIT_DIR" ] || refuse "'$WT_REAL' has an unresolvable git directory" +WT_COMMON=$(common_dir_of "$WT_REAL") || true +[ -n "$WT_COMMON" ] || refuse "'$WT_REAL' has no resolvable git common directory" +[ "$WT_GIT_DIR" != "$WT_COMMON" ] || refuse "'$WT_REAL' is a primary checkout, not an isolated worktree" + +PROJ_COMMON=$(common_dir_of "$PROJ_REAL") || true +[ -n "$PROJ_COMMON" ] || refuse "project '$PROJ_REAL' is not inside a git repository" +[ "$WT_COMMON" = "$PROJ_COMMON" ] || refuse "'$WT_REAL' is not a worktree of project '$PROJ_REAL'" + +command -v node >/dev/null 2>&1 || refuse "node is required to record workspace trust and was not found on PATH" + +STORE_DIR="$HOME_REAL/.gemini/antigravity-cli" +mkdir -p "$STORE_DIR" 2>/dev/null || true +STORE_DIR_REAL=$(real_dir "$STORE_DIR") || true +[ -n "$STORE_DIR_REAL" ] || refuse "agy settings directory '$STORE_DIR' does not exist and could not be created" +STORE="$STORE_DIR_REAL/settings.json" +if [ -L "$STORE" ]; then + STORE_REAL=$(real_file "$STORE") || true + [ -n "$STORE_REAL" ] || refuse "'$STORE' is a symlink whose target cannot be resolved" + STORE=$STORE_REAL +fi +if [ -e "$STORE" ]; then + [ -f "$STORE" ] || refuse "'$STORE' is not a regular file" + [ -O "$STORE" ] || refuse "'$STORE' is not owned by this user" + [ -w "$STORE" ] || refuse "'$STORE' is not writable" +fi + +# Read-modify-write with a fingerprint check before the rename and a readback +# after it, the bin/fm-claude-trust.sh shape: agy itself rewrites this file +# when a worker answers a dialog or changes a setting, so a store that moved +# under us is retried once and then refused rather than clobbered. +if ! node - "$STORE" "$WT_LOGICAL" "$WT_REAL" <<'NODE' +const fs = require("node:fs"); +const path = require("node:path"); +const crypto = require("node:crypto"); +const [store, ...wanted] = process.argv.slice(2); +const paths = [...new Set(wanted)]; +const readStore = () => { + try { + return fs.readFileSync(store); + } catch (err) { + if (err.code === "ENOENT") return null; + throw err; + } +}; +const fingerprint = (buf) => + buf === null ? "absent" : crypto.createHash("sha256").update(buf).digest("hex"); +const listed = (root) => + Array.isArray(root.trustedWorkspaces) && paths.every((p) => root.trustedWorkspaces.includes(p)); +const attempt = () => { + const original = readStore(); + const before = fingerprint(original); + let root = {}; + if (original !== null) { + const raw = original.toString("utf8"); + if (raw.trim() !== "") { + root = JSON.parse(raw); + if (root === null || typeof root !== "object" || Array.isArray(root)) { + throw new Error(`${store} is not a JSON object`); + } + } + } + if (root.trustedWorkspaces === undefined || root.trustedWorkspaces === null) root.trustedWorkspaces = []; + if (!Array.isArray(root.trustedWorkspaces)) { + throw new Error(`${store} has a non-array "trustedWorkspaces" value`); + } + if (listed(root)) return "recorded"; + for (const p of paths) { + if (!root.trustedWorkspaces.includes(p)) root.trustedWorkspaces.push(p); + } + const unique = `${process.pid}.${crypto.randomBytes(8).toString("hex")}`; + const tmp = path.join(path.dirname(store), `.settings.json.fm-trust.${unique}`); + fs.writeFileSync(tmp, `${JSON.stringify(root, null, 2)}\n`, { mode: 0o600, flag: "wx" }); + let renamed = false; + try { + if (fingerprint(readStore()) !== before) return "moved"; + fs.renameSync(tmp, store); + renamed = true; + } finally { + if (!renamed) fs.rmSync(tmp, { force: true }); + } + return listed(JSON.parse(fs.readFileSync(store, "utf8"))) ? "recorded" : "dropped"; +}; +try { + for (let i = 0; i < 3; i += 1) { + const result = attempt(); + if (result === "recorded") process.exit(0); + if (result === "moved" && i >= 1) { + console.error(`error: ${store} was modified while trust was being recorded; refusing to overwrite it`); + process.exit(1); + } + } +} catch (err) { + console.error(`error: ${err.message}`); + process.exit(1); +} +console.error(`error: ${store} did not retain trust for ${paths.join(", ")} after 3 attempts`); +process.exit(1); +NODE +then + refuse "could not record trust for '$WT_LOGICAL' in '$STORE'" +fi + +if [ "$WT_LOGICAL" != "$WT_REAL" ]; then + echo "trusted: $WT_LOGICAL ($WT_REAL)" +else + echo "trusted: $WT_REAL" +fi diff --git a/bin/fm-bootstrap.sh b/bin/fm-bootstrap.sh index b5e010905cf..98b791e52f7 100755 --- a/bin/fm-bootstrap.sh +++ b/bin/fm-bootstrap.sh @@ -1114,7 +1114,7 @@ crew_dispatch_validate() { return 0 fi err=$(jq -r ' - def verified($h): ["claude","codex","opencode","pi","pi-signed","grok","kimi","cursor","muse","rovo","omp"] | index($h); + def verified($h): ["claude","codex","opencode","pi","pi-signed","grok","kimi","cursor","agy","muse","rovo","omp"] | index($h); def effort_ok($h; $m; $e): if $e == null then true elif ($e | type) != "string" then false @@ -1122,6 +1122,7 @@ crew_dispatch_validate() { elif $h == "claude" then (["low","medium","high","xhigh","max"] | index($e)) elif $h == "codex" then (["low","medium","high","xhigh"] | index($e)) elif $h == "grok" then (["low","medium","high"] | index($e)) + elif $h == "agy" then (["low","medium","high"] | index($e)) elif $h == "pi" or $h == "pi-signed" or $h == "omp" then (["low","medium","high","xhigh","max"] | index($e)) elif $h == "muse" then (["low","medium","high","xhigh","max"] | index($e)) elif $h == "rovo" then (["low","medium","high","max"] | index($e)) diff --git a/bin/fm-busy-lib.sh b/bin/fm-busy-lib.sh index dce79a17941..d8f7a0ee111 100755 --- a/bin/fm-busy-lib.sh +++ b/bin/fm-busy-lib.sh @@ -42,7 +42,7 @@ # fm-interrupt the legacy Claude fm-send --key Escape idle event # fm-recovery a documented recovery reset after relaunch # Classifier-only sources (never written into a record): -# endpoint-gone, herdr-native, grok-regex, rovo-regex, muse-session-log, +# endpoint-gone, herdr-native, grok-regex, rovo-regex, agy-regex, muse-session-log, # cursor-transcript, missing, malformed, gen-mismatch, source-mismatch, # kimi-unverified, codex-unverified, capture-failed, no-target # @@ -53,14 +53,15 @@ # 3. a valid, gen-matching, source-trusted record -> its state and source # 4. no record at all: herdr's native busy verdict is trusted as busy # (generation state is sufficient for busy, not for idle), then the -# muse session-log and cursor transcript pull sources, then the Grok/Rovo -# temporary regex fallbacks classify a grok or rovo task from its -# rendered tail, then unknown missing +# muse session-log and cursor transcript pull sources, then the +# Grok/Rovo/AGY temporary regex fallbacks classify a grok, rovo, or agy +# task from its rendered tail, then unknown missing # 5. malformed, stale, or untrusted records -> unknown, never a fallback -# Grok and Rovo are the ONLY rendered-text classifications that survive the -# redesign, because neither's structured lifecycle was credited-live-verified +# Grok, Rovo, and AGY are the ONLY rendered-text classifications that survive the +# redesign, because none of their structured lifecycles was credited-live-verified # in the approved audit (Rovo's clean ACP stopReason lives outside the TUI -# path firstmate drives, see references/harness/rovo.md); each is scoped to +# path firstmate drives, see references/harness/rovo.md; agy 1.2.0 exposes no +# hook surface at all, see references/harness/agy.md); each is scoped to # its own harness= and can never classify another adapter. The delivery # guards in bin/fm-composer-lib.sh match rendered footers for submit # acknowledgement and away-mode supervisor injection only; neither is a @@ -851,12 +852,27 @@ fm_busy_rovo_tail_busy() { | grep -qiE "${FM_BUSY_ROVO_REGEX:-Rovo is thinking}" } +# fm_busy_agy_tail_busy: the AGY-only temporary rendered-tail fallback. +# Consumes the tail on stdin; 0 when AGY's verified busy signature matches: +# the `esc to cancel` token in the status row the TUI pins to the bottom of +# the pane while a turn runs (verified live on agy 1.2.0; the idle status row +# shows `? for shortcuts` instead). The `Generating...` spinner word that +# renders beside it is deliberately NOT matched: it is a free-floating output +# line, so ordinary worker output echoing the word would classify an idle +# worker as busy. agy exposes no hook surface, so this fallback is the only +# pane-side source; it is never armed as a semantic writer +# (fm_busy_sources_for_harness trusts nothing for agy). +fm_busy_agy_tail_busy() { + grep -v '^[[:space:]]*$' | tail -12 \ + | grep -qiE 'esc[[:space:]]+to[[:space:]]+cancel' +} + # fm_busy_classify: semantic classification for a task whose endpoint the # caller has already established as present. Prints " ": # busy|idle|unknown plus the producing source (see header). Never probes # process state. is optional pre-captured plain output used only by -# the Grok arm; when absent the Grok arm captures through fm_backend_capture -# if available, else reports unknown capture-failed. +# the grok, rovo, and agy arms; when absent each captures through +# fm_backend_capture if available, else reports unknown capture-failed. fm_busy_classify() { # [tail40] local backend=$1 target=$2 harness=$3 id=$4 state=$5 tail40=${6-} local out rc r_state r_source native log @@ -979,6 +995,27 @@ fm_busy_classify() { # [tail40] fi return 0 ;; + agy) + if [ -z "$tail40" ]; then + if command -v fm_backend_capture >/dev/null 2>&1; then + tail40=$(fm_backend_capture "$backend" "$target" 40 2>/dev/null) || { + printf 'unknown capture-failed' + return 0 + } + else + printf 'unknown capture-failed' + return 0 + fi + fi + # Best-effort like rovo: a long turn can scroll the busy marker out of + # the captured tail, so its absence means "can't tell," never idle. + if printf '%s' "$tail40" | fm_busy_agy_tail_busy; then + printf 'busy agy-regex' + else + printf 'unknown agy-regex' + fi + return 0 + ;; esac printf 'unknown missing' } diff --git a/bin/fm-composer-lib.sh b/bin/fm-composer-lib.sh index cdea8d98abd..058dadc7293 100644 --- a/bin/fm-composer-lib.sh +++ b/bin/fm-composer-lib.sh @@ -290,7 +290,7 @@ fm_composer_strip_ghost() { # Matching a footer to confirm a keystroke landed is a different question from # asking what a worker is doing, and the two must not be conflated. # Delivery-only rendered busy footers per harness. claude/codex: "esc to -# interrupt"; opencode: "esc interrupt"; pi: "Working..."; omp: "Working…"; grok: "Ctrl+c:cancel". +# interrupt"; opencode: "esc interrupt"; pi: "Working..."; omp: "Working…"; grok: "Ctrl+c:cancel"; agy: "esc to cancel". # Claude's current spinner has a rotating glyph and word, but every active-turn # line has an ellipsis followed by a parenthesized elapsed duration. Keep this # signature separate from the shared default because that shape is not generic @@ -311,7 +311,11 @@ fm_composer_strip_ghost() { # part of that union for the same reason the others are: without it a cursor # submit could never be acknowledged, because cursor parks its terminal cursor # outside its composer and the composer verdict is therefore always `unknown`. -FM_DELIVERY_BUSY_REGEX_DEFAULT='esc (to )?interrupt|Working(\.\.\.|…)|Ctrl\+c:cancel|ctrl\+c to stop' +# agy's `esc to cancel` is part of the union for the same reason: an explicit +# tmux agy endpoint reaches the submit core with no recorded harness, and its +# bare `>` composer verdict is `unknown`, so the busy footer is the only +# turn-started acknowledgement that path can read. +FM_DELIVERY_BUSY_REGEX_DEFAULT='esc (to )?interrupt|Working(\.\.\.|…)|Ctrl\+c:cancel|ctrl\+c to stop|esc[[:space:]]+to[[:space:]]+cancel' FM_DELIVERY_CLAUDE_BUSY_REGEX_DEFAULT='esc to interrupt|…[[:space:]]+\([0-9]+[smh]' FM_DELIVERY_CODEX_BUSY_REGEX_DEFAULT='esc to interrupt' FM_DELIVERY_OPENCODE_BUSY_REGEX_DEFAULT='esc interrupt' @@ -342,6 +346,14 @@ FM_DELIVERY_GROK_BUSY_REGEX_DEFAULT='Ctrl\+c:cancel' # injection. Cursor's recorded worker state comes from its transcript fold in # bin/fm-busy-lib.sh, never from this row. FM_DELIVERY_CURSOR_BUSY_REGEX_DEFAULT='ctrl\+c to stop' +# agy (Antigravity CLI) renders a pinned status row while a turn runs: the +# `esc to cancel` token on the left and the model cell on the right (verified +# live, agy 1.2.0; the idle row shows `? for shortcuts` instead). The +# `Generating...` spinner word beside it is a free-floating output line and is +# deliberately not matched, so echoed worker output cannot fake an +# acknowledgement. Delivery guard only; recorded worker state comes from the +# agy-regex fold in bin/fm-busy-lib.sh. +FM_DELIVERY_AGY_BUSY_REGEX_DEFAULT='esc[[:space:]]+to[[:space:]]+cancel' FM_DELIVERY_KIMI_BUSY_REGEX_DEFAULT='^[[:space:]]*(🌑|🌒|🌓|🌔|🌕|🌖|🌗|🌘)[[:space:]]+·[[:space:]]+' fm_busy_lines_match() { # [harness] @@ -357,6 +369,7 @@ fm_busy_lines_match() { # [harness] pi|pi-signed) regex=$FM_DELIVERY_PI_BUSY_REGEX_DEFAULT ;; omp) regex=$FM_DELIVERY_OMP_BUSY_REGEX_DEFAULT ;; grok) regex=$FM_DELIVERY_GROK_BUSY_REGEX_DEFAULT ;; + agy) regex=$FM_DELIVERY_AGY_BUSY_REGEX_DEFAULT ;; kimi) regex=$FM_DELIVERY_KIMI_BUSY_REGEX_DEFAULT ;; cursor) regex=$FM_DELIVERY_CURSOR_BUSY_REGEX_DEFAULT ;; '') regex=$FM_DELIVERY_BUSY_REGEX_DEFAULT ;; diff --git a/bin/fm-control-lib.sh b/bin/fm-control-lib.sh index 96a7abc4860..516a00b4364 100644 --- a/bin/fm-control-lib.sh +++ b/bin/fm-control-lib.sh @@ -63,7 +63,7 @@ fm_control_verb_allowed() { # # than guessed at, exactly as a spawn on it would be. fm_control_harness_supported() { # case "${1-}" in - claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp) return 0 ;; + claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp|agy) return 0 ;; esac return 1 } @@ -74,13 +74,15 @@ fm_control_harness_supported() { # # harness= that way), which is why the spawn adapters match `claude*`, `muse*`, # and friends. This is the one place that prefix rule is stated. `pi` and # `pi-signed` are exact because a `pi*` prefix would swallow the signed adapter, -# `omp` is exact because an `omp*` prefix would claim unrelated commands, and an +# `omp` is exact because an `omp*` prefix would claim unrelated commands, `agy` +# is exact for the same reason on an even shorter name, and an # unrecognized value returns nonzero rather than being guessed into a family. fm_control_harness_family() { # case "${1-}" in pi) printf 'pi' ;; pi-signed) printf 'pi-signed' ;; omp) printf 'omp' ;; + agy) printf 'agy' ;; claude*) printf 'claude' ;; codex*) printf 'codex' ;; opencode*) printf 'opencode' ;; @@ -94,8 +96,8 @@ fm_control_harness_family() { # esac } -# Which task kinds an adapter is verified to run. muse, gemini, and rovo are -# crewmate/scout adapters only: none has a primary supervision protocol, +# Which task kinds an adapter is verified to run. muse, gemini, rovo, and agy +# are crewmate/scout adapters only: none has a primary supervision protocol, # and bin/fm-spawn.sh refuses a --secondmate launch on any of them. The control # plane asks this BEFORE it stops anything, so an incompatible relaunch target is # refused while the current agent is still running rather than after it has @@ -104,7 +106,7 @@ fm_control_harness_supports_kind() { # local harness=${1-} kind=${2-} fm_control_harness_supported "$harness" || return 1 case "$harness" in - muse|gemini|rovo) [ "$kind" != secondmate ] || return 1 ;; + muse|gemini|rovo|agy) [ "$kind" != secondmate ] || return 1 ;; esac return 0 } @@ -114,12 +116,14 @@ fm_control_harness_supports_kind() { # # gemini names its own key in the running turn's status row # (`(esc to cancel, s)`), and a single Escape was verified to cancel it. # rovo cancels on a single Escape too, printing "Agent cancelled" (verified, -# 202609.1.2). omp (Oh My Pi) shares Pi's single Escape, empty composer +# 202609.1.2). agy cancels on a single Escape, printing the Interrupted row +# with an idle composer and no repollution (verified live, agy 1.2.0 through +# Herdr). omp (Oh My Pi) shares Pi's single Escape, empty composer # afterwards, and /quit exit (verified omp 18.1.2 in a PTY, re-verified 18.1.11 # through Herdr). fm_control_interrupt_key() { # case "${1-}" in - claude|codex|opencode|pi|pi-signed|omp|kimi|cursor|gemini|muse|rovo) printf 'Escape' ;; + claude|codex|opencode|pi|pi-signed|omp|kimi|cursor|gemini|muse|rovo|agy) printf 'Escape' ;; grok) printf 'C-c' ;; *) return 1 ;; esac @@ -130,7 +134,7 @@ fm_control_interrupt_key() { # fm_control_interrupt_repeat() { # case "${1-}" in opencode) printf '2' ;; - claude|codex|pi|pi-signed|omp|grok|kimi|cursor|gemini|muse|rovo) printf '1' ;; + claude|codex|pi|pi-signed|omp|grok|kimi|cursor|gemini|muse|rovo|agy) printf '1' ;; *) return 1 ;; esac } @@ -151,7 +155,7 @@ fm_control_interrupt_repeat() { # fm_control_interrupt_clear_key() { # case "${1-}" in muse) printf 'C-u' ;; - claude|codex|opencode|pi|pi-signed|omp|grok|kimi|cursor|gemini|rovo) ;; + claude|codex|opencode|pi|pi-signed|omp|grok|kimi|cursor|gemini|rovo|agy) ;; *) return 1 ;; esac } @@ -166,7 +170,7 @@ fm_control_interrupt_ack_source() { # # rovo's TUI prints "Agent cancelled" on Escape, but for parity with # claude/cursor this stays 'none': the ack is a rendered string, not a # recorded state source, and rovo has no busy wiring to confirm against. - claude|codex|opencode|pi|pi-signed|omp|grok|kimi|cursor|gemini|rovo) printf 'none' ;; + claude|codex|opencode|pi|pi-signed|omp|grok|kimi|cursor|gemini|rovo|agy) printf 'none' ;; *) return 1 ;; esac } @@ -175,7 +179,7 @@ fm_control_interrupt_ack_source() { # fm_control_exit_command() { # case "${1-}" in claude|opencode|grok|kimi|cursor|muse|rovo) printf '/exit' ;; - codex|pi|pi-signed|omp|gemini) printf '/quit' ;; + codex|pi|pi-signed|omp|gemini|agy) printf '/quit' ;; *) return 1 ;; esac } diff --git a/bin/fm-harness.sh b/bin/fm-harness.sh index 96443cf60c9..ecc19935197 100755 --- a/bin/fm-harness.sh +++ b/bin/fm-harness.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Detect the agent harness this process tree runs on. -# Usage: fm-harness.sh print own harness: claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp|unknown +# Usage: fm-harness.sh print own harness: claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp|agy|unknown # fm-harness.sh crew print the effective CREWMATE harness # (config/crew-harness; "default" resolves to own) # fm-harness.sh secondmate print the harness the PRIMARY uses to launch @@ -167,6 +167,15 @@ detect_own() { # named `claude` with its own node child, and that fallback's *claude* # args glob would otherwise claim it if that subtree were ever walked. omp) echo omp; return ;; + # agy (Antigravity CLI) is a Go-compiled single binary whose process name + # is exactly `agy` (verified, agy 1.2.0: `ps -o comm=` reports agy and + # Herdr's process-info reports name agy with argv[0] agy). Anchored, never + # *agy*, so unrelated commands cannot be misread as this harness. agy + # publishes no harness-identity marker of its own (a live 1.2.0 TUI + # carries no AGY_* or ANTIGRAVITY_* variable; AGENT=1 seen there is an + # inherited launcher value, not an agy identity), so like muse it is + # detected by ancestry alone. + agy) echo agy; return ;; node*|python*) # Bare interpreter: match the harness name in its script path. args=$(ps -o args= -p "$pid" 2>/dev/null) diff --git a/bin/fm-send.sh b/bin/fm-send.sh index aa74940c08e..885efff7002 100755 --- a/bin/fm-send.sh +++ b/bin/fm-send.sh @@ -70,10 +70,11 @@ # failure); any other nonzero = the send failed and nothing may be assumed # delivered. Submission dispatches through the target's recorded backend; the # tmux adapter shares its composer/submit core with the away-mode daemon via -# bin/fm-tmux-lib.sh. Tune with FM_SEND_RETRIES (default 3) / FM_SEND_SLEEP -# (0.4). Slash commands, and codex `$...` skill invocations resolved through -# harness meta, get a longer pre-Enter settle so completion popups do not -# swallow Enter. A remote secondmate target has no typed text plane at all: +# bin/fm-tmux-lib.sh. Tune with FM_SEND_RETRIES (default 3; agy typed targets +# default to 20 for agy's late busy render) / FM_SEND_SLEEP (0.4). Slash +# commands, and codex `$...` skill invocations resolved through harness meta, +# get a longer pre-Enter settle so completion popups do not swallow Enter. +# A remote secondmate target has no typed text plane at all: # every remote text steer rides the inbox (a marked secondmate request already # reaches the harness as marker-prefixed chat rather than a parser command, so # routing a remote "/..." or "$..." through the record changes nothing the @@ -1030,7 +1031,21 @@ else ;; *) settle=0.3 ;; esac - retries=${FM_SEND_RETRIES:-3} + # Per-harness submit-confirm budget. agy's bare `>` composer verdict is + # `unknown`, so a landed submit is acknowledged only by the idle-to-busy + # transition poll, and agy renders its verified busy footer well after the + # shared budget expires: ~1.5s after Enter for a short steer, ~4-5s for a + # realistic longer brief (live-measured, agy 1.2.1), against the shared + # default's 3 x 0.4s. With the shared default a typed steer to an agy + # endpoint was reported exit-1 non-delivery for a message that landed and + # ran, inviting a duplicate resend. agy typed targets get a longer default + # budget (~8s at the default cadence, twice the worst measured render); an + # explicit FM_SEND_RETRIES still wins, and every other harness keeps the + # shared 3-retry default untouched. + case "$TARGET_HARNESS" in + agy) retries=${FM_SEND_RETRIES:-20} ;; + *) retries=${FM_SEND_RETRIES:-3} ;; + esac sleep_s=${FM_SEND_SLEEP:-0.4} # Type once, submit, verify. Only exact empty confirms delivery; every other # verdict preserves the loud refusal boundary. Only LOCAL targets reach this diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 13aaad7df50..2188298b37e 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -133,7 +133,7 @@ # profile consultation. A --secondmate spawn is exempt and resolves the SECONDMATE # harness (config/secondmate-harness -> config/crew-harness -> own), so the # secondmate-vs-crewmate split is DURABLE across every respawn (recovery, -# /updatefirstmate, restart). A bare adapter name (claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp) +# /updatefirstmate, restart). A bare adapter name (claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp|agy) # overrides it for this spawn (either kind). A non-flag string containing # whitespace is treated as a RAW launch command - the escape hatch for verifying # new adapters. For pi and pi-signed, fm-spawn resolves the selected executable @@ -281,6 +281,7 @@ # __CURSORBIN__ resolved, cursor-verified executable for a cursor launch # __GEMINISETTINGS__ firstmate-owned per-task gemini settings file (busy-state hooks) # __ROVOBIN__ resolved, rovo-verified executable for a rovo launch +# __AGYBIN__ resolved, agy-verified executable for an agy launch # Verified per-harness turn-end hooks are installed automatically where enabled; some live outside the worktree. # Kimi uses one surgically installed Firstmate region in $HOME/.kimi-code/config.toml, # a firstmate-owned global hook and registry, and a gitignored per-task pointer. @@ -288,7 +289,7 @@ # plus a gitignored .fm-grok-turnend worktree pointer and a state token. # muse installs no hook at all - its plugin engine is off in the default build - so # it writes state/.muse-session to bind the pane to muse's own session event -# log; muse and gemini are crewmate/scout only and are refused for --secondmate. +# log; muse, gemini, and agy are crewmate/scout only and are refused for --secondmate. # rovo installs no hook either - its eventHooks fire at tool granularity only, # never turn-end - so it carries no busy-source wiring at all and no turn-end # hook. A positional brief is dead-on-arrival (rovo loads, never works, and drops @@ -296,6 +297,14 @@ # only after a TUI readiness gate, then a delivery-confirmation gate - the same # launch-then-send shape as kimi. Its busy state is a screen-scrape fallback like # grok. rovo is crewmate/scout only and is refused for --secondmate, like muse. +# agy installs no hook either - it exposes no hook surface at all - so it +# carries no busy-source wiring and no turn-end hook. Its brief rides the launch +# command, but a fresh worktree would park it on a folder-trust dialog, so the +# spawn pre-registers the worktree in agy's own trust store through +# bin/fm-agy-trust.sh (the claude shape, but non-fatal) and then waits for a +# busy turn - answering the dialog first if it renders anyway - before +# reporting success (the rovo/kimi launch-then-confirm shape). Its busy state +# is a screen-scrape fallback like grok and rovo, and it is crewmate/scout only. # cursor installs no per-task hook either: it writes state/.cursor-session to # bind the pane to cursor's own conversation transcript (projects root, the exact # workspace path cursor records in .workspace-trusted, and the conversations that @@ -481,6 +490,8 @@ fm_backlog_directory_present "$STATE" "state directory" || { . "$SCRIPT_DIR/fm-trace-context-lib.sh" # shellcheck source=bin/fm-remote-readiness-lib.sh . "$SCRIPT_DIR/fm-remote-readiness-lib.sh" +# shellcheck source=bin/fm-timeout-lib.sh +. "$SCRIPT_DIR/fm-timeout-lib.sh" # Fail closed before any fleet mutation: a no-mistakes gate agent must never spawn # a direct report (see bin/fm-gate-refuse-lib.sh). fm_refuse_if_gate_agent @@ -1389,7 +1400,7 @@ if [ "$RELAUNCH" -eq 1 ]; then } elif [ "$KIND" = secondmate ]; then case "${POS[1]:-}" in - ''|claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp) + ''|claude|codex|opencode|pi|pi-signed|grok|kimi|cursor|gemini|muse|rovo|omp|agy) ARG3=${POS[1]:-} ;; *' '*) @@ -1467,6 +1478,37 @@ omp_model_validate() { # return 1 } +# agy pre-launch model validation. `agy models` (agy 1.2.0) prints one model per +# line as "\t