From 8053f614dfaefc4a2869a61803a78d487c661e70 Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 17:48:41 -0400 Subject: [PATCH 1/7] feat(bin): reconcile gated checks-green PRs into the Cipher pr-ready hook A gated PR that reached checks-green only after registration - via rebase or sync, repair or recovery, or a manual coordinator reconciliation - never re-invoked the pr-ready trigger, so a healthy bridge could miss the transition entirely (iinvy-control-plane#65), and iinvy-control-plane was not gated at all. A second live case (iinvy#294) showed local run-step state can also under-report while the pipeline's CI monitor is wedged even though GitHub already reports the PR open and CLEAN. - gate morris2spears/iinvy-control-plane as a third Cipher repository and make every consumer handle an arbitrary gated set - add a watcher-cadence 'reconcile' sweep that re-registers every recorded gated checks-green PR through bin/fm-pr-check.sh, the one canonical trigger, refreshing the exact head and staying silent unless a new event is acknowledged - accept GitHub's own open-and-CLEAN answer as checks-green truth in the pr-ready preflight, the retry sweep, and the reconcile sweep, so a wedged local CI monitor cannot suppress the event - extract shared live-head and forge-green readers into fm-pr-lib.sh - regression coverage: behind/red -> sync -> green, duplicate reconciliation, head advance after acknowledgement, watcher recovery end to end, and the wedged-monitor forge-green case Closes #19 Claude-Session: https://claude.ai/code/session_01S9g25zaupxf13Rdx55HsLa --- .agents/skills/cipher-hook/SKILL.md | 12 +- AGENTS.md | 4 +- bin/fm-cipher-hook-repositories | 1 + bin/fm-cipher-hook.sh | 87 ++++++++++-- bin/fm-pr-check.sh | 8 +- bin/fm-pr-lib.sh | 32 ++++- bin/fm-watch.sh | 13 ++ docs/configuration.md | 10 +- tests/fm-cipher-hook.test.sh | 204 +++++++++++++++++++++++++++- 9 files changed, 338 insertions(+), 33 deletions(-) diff --git a/.agents/skills/cipher-hook/SKILL.md b/.agents/skills/cipher-hook/SKILL.md index 529ec2442da..b9944008f42 100644 --- a/.agents/skills/cipher-hook/SKILL.md +++ b/.agents/skills/cipher-hook/SKILL.md @@ -1,8 +1,8 @@ --- name: cipher-hook description: >- - Agent-only playbook for a genuine needs-decision transition, an iinvy checks-green transition, an authenticated cipher-comment notification, or a cipher-retry check notification. - It owns the Cipher authority split, durable GitHub answer fetch, iinvy merge hold, held-delivery retry outcomes, and local receive command that avoids primary-pane ambiguity. + Agent-only playbook for a genuine needs-decision transition, an iinvy checks-green transition, an authenticated cipher-comment notification, or a cipher-retry or cipher-reconcile check notification. + It owns the Cipher authority split, durable GitHub answer fetch, iinvy merge hold, held-delivery retry and reconciliation outcomes, and local receive command that avoids primary-pane ambiguity. user-invocable: false metadata: internal: true @@ -10,7 +10,7 @@ metadata: # Cipher hook -Load this after current-state reconciliation proves a genuine `needs-decision`, when a checks-green pull request belongs to `morris2spears/iinvy` or `morris2spears/iinvy-storefront`, or on a `cipher-comment` or `cipher-retry` check notification. +Load this after current-state reconciliation proves a genuine `needs-decision`, when a checks-green pull request belongs to a repository listed in [`bin/fm-cipher-hook-repositories`](../../../bin/fm-cipher-hook-repositories), or on a `cipher-comment`, `cipher-retry`, or `cipher-reconcile` check notification. The local setup and wire schema are owned by [`docs/configuration.md`](../../../docs/configuration.md#cipherhermes-bridge), while the command contracts are owned by the headers of [`bin/fm-cipher-hook.sh`](../../../bin/fm-cipher-hook.sh) and [`bin/fm-cipher-receive.sh`](../../../bin/fm-cipher-receive.sh). ## Genuine needs-decision @@ -38,9 +38,11 @@ Never use gateway response prose as the decision ledger because GitHub is author ## Iinvy checks-green boundary -`bin/fm-pr-check.sh` emits the exact-head event automatically after it records a checks-green iinvy PR. +`bin/fm-pr-check.sh` emits the exact-head event automatically after it records a checks-green iinvy PR, and the watcher's `reconcile` sweep re-registers through that same trigger when a recorded gated PR reaches checks-green only later - after a rebase or sync, a repair or recovery, or a manual coordinator reconciliation. +Checks-green is decided by local reconciliation or by GitHub's own open-and-CLEAN answer, so a wedged local CI monitor never hides a forge-green PR, and the trigger's `armed:` line confirms only the merge watch, never delivery. +A `cipher-reconcile` check notification reporting `delivered iinvy-pr-ready ` is that checks-green transition reaching Cipher: treat it as the PR-ready milestone, report the PR to the captain with its full URL if not already reported, and keep the merge with Cipher exactly as below. A missing or disabled route, timeout, unavailable gateway, invalid acknowledgement, or delivery failure keeps the merge held. -Do not invoke the ordinary merge command for either gated repository, even after event delivery succeeds. +Do not invoke the ordinary merge command for any gated repository, even after event delivery succeeds. Cipher alone invokes `bin/fm-cipher-hook.sh merge ` after its narrow production-outage inspection, and that command still enters the guarded merge helper with an exact-head condition. Cipher's inspection is limited to cross-repository provider and consumer contracts, migration or deployment order, runtime install/import/restart behavior, and production-realistic health or smoke gates. It does not repeat code review, style review, architecture review, or no-mistakes review. diff --git a/AGENTS.md b/AGENTS.md index e9411570e85..339e28ad539 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -323,7 +323,7 @@ The worker reports the PR when CI first becomes green rather than waiting for me For PR-based ship tasks, the ready signal depends on mode: `no-mistakes` reports `done: PR checks green` after CI is green, while `direct-PR` reports `done: PR ` after opening the PR. Run `bin/fm-pr-check.sh ` - it records `pr=` and the forge's `pr_head=` when available in the task's meta and arms the watcher's merge poll. -For a checks-green PR in either canonical iinvy repository, load `cipher-hook`; its exact-head production-outage boundary supersedes routine merge authority. +For a checks-green PR in any canonical Cipher-gated iinvy repository, load `cipher-hook`; its exact-head production-outage boundary supersedes routine merge authority. Tell the captain the PR's full URL, always the complete `https://...` link rather than a bare `#number`, a concise outcome summary, and the no-mistakes risk level when applicable. A captain instruction to merge is explicit authority; `yolo` is the only standing routine authority. The same poll also watches for the captain declining the work by closing the pull request without merging it after commenting on it; load `pr-decline-feedback` on that wake before acting on his comment. @@ -491,7 +491,7 @@ These skills are not captain-invocable; load them only at their precise triggers - `bootstrap-diagnostics` - load whenever the session-start digest's bootstrap section prints an actionable diagnostic line (`MISSING:`, `MISSING_MANUAL:`, `BACKEND_INVALID:`, `NEEDS_GH_AUTH`, `TANGLE:`, `CREW_DISPATCH: invalid`, `FLEET_SYNC:`, `PR_CHECK_MIGRATION:`, `SECONDMATE_SYNC:`, `SECONDMATE_LIVENESS:`, `NUDGE_SECONDMATES:`, `FMX:`, or `FMTG:`); silence and `BOOTSTRAP_INFO:` need no load. - `diagnostic-reasoning` - load before scoping a reported bug and before acting on a diagnostic report. - `ask-user-authority` - load before deciding any ask-user finding, regardless of the project's `yolo` posture. -- `cipher-hook` - load after reconciling a genuine `needs-decision`, for a checks-green iinvy or iinvy-storefront PR, on an authenticated `cipher-comment` check notification, or on a `cipher-retry` check notification. +- `cipher-hook` - load after reconciling a genuine `needs-decision`, for a checks-green PR in a Cipher-gated iinvy repository, on an authenticated `cipher-comment` check notification, or on a `cipher-retry` or `cipher-reconcile` check notification. - `quota-array-dispatch` - load before choosing among a matched crew-dispatch profile array from current quota-axi output. - `harness-adapters` - load before spawning or recovering a crewmate or secondmate, handling a trust dialog, sending a harness-specific skill invocation, interrupting or exiting an agent, resuming an exited agent, or verifying a new harness adapter. - `firstmate-orca` - load before switching to Orca, spawning or supervising Orca-backed work, smoke-testing Orca backend behavior, debugging Orca task state, or reconciling Orca-backed task metadata. diff --git a/bin/fm-cipher-hook-repositories b/bin/fm-cipher-hook-repositories index 8aaee1c8456..0fbb9548bde 100644 --- a/bin/fm-cipher-hook-repositories +++ b/bin/fm-cipher-hook-repositories @@ -1,3 +1,4 @@ # Exact lower-case GitHub repositories whose checks-green merge boundary belongs to Cipher. morris2spears/iinvy morris2spears/iinvy-storefront +morris2spears/iinvy-control-plane diff --git a/bin/fm-cipher-hook.sh b/bin/fm-cipher-hook.sh index 8730d756984..85115ad7f98 100755 --- a/bin/fm-cipher-hook.sh +++ b/bin/fm-cipher-hook.sh @@ -3,8 +3,10 @@ # Cipher's narrow exact-head entrypoint into the guarded iinvy merge path. # # `needs-decision` first proves that the named keyed decision remains open and -# current; `pr-ready` first proves that current-state reconciliation reports a -# checks-green PR. The Python module receives only validated identity fields, +# current; `pr-ready` first proves the PR is genuinely checks-green - either +# current-state reconciliation reports it, or GitHub itself reports the pull +# request open and CLEAN, so a wedged or stale local CI monitor cannot hide a +# forge-green PR forever. The Python module receives only validated identity fields, # never worker prose. It owns strict payload/config validation, HMAC-SHA256 over # the exact request bytes, stable request IDs, private request/sent/ack/hold # records under state/cipher-hooks/, bounded retry, and localhost transport. @@ -37,11 +39,24 @@ # " status line while the keyed decision is still open, so an # answered decision cannot linger stale or keep held duplicates alive. # +# `reconcile` is the watcher's checks-green reconciliation sweep. For every +# recorded gated GitHub pull request that is currently checks-green - by local +# reconciliation or by GitHub's own open-and-CLEAN answer - it re-registers +# through bin/fm-pr-check.sh - the one canonical trigger, which refreshes the +# exact head and re-enters this pr-ready path - so a green transition reached +# after registration (a rebase or sync, a repair or recovery, a manual +# coordinator reconciliation, a wedged local CI monitor) still emits its +# exact-head event durably instead of relying on agent prose. It prints one +# line per newly acknowledged event and nothing otherwise; a held current +# identity stays with `retry-held` or, for configuration-class holds, with +# captain repair. +# # Usage: # fm-cipher-hook.sh needs-decision [decision-id] # fm-cipher-hook.sh pr-ready # fm-cipher-hook.sh retry-held # fm-cipher-hook.sh resolve-decision +# fm-cipher-hook.sh reconcile # fm-cipher-hook.sh merge [-- ] # fm-cipher-hook.sh verify-merge # fm-cipher-hook.sh repo-gated @@ -61,7 +76,7 @@ CREW_STATE_BIN=${FM_CREW_STATE_BIN:-$SCRIPT_DIR/fm-crew-state.sh} . "$SCRIPT_DIR/fm-classify-lib.sh" usage() { - sed -n '2,47s/^# \{0,1\}//p' "$0" + sed -n '2,62s/^# \{0,1\}//p' "$0" } run_python() { @@ -76,6 +91,17 @@ current_state() { # "$CREW_STATE_BIN" "$1" 2>/dev/null || true } +# Checks-green means local reconciliation reports it OR the forge itself does. +# Local run-step state is the cheap primary read, but it can under-report while +# the pipeline's own CI monitor is wedged or stale, so GitHub's open-and-CLEAN +# answer is accepted as equal truth before an event is refused or skipped. +pr_checks_green_now() { # + case "$1" in + "state: done"*"checks green"*) return 0 ;; + esac + fm_pr_github_checks_green "$2" +} + decision_is_open() { # local id=$1 decision=$2 row key verb rest [ -f "$STATE/$id.status" ] && [ ! -L "$STATE/$id.status" ] || return 1 @@ -160,10 +186,7 @@ case "${1:-}" in exit 2 } STATE_LINE=$(current_state "$ID") - case "$STATE_LINE" in - "state: done"*"checks green"*) ;; - *) exit 4 ;; - esac + pr_checks_green_now "$STATE_LINE" "$URL" || exit 4 run_python deliver iinvy-pr-ready "$ID" "$URL" exit $? ;; @@ -206,13 +229,11 @@ case "${1:-}" in # the event on a temporarily not-green read would drop a delivery # that must still retry. A merged or declined pull request instead # supersedes once teardown removes the task metadata. - case "$STATE_LINE" in - "state: done"*"checks green"*) - if FM_CIPHER_RETRIES=1 run_python deliver iinvy-pr-ready "$ID" "$ARGUMENT"; then - printf 'delivered %s iinvy-pr-ready %s\n' "$REQUEST_ID" "$ID" - fi - ;; - esac + if pr_checks_green_now "$STATE_LINE" "$ARGUMENT"; then + if FM_CIPHER_RETRIES=1 run_python deliver iinvy-pr-ready "$ID" "$ARGUMENT"; then + printf 'delivered %s iinvy-pr-ready %s\n' "$REQUEST_ID" "$ID" + fi + fi ;; esac done <&2; exit 2; } + ACKS="$STATE/cipher-hooks/acks" + HOLDS="$STATE/cipher-hooks/holds" + for META in "$STATE"/*.meta; do + [ -f "$META" ] && [ ! -L "$META" ] || continue + ID=$(basename "$META" .meta) + fm_task_id_creation_valid "$ID" || continue + URL=$(grep '^pr=' "$META" | tail -1 | cut -d= -f2-) + [ -n "$URL" ] || continue + fm_pr_url_parse "$URL" || continue + [ "$FM_PR_PROVIDER" = github ] || continue + fm_cipher_repo_gated "$FM_PR_PATH" || continue + STATE_LINE=$(current_state "$ID") + pr_checks_green_now "$STATE_LINE" "$URL" || continue + RECORDED_HEAD=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2-) + LIVE_HEAD=$(fm_pr_github_live_head "$META" "$URL") + RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || RID= + if [ -z "$LIVE_HEAD" ] || [ "$LIVE_HEAD" = "$RECORDED_HEAD" ]; then + # Same or unknown live head: an acknowledged current identity is + # complete, and a held one belongs to retry-held or captain repair. + # Only a live head that moved past the recorded one re-registers + # regardless, so a post-hold rebase still gets its fresh event. + if [ -n "$RID" ] && { [ -f "$ACKS/$RID.json" ] || [ -f "$HOLDS/$RID.json" ]; }; then + continue + fi + fi + BEFORE_ACKS=$(ls "$ACKS" 2>/dev/null || true) + "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" >/dev/null 2>&1 || true + RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || continue + [ -f "$ACKS/$RID.json" ] || continue + case "$BEFORE_ACKS" in + *"$RID.json"*) ;; + *) printf 'delivered %s iinvy-pr-ready %s\n' "$RID" "$ID" ;; + esac + done + exit 0 + ;; verify-merge) [ "$#" -eq 4 ] || { echo "error: invalid Cipher hook request" >&2; exit 2; } run_python verify-merge "$2" "$3" "$4" diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index b1ff771435b..0360de355f9 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -69,13 +69,9 @@ fi # bin/fm-teardown.sh reads the head from the forge at teardown rather than from # metadata and falls back to its provider-agnostic content check, and # bin/fm-review-diff.sh resolves the head from the remote when none is recorded. -WT=$(grep '^worktree=' "$META" | tail -1 | cut -d= -f2- || true) PR_HEAD= -if [ "$PROVIDER" = github ] && [ -n "$WT" ] && [ -d "$WT" ] && command -v gh >/dev/null 2>&1; then - if REMOTE_HEAD=$(cd "$WT" && gh pr view "$URL" --json headRefOid -q .headRefOid 2>/dev/null) \ - && fm_pr_head_valid "$REMOTE_HEAD"; then - PR_HEAD=$REMOTE_HEAD - fi +if [ "$PROVIDER" = github ]; then + PR_HEAD=$(fm_pr_github_live_head "$META" "$URL") fi META_TMP= diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index d5c5b161502..27f9bf0c151 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -224,8 +224,35 @@ fm_pr_head_valid() { [[ "$head" =~ ^[0-9a-f]{40}$|^[0-9a-f]{64}$ ]] } -# The two GitHub repositories whose checks-green merge boundary belongs to -# Cipher. Membership is decided here, by a literal case-insensitive comparison +# Best-effort live GitHub head for a task's recorded pull request, read through +# gh from the recorded task worktree. Prints the validated SHA or nothing, and +# never fails, so a missing worktree, absent gh, or forge error reads as "head +# unknown" rather than an error a caller could mistake for state. +fm_pr_github_live_head() { # + local meta=$1 url=$2 wt head + wt=$(grep '^worktree=' "$meta" | tail -1 | cut -d= -f2-) || true + [ -n "$wt" ] && [ -d "$wt" ] && command -v gh >/dev/null 2>&1 || return 0 + head=$(cd "$wt" && gh pr view "$url" --json headRefOid -q .headRefOid 2>/dev/null) || return 0 + fm_pr_head_valid "$head" || return 0 + printf '%s\n' "$head" +} + +# GitHub's own merge-readiness for a pull request: open and CLEAN, meaning +# every required check passed and the merge is not blocked. This is forge-side +# truth, independent of any local pipeline or monitor state, so a wedged or +# stale local CI monitor cannot hide a genuinely green pull request. Any error, +# absent gh, or transitional forge answer reads as not green - the safe +# direction for a caller deciding whether to emit a merge-boundary event. +fm_pr_github_checks_green() { # + local url=$1 answer + command -v gh >/dev/null 2>&1 || return 1 + answer=$(gh pr view "$url" --json state,mergeStateStatus \ + -q '.state + " " + .mergeStateStatus' 2>/dev/null) || return 1 + [ "$answer" = "OPEN CLEAN" ] +} + +# The GitHub repositories whose checks-green merge boundary belongs to Cipher. +# Membership is decided here, by a literal case-insensitive comparison # with no file, subprocess, or configuration dependency, so an unrelated # repository can never be blocked by a failure in the bridge's machinery and a # gated repository can never be released by one. bin/fm-cipher-hook-repositories @@ -233,6 +260,7 @@ fm_pr_head_valid() { FM_CIPHER_GATED_REPOSITORIES=( morris2spears/iinvy morris2spears/iinvy-storefront + morris2spears/iinvy-control-plane ) fm_cipher_repo_gated() { # diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index baca369d693..0e1efda82f0 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -821,6 +821,19 @@ while :; do fi break done + # Checks-green reconciliation for Cipher-gated pull requests runs on this + # same slow cadence over durable task metadata, so a green transition + # reached after registration (rebase/sync, repair, manual coordinator + # reconciliation) still emits its exact-head event. Silent unless a new + # event is acknowledged, so steady state costs no wake. + run_check_capture "$SCRIPT_DIR/fm-cipher-hook.sh" reconcile || exit 1 + out=$FM_CHECK_RESULT + if [ -n "$out" ]; then + reason="check: cipher-reconcile: $out" + fm_wake_append check cipher-reconcile "$reason" || exit 1 + touch "$STATE/.last-check" + wake "$reason" + fi touch "$STATE/.last-check" fi diff --git a/docs/configuration.md b/docs/configuration.md index 68c408804a4..8e85e56fed5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -73,7 +73,7 @@ A first delivery succeeds only on HTTP 202 with exactly `{"status":"accepted","r An idempotent retry succeeds only on HTTP 200 with exactly `{"status":"duplicate","delivery_id":""}` and no additional fields. Every other HTTP status or response shape is invalid and holds the event. -`bin/fm-cipher-hook.sh` validates current reconciled state before delivery, and `bin/fm-cipher-hook.py` owns the payload, HMAC, retry, response, and private-record mechanics. +`bin/fm-cipher-hook.sh` validates genuine current state before delivery - reconciled local state, or for the PR-ready event GitHub's own open-and-CLEAN answer - and `bin/fm-cipher-hook.py` owns the payload, HMAC, retry, response, and private-record mechanics. Requests are written before network delivery under mode-0700 `state/cipher-hooks/`, with separate mode-0600 request, sent, acknowledgement, hold, diagnostic, and authenticated-return records. An acknowledged logical event is not sent again after restart, while a transiently held event retries the same exact body and request ID with a fresh V2 timestamp. Timeouts, connection failures, transient HTTP failures, authentication failures, malformed responses, unsafe local files, and schema failures never print a response body or secret. @@ -85,6 +85,12 @@ A transiently held event whose task records are gone, whose identity was replace A held iinvy pull-request event is deliberately not superseded when its checks are merely not green right now - checks can regress and return to green on the same head under the same request identity - so it keeps retrying until it is delivered or until teardown removes the task records. Configuration-class holds - a missing or invalid route configuration, a bad secret, a non-transient HTTP rejection, or an invalid acknowledgement - are deliberately not auto-retried; after repairing the configuration, re-run the original trigger command, which adopts and retries the same recorded event. +Checks-green reconciliation is likewise automatic and durable rather than agent-driven. +On the same slow check cadence, the watcher runs `bin/fm-cipher-hook.sh reconcile`, which re-registers every recorded gated pull request that is currently checks-green through `bin/fm-pr-check.sh`, the one canonical trigger that refreshes the exact head and re-enters the idempotent PR-ready path. +Checks-green itself is decided by local current-state reconciliation or by GitHub's own open-and-CLEAN answer, whichever reports it first, so a wedged or stale local CI monitor cannot silently keep a forge-green pull request from ever emitting its event. +A green transition reached only after registration - a rebase or sync onto the current default branch, a repair or recovery, or a manual coordinator reconciliation run directly in the task's local copy - therefore still emits its exact-head event even though the registration-time trigger saw the pull request before it was green. +The sweep wakes Firstmate with a `cipher-reconcile` check notification only when a new event is acknowledged; a task that is not green, an identity that is already acknowledged, and a held identity awaiting retry or configuration repair all stay silent, so repeated reconciliation never redelivers. + An absent bridge or `decision_route=disabled` leaves the existing Firstmate decision authority unchanged. When the decision route is enabled, Cipher may select only a routine reversible option within the accepted GitHub issue contract and must write its recommendation, selected option, reasoning, and reversal path on GitHub before asking Firstmate to continue. Cipher escalates to Morris instead of deciding when no safe recommendation exists or the choice expands the product or engineering contract, is destructive or irreversible, changes security or credentials, migrates production data, or spends money. @@ -92,7 +98,7 @@ Firstmate keeps the worker parked until it fetches the exact durable GitHub comm After sending the worker its decision, `bin/fm-cipher-hook.sh resolve-decision ` durably closes the keyed status decision with one idempotent `resolved [key=]: Cipher decision accepted ` line, so an answered decision cannot linger open and any held duplicate delivery supersedes on the next sweep. That command refuses unless both the acknowledged needs-decision request and the authenticated decision-comment record exist, so a decision can never be marked Cipher-answered without its durable GitHub answer. -The iinvy route is fail-safe rather than optional for `morris2spears/iinvy` and `morris2spears/iinvy-storefront`. +The iinvy route is fail-safe rather than optional for every repository listed in `bin/fm-cipher-hook-repositories`, the single owner of the gated set. A missing config, disabled route, unavailable gateway, timeout, invalid response, or missing exact head holds those merges, while every other repository keeps its existing PR-ready and merge behavior without reading bridge configuration or sending an event. Cipher's review is limited to cross-repository provider and consumer contracts, migration and deployment ordering, runtime dependency install/import/restart behavior, and production-realistic smoke or health gates. Cipher invokes `bin/fm-cipher-hook.sh merge` with the request ID it inspected, and the guarded merge path adds GitHub's exact-head condition so a later head cannot inherit an earlier inspection. diff --git a/tests/fm-cipher-hook.test.sh b/tests/fm-cipher-hook.test.sh index a97a1845154..13a3d92e6e4 100755 --- a/tests/fm-cipher-hook.test.sh +++ b/tests/fm-cipher-hook.test.sh @@ -140,7 +140,12 @@ SH cat > "$dir/fakebin/gh" <<'SH' #!/usr/bin/env bash case "${1:-} ${2:-}" in - "pr view") printf '%s\n' "${FM_TEST_HEAD:-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}" ;; + "pr view") + case "$*" in + *mergeStateStatus*) printf '%s\n' "${FM_TEST_FORGE_GREEN:-}" ;; + *) printf '%s\n' "${FM_TEST_HEAD:-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}" ;; + esac + ;; esac SH cat > "$dir/fakebin/gh-axi" <<'SH' @@ -633,6 +638,7 @@ test_pr_check_allowlist_and_safe_holds() { <(grep -v -e '^#' -e '^$' "$ROOT/bin/fm-cipher-hook-repositories") >/dev/null \ || fail "the shell gate list and the Cipher allowlist file disagree" fm_cipher_repo_gated Morris2Spears/iinvy || fail "mixed-case iinvy owner was not recognized as gated" + fm_cipher_repo_gated morris2spears/iinvy-control-plane || fail "the control-plane repository was not recognized as gated" fm_cipher_repo_gated example/other && fail "an unrelated repository was treated as gated" dir=$(make_case pr-check) export FM_TEST_CREW_STATE_MARKER="$dir/crew-state.called" @@ -653,7 +659,7 @@ test_pr_check_allowlist_and_safe_holds() { expect_code 0 "$rc" "non-green iinvy PR registration should keep waiting for checks" assert_absent "$dir/state/cipher-hooks" "non-green iinvy PR emitted a Cipher event" - for repo in morris2spears/iinvy morris2spears/iinvy-storefront; do + for repo in "${FM_CIPHER_GATED_REPOSITORIES[@]}"; do rm -f "$dir/config/cipher-hooks" "$dir/crew-state.called" set +e prepare_pr_case "$dir" "missing-${repo##*/}" "$repo" > "$dir/missing.out" 2> "$dir/missing.err" @@ -992,6 +998,197 @@ test_watcher_retries_held_delivery() { pass "normal supervision retries a held delivery after gateway recovery and wakes firstmate once" } +test_reconcile_delivers_post_registration_green() { + local dir port out rc request_id count head_c + head_c='cccccccccccccccccccccccccccccccccccccccc' + dir=$(make_case reconcile) + cat > "$dir/data/backlog.md" <<'EOF' +- [ ] sync-task - close the reconciliation gap https://github.com/morris2spears/iinvy-control-plane/issues/19 (kind: ship) +EOF + port=$(start_server "$dir" accepted) + write_config "$dir" "$port" enabled enabled + + # Registration while the PR is behind or red records the PR, keeps waiting + # for checks, and spends no Cipher event. + set +e + prepare_pr_case "$dir" sync-task morris2spears/iinvy-control-plane "$HEAD_A" \ + 'state: working · source: run-step · ci running' > "$dir/register.out" 2> "$dir/register.err" + rc=$? + set -e + expect_code 0 "$rc" "non-green control-plane registration should keep waiting for checks" + assert_absent "$dir/state/cipher-hooks" "non-green registration emitted a Cipher event" + + # A reconcile sweep while the task is still not green stays silent. + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_hook "$dir" reconcile 2> "$dir/reconcile-red.err") || fail "not-green reconcile sweep failed" + [ -z "$out" ] || fail "not-green reconcile produced output: $out" + assert_absent "$dir/state/cipher-hooks" "not-green reconcile emitted a Cipher event" + + # A manual coordinator sync/rebase advances the head and checks reach green + # outside the registration path. The sweep re-registers through the one + # canonical trigger, refreshes the exact head, and delivers exactly once. + out=$(FM_TEST_HEAD=$HEAD_B \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2> "$dir/reconcile-green.err") || fail "green reconcile sweep failed" + request_id=$(request_id_for_kind "$dir" iinvy-pr-ready) + [ -n "$request_id" ] || fail "reconcile did not record the PR-ready request" + [ "$out" = "delivered $request_id iinvy-pr-ready sync-task" ] \ + || fail "reconcile did not report the delivered outcome: $out" + grep -qxF "pr_head=$HEAD_B" "$dir/state/sync-task.meta" \ + || fail "reconcile did not refresh the synced exact head in task metadata" + jq -e --arg head "$HEAD_B" 'select(.body.pr_head_sha == $head and .v2_valid)' \ + "$dir/server.log" >/dev/null || fail "delivered event did not bind the synced exact head" + assert_present "$dir/state/cipher-hooks/requests/$request_id.json" \ + "reconcile delivery did not record its durable request" + assert_present "$dir/state/cipher-hooks/sent/$request_id.json" \ + "reconcile delivery did not record its delivery attempts" + assert_present "$dir/state/cipher-hooks/acks/$request_id.json" \ + "reconcile delivery was not durably acknowledged" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 1 ] || fail "reconcile delivered $count times" + + # Duplicate reconciliation of the same green exact head is silent and does + # not redeliver. + out=$(FM_TEST_HEAD=$HEAD_B \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "duplicate reconcile sweep failed" + [ -z "$out" ] || fail "duplicate reconcile redelivered: $out" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 1 ] || fail "duplicate reconcile reached the gateway" + + # A further head advance while still green emits exactly one fresh + # exact-head event under a new request identity. + out=$(FM_TEST_HEAD=$head_c \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "advanced-head reconcile sweep failed" + case "$out" in + "delivered fmch-v1-"*" iinvy-pr-ready sync-task") ;; + *) fail "advanced head reconcile did not deliver a fresh event: $out" ;; + esac + case "$out" in + *"$request_id"*) fail "advanced head reused the earlier request identity" ;; + esac + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 2 ] || fail "advanced head reconcile delivered $count total events" + pass "reconcile delivers a post-registration checks-green transition once with the fresh exact head" +} + +test_watcher_reconciles_post_registration_green() { + local dir port request_id out wpid i + dir=$(make_case watcher-reconcile) + cat > "$dir/data/backlog.md" <<'EOF' +- [ ] recover-task - reconcile after recovery https://github.com/morris2spears/iinvy-control-plane/issues/20 (kind: ship) +EOF + port=$(start_server "$dir" accepted) + write_config "$dir" "$port" enabled enabled + # Register while red, then lose the observing session: only durable records + # remain when the PR later reaches green. + set +e + prepare_pr_case "$dir" recover-task morris2spears/iinvy-control-plane "$HEAD_A" \ + 'state: working · source: run-step · ci running' >/dev/null 2>&1 + set -e + assert_absent "$dir/state/cipher-hooks" "red registration emitted a Cipher event" + + out="$dir/watch.out" + FM_ROOT_OVERRIDE="$ROOT" \ + FM_HOME="$dir" \ + FM_STATE_OVERRIDE="$dir/state" \ + FM_DATA_OVERRIDE="$dir/data" \ + FM_CONFIG_OVERRIDE="$dir/config" \ + FM_CREW_STATE_BIN="$dir/fakebin/fm-crew-state" \ + FM_TEST_HEAD=$HEAD_B \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + FM_CIPHER_RETRIES=1 FM_CIPHER_RETRY_DELAY_SECS=0 \ + FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=1 FM_HEARTBEAT=999999 \ + PATH="$dir/fakebin:$PATH" \ + "$ROOT/bin/fm-watch.sh" > "$out" 2> "$dir/watch.err" & + wpid=$! + i=0 + while kill -0 "$wpid" 2>/dev/null && [ "$i" -lt 200 ]; do + sleep 0.1 + i=$((i + 1)) + done + if kill -0 "$wpid" 2>/dev/null; then + kill "$wpid" 2>/dev/null || true + wait "$wpid" 2>/dev/null || true + fail "watcher did not exit on the cipher reconcile wake" + fi + wait "$wpid" 2>/dev/null || true + request_id=$(request_id_for_kind "$dir" iinvy-pr-ready) + [ -n "$request_id" ] || fail "watcher reconcile did not record the PR-ready request" + assert_grep "check: cipher-reconcile: delivered $request_id iinvy-pr-ready recover-task" "$out" \ + "watcher did not surface the reconciled delivery outcome" + assert_grep "cipher-reconcile" "$dir/state/.wake-queue" \ + "watcher did not queue the durable cipher reconcile wake" + assert_present "$dir/state/cipher-hooks/acks/$request_id.json" \ + "watcher reconcile did not record the durable acknowledgement" + grep -qxF "pr_head=$HEAD_B" "$dir/state/recover-task.meta" \ + || fail "watcher reconcile did not refresh the green exact head" + pass "normal supervision reconciles a post-registration checks-green transition and wakes firstmate once" +} + +test_forge_green_overrides_wedged_local_monitor() { + local dir port out rc request_id count + dir=$(make_case forge-green) + cat > "$dir/data/backlog.md" <<'EOF' +- [ ] wedged-task - transactionless entity https://github.com/morris2spears/iinvy/issues/292 (kind: ship) +- [ ] wedged-at-arm-task - same wedge at registration https://github.com/morris2spears/iinvy/issues/293 (kind: ship) +EOF + port=$(start_server "$dir" accepted) + write_config "$dir" "$port" enabled enabled + + # Registration while neither the local pipeline nor GitHub is green: the + # "armed:" line is a watch confirmation only, never a delivery. + set +e + prepare_pr_case "$dir" wedged-task morris2spears/iinvy "$HEAD_A" \ + 'state: working · source: run-step · validating (running)' \ + > "$dir/register.out" 2> "$dir/register.err" + rc=$? + set -e + expect_code 0 "$rc" "not-yet-green registration should keep waiting for checks" + assert_grep "armed: state/wedged-task.check.sh" "$dir/register.out" \ + "registration did not arm the merge poll" + assert_absent "$dir/state/cipher-hooks" "an armed poll was mistaken for a delivery" + + # GitHub reaches green/CLEAN while the pipeline's own CI monitor stays + # silently wedged in a validating state: forge-side truth delivers anyway. + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + FM_TEST_CREW_STATE='state: working · source: run-step · validating (running)' \ + run_hook "$dir" reconcile 2> "$dir/reconcile.err") || fail "forge-green reconcile sweep failed" + request_id=$(request_id_for_kind "$dir" iinvy-pr-ready) + [ -n "$request_id" ] || fail "forge-green reconcile did not record the PR-ready request" + [ "$out" = "delivered $request_id iinvy-pr-ready wedged-task" ] \ + || fail "forge-green reconcile did not report the delivered outcome: $out" + assert_present "$dir/state/cipher-hooks/acks/$request_id.json" \ + "forge-green delivery was not durably acknowledged" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 1 ] || fail "forge-green reconcile delivered $count times" + + # Repeating the sweep with the monitor still wedged does not redeliver. + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + FM_TEST_CREW_STATE='state: working · source: run-step · validating (running)' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "repeated forge-green sweep failed" + [ -z "$out" ] || fail "repeated forge-green sweep redelivered: $out" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 1 ] || fail "repeated forge-green sweep reached the gateway" + + # When GitHub is already green at registration time, the registration-time + # trigger itself delivers despite the wedged local monitor. + set +e + FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + prepare_pr_case "$dir" wedged-at-arm-task morris2spears/iinvy "$HEAD_B" \ + 'state: working · source: run-step · validating (running)' \ + > "$dir/register2.out" 2> "$dir/register2.err" + rc=$? + set -e + expect_code 0 "$rc" "forge-green registration should deliver and arm" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 2 ] || fail "forge-green registration delivered $count total events" + jq -e --arg head "$HEAD_B" 'select(.body.pr_head_sha == $head and .body.task_id == "wedged-at-arm-task")' \ + "$dir/server.log" >/dev/null || fail "registration-time forge-green event did not bind its exact head" + pass "GitHub-side green truth delivers the PR-ready event despite a wedged local CI monitor" +} + test_v2_decision_and_pr_delivery_dedupe test_note_keyed_decision_single_hook_and_park test_resolve_decision_requires_and_follows_authenticated_answer @@ -1006,3 +1203,6 @@ test_receive_api_ignores_self_repo_worker_pane test_retry_held_after_gateway_recovery test_retry_supersedes_obsolete_holds test_watcher_retries_held_delivery +test_reconcile_delivers_post_registration_green +test_watcher_reconciles_post_registration_green +test_forge_green_overrides_wedged_local_monitor From 1f65892fdb63bb6b86e9721cb2871c2d04cf83af Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 18:17:26 -0400 Subject: [PATCH 2/7] no-mistakes(review): require real passed checks for forge-green and harden reconcile sweep --- .agents/skills/cipher-hook/SKILL.md | 2 +- bin/fm-cipher-hook.sh | 89 ++++++++++++++++++++++++----- bin/fm-pr-check-migrate.sh | 13 +++++ bin/fm-pr-lib.sh | 75 ++++++++++++++++++++---- docs/architecture.md | 2 +- docs/configuration.md | 7 ++- tests/fm-cipher-hook.test.sh | 48 ++++++++++++++-- 7 files changed, 203 insertions(+), 33 deletions(-) diff --git a/.agents/skills/cipher-hook/SKILL.md b/.agents/skills/cipher-hook/SKILL.md index b9944008f42..0fa3ac38131 100644 --- a/.agents/skills/cipher-hook/SKILL.md +++ b/.agents/skills/cipher-hook/SKILL.md @@ -39,7 +39,7 @@ Never use gateway response prose as the decision ledger because GitHub is author ## Iinvy checks-green boundary `bin/fm-pr-check.sh` emits the exact-head event automatically after it records a checks-green iinvy PR, and the watcher's `reconcile` sweep re-registers through that same trigger when a recorded gated PR reaches checks-green only later - after a rebase or sync, a repair or recovery, or a manual coordinator reconciliation. -Checks-green is decided by local reconciliation or by GitHub's own open-and-CLEAN answer, so a wedged local CI monitor never hides a forge-green PR, and the trigger's `armed:` line confirms only the merge watch, never delivery. +Checks-green is decided by local reconciliation or by GitHub's own answer - open, CLEAN, and a check rollup carrying a real passed check, never mergeability alone - so a wedged local CI monitor never hides a forge-green PR while a PR whose CI has not run is never mistaken for one, and the trigger's `armed:` line confirms only the merge watch, never delivery. A `cipher-reconcile` check notification reporting `delivered iinvy-pr-ready ` is that checks-green transition reaching Cipher: treat it as the PR-ready milestone, report the PR to the captain with its full URL if not already reported, and keep the merge with Cipher exactly as below. A missing or disabled route, timeout, unavailable gateway, invalid acknowledgement, or delivery failure keeps the merge held. Do not invoke the ordinary merge command for any gated repository, even after event delivery succeeds. diff --git a/bin/fm-cipher-hook.sh b/bin/fm-cipher-hook.sh index 85115ad7f98..41dcbe80258 100755 --- a/bin/fm-cipher-hook.sh +++ b/bin/fm-cipher-hook.sh @@ -93,8 +93,11 @@ current_state() { # # Checks-green means local reconciliation reports it OR the forge itself does. # Local run-step state is the cheap primary read, but it can under-report while -# the pipeline's own CI monitor is wedged or stale, so GitHub's open-and-CLEAN -# answer is accepted as equal truth before an event is refused or skipped. +# the pipeline's own CI monitor is wedged or stale, so GitHub's own answer is +# accepted as equal truth before an event is refused or skipped. That forge +# answer is deliberately strict - open, CLEAN, and a check rollup carrying a +# real passed check - so a pull request whose CI has not run cannot pass as +# green here (see fm_pr_github_snapshot in bin/fm-pr-lib.sh). pr_checks_green_now() { # case "$1" in "state: done"*"checks green"*) return 0 ;; @@ -245,6 +248,52 @@ EOF [ "$#" -eq 1 ] || { echo "error: invalid Cipher hook request" >&2; exit 2; } ACKS="$STATE/cipher-hooks/acks" HOLDS="$STATE/cipher-hooks/holds" + ANNOUNCED="$STATE/cipher-hooks/announced" + # The sweep runs on the watcher's own cadence, so bin/fm-pr-check.sh is + # invoked here by a descendant of the watcher rather than by an agent or + # coordinator. Its migration takes watcher exclusion by terminating the + # live watcher, which would be this process's own ancestor, so the + # watcher-internal path asks the migration to defer instead. An un-migrated + # home simply reconciles on a later cadence, after a coordinator-run + # bin/fm-pr-check.sh has crossed that boundary safely. + export FM_PR_CHECK_MIGRATION_DEFER=1 + # An announcement is durable, not in-process: the check that prints it can + # be killed by the watcher's check timeout after the acknowledgement is + # already written, and diffing the acks directory in memory would then + # leave that PR-ready silently unannounced forever. A task the sweep is + # about to register is marked pending first, so an acknowledgement that + # appears for a pending task is announced on a later cadence even if the + # sweep that produced it never got to print. An acknowledgement the sweep + # never registered was already reported by its own registration, so it is + # recorded as announced without a wake and steady state stays silent. + marker_path() { # + case "$1" in + *[!A-Za-z0-9._-]*|''|.|..) return 1 ;; + esac + printf '%s/%s\n' "$ANNOUNCED" "$1" + } + mark_announced() { # + local file + file=$(marker_path "$1") || return 0 + (umask 077 && mkdir -p "$ANNOUNCED" && : > "$file") 2>/dev/null || true + return 0 + } + clear_marker() { # + local file + file=$(marker_path "$1") || return 0 + rm -f -- "$file" 2>/dev/null || true + return 0 + } + announce_reconciled() { # + local rid=$1 id=$2 file + file=$(marker_path "$rid") || { clear_marker "$id.pending"; return 0; } + if [ ! -f "$file" ]; then + printf 'delivered %s iinvy-pr-ready %s\n' "$rid" "$id" + mark_announced "$rid" + fi + clear_marker "$id.pending" + return 0 + } for META in "$STATE"/*.meta; do [ -f "$META" ] && [ ! -L "$META" ] || continue ID=$(basename "$META" .meta) @@ -254,28 +303,42 @@ EOF fm_pr_url_parse "$URL" || continue [ "$FM_PR_PROVIDER" = github ] || continue fm_cipher_repo_gated "$FM_PR_PATH" || continue + # One forge round-trip answers both questions this sweep asks of a gated + # pull request - is it green, and where is its head now - inside a check + # budget shared with every other task in the glob. + WORKTREE=$(grep '^worktree=' "$META" | tail -1 | cut -d= -f2-) + fm_pr_github_snapshot "$WORKTREE" "$URL" + LIVE_HEAD=$FM_PR_GITHUB_HEAD STATE_LINE=$(current_state "$ID") - pr_checks_green_now "$STATE_LINE" "$URL" || continue + case "$STATE_LINE" in + "state: done"*"checks green"*) ;; + *) [ "$FM_PR_GITHUB_GREEN" = 1 ] || continue ;; + esac RECORDED_HEAD=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2-) - LIVE_HEAD=$(fm_pr_github_live_head "$META" "$URL") RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || RID= if [ -z "$LIVE_HEAD" ] || [ "$LIVE_HEAD" = "$RECORDED_HEAD" ]; then # Same or unknown live head: an acknowledged current identity is - # complete, and a held one belongs to retry-held or captain repair. - # Only a live head that moved past the recorded one re-registers - # regardless, so a post-hold rebase still gets its fresh event. - if [ -n "$RID" ] && { [ -f "$ACKS/$RID.json" ] || [ -f "$HOLDS/$RID.json" ]; }; then + # complete and only needs its announcement to be durable, and a held + # one belongs to retry-held or captain repair. Only a live head that + # moved past the recorded one re-registers regardless, so a post-hold + # rebase still gets its fresh event. + if [ -n "$RID" ] && [ -f "$ACKS/$RID.json" ]; then + if [ -f "$ANNOUNCED/$ID.pending" ]; then + announce_reconciled "$RID" "$ID" + else + mark_announced "$RID" + fi + continue + fi + if [ -n "$RID" ] && [ -f "$HOLDS/$RID.json" ]; then continue fi fi - BEFORE_ACKS=$(ls "$ACKS" 2>/dev/null || true) + mark_announced "$ID.pending" "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" >/dev/null 2>&1 || true RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || continue [ -f "$ACKS/$RID.json" ] || continue - case "$BEFORE_ACKS" in - *"$RID.json"*) ;; - *) printf 'delivered %s iinvy-pr-ready %s\n' "$RID" "$ID" ;; - esac + announce_reconciled "$RID" "$ID" done exit 0 ;; diff --git a/bin/fm-pr-check-migrate.sh b/bin/fm-pr-check-migrate.sh index 9451871449d..24c35eaf6e3 100755 --- a/bin/fm-pr-check-migrate.sh +++ b/bin/fm-pr-check-migrate.sh @@ -278,6 +278,19 @@ fi # shellcheck source=bin/fm-wake-lib.sh disable=SC1091 . "$SCRIPT_DIR/fm-wake-lib.sh" +# Watcher exclusion below pauses the live watcher by signalling it. A caller +# that is itself running under that watcher - the Cipher checks-green +# reconciliation sweep on the watcher's own cadence - would be asking this +# migration to kill its own ancestor, so such a caller sets +# FM_PR_CHECK_MIGRATION_DEFER and the migration refuses instead of racing it. +# Reaching this point means real migration work remains; it is left for the +# next coordinator-run or agent-run invocation, which owns the exclusion +# protocol safely. +if [ "${FM_PR_CHECK_MIGRATION_DEFER:-0}" = 1 ]; then + echo "PR_CHECK_MIGRATION: deferred; migration needs watcher exclusion and cannot run under the watcher" >&2 + exit 1 +fi + stopped_watcher=0 pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true) if fm_pid_alive "$pid"; then diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index 27f9bf0c151..a970dcb5a49 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -224,6 +224,64 @@ fm_pr_head_valid() { [[ "$head" =~ ^[0-9a-f]{40}$|^[0-9a-f]{64}$ ]] } +# One GitHub round-trip for the two facts a merge-boundary caller needs about a +# pull request: its exact live head, and whether the forge itself reports the +# pull request genuinely checks-green. Sets FM_PR_GITHUB_HEAD to the validated +# head SHA or nothing and FM_PR_GITHUB_GREEN to 1 or 0, and never fails, so an +# absent gh, a forge error, or any transitional answer reads as head-unknown +# and not green - the safe direction for a caller deciding whether to emit a +# merge-boundary event. +# +# Green requires the pull request open and CLEAN AND its own check rollup to +# carry at least one completed successful check with no unfinished or +# unsuccessful entry beside it. Open-and-CLEAN alone is never proof: GitHub +# answers CLEAN for a pull request with no checks at all - the window before CI +# registers its first run, and permanently in a repository that requires none - +# and merging that on a checks-green claim would merge something unverified. +# Rollup entries are classified by their own fields rather than by type name: a +# check run carries status/conclusion, a commit status carries state. +# shellcheck disable=SC2016 # A jq program, evaluated by gh and never by the shell. +FM_PR_GITHUB_SNAPSHOT_QUERY=' + (.statusCheckRollup // []) as $checks + | ($checks | map( + (.status == "COMPLETED" + and (.conclusion == "SUCCESS" or .conclusion == "NEUTRAL" or .conclusion == "SKIPPED")) + or .state == "SUCCESS")) as $settled + | ($checks | map( + (.status == "COMPLETED" and .conclusion == "SUCCESS") + or .state == "SUCCESS")) as $passed + | [(.state // ""), (.mergeStateStatus // ""), (.headRefOid // "-"), + (if ($passed | any) and ($settled | all) then "1" else "0" end)] + | join(" ")' + +fm_pr_github_snapshot() { # + local wt=${1-} url=${2-} answer state merge head green + FM_PR_GITHUB_HEAD= + FM_PR_GITHUB_GREEN=0 + command -v gh >/dev/null 2>&1 || return 0 + if [ -n "$wt" ] && [ -d "$wt" ]; then + answer=$(cd "$wt" && gh pr view "$url" \ + --json state,mergeStateStatus,headRefOid,statusCheckRollup \ + -q "$FM_PR_GITHUB_SNAPSHOT_QUERY" 2>/dev/null) || return 0 + else + answer=$(gh pr view "$url" \ + --json state,mergeStateStatus,headRefOid,statusCheckRollup \ + -q "$FM_PR_GITHUB_SNAPSHOT_QUERY" 2>/dev/null) || return 0 + fi + read -r state merge head green < printf '%s\n' "$head" } -# GitHub's own merge-readiness for a pull request: open and CLEAN, meaning -# every required check passed and the merge is not blocked. This is forge-side -# truth, independent of any local pipeline or monitor state, so a wedged or -# stale local CI monitor cannot hide a genuinely green pull request. Any error, -# absent gh, or transitional forge answer reads as not green - the safe -# direction for a caller deciding whether to emit a merge-boundary event. +# GitHub's own answer to "is this pull request genuinely checks-green": the +# forge-side truth a wedged or stale local CI monitor cannot hide. Every +# not-green direction - absent gh, forge error, no checks yet, a check still +# running - reads as not green. fm_pr_github_checks_green() { # - local url=$1 answer - command -v gh >/dev/null 2>&1 || return 1 - answer=$(gh pr view "$url" --json state,mergeStateStatus \ - -q '.state + " " + .mergeStateStatus' 2>/dev/null) || return 1 - [ "$answer" = "OPEN CLEAN" ] + fm_pr_github_snapshot "" "${1-}" + [ "$FM_PR_GITHUB_GREEN" = 1 ] } # The GitHub repositories whose checks-green merge boundary belongs to Cipher. diff --git a/docs/architecture.md b/docs/architecture.md index 37341d50125..5fe5fe15278 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -198,7 +198,7 @@ The firstmate repo itself is the exception: its `.no-mistakes/` directory is loc PR-based task merges go through `bin/fm-pr-merge.sh`, which records `pr=` and any available `pr_head=` through `bin/fm-pr-check.sh` before calling `gh-axi pr merge`. The helper requires a full `https://github.com///pull/` URL, invokes `gh-axi pr merge --repo /`, defaults to `--squash`, preserves explicit merge-method flags, and rejects malformed URLs or repo override flags before recording merge state; a well-formed GitLab merge request URL (see [docs/gitlab-merge-watch.md](gitlab-merge-watch.md)) is refused too, explicitly, rather than sent to the wrong forge. After a successful merge, the same helper best-effort closes any still-open GitHub issue that the task's own `data/backlog.md` line links in the merged PR's repository, regardless of PR body wording; [`bin/fm-pr-merge.sh`](../bin/fm-pr-merge.sh)'s header owns why the backlog line, not the PR body, is the authority and which cases are silently skipped. -The optional Cipher bridge hooks into reconciled keyed decisions and PR metadata registration rather than conversation text, while the two canonical iinvy repositories add an exact-head authorization check inside `bin/fm-pr-merge.sh`; [configuration.md](configuration.md#cipherhermes-bridge) owns the wire, authority, setup, and rollback contract. +The optional Cipher bridge hooks into reconciled keyed decisions and PR metadata registration rather than conversation text, while the gated iinvy repositories listed in [`bin/fm-cipher-hook-repositories`](../bin/fm-cipher-hook-repositories) add an exact-head authorization check inside `bin/fm-pr-merge.sh`; [configuration.md](configuration.md#cipherhermes-bridge) owns the wire, authority, setup, and rollback contract. Cipher's authenticated return path writes a task-bound durable notification through `bin/fm-cipher-receive.sh` instead of selecting a terminal pane, so self-repo workers cannot create false primary ambiguity and terminal transports retain their exactly-one-primary refusal. Teardown is fail-closed for ship worktrees: dirty worktrees refuse, and committed work must be landed before the worktree is returned. [`bin/fm-teardown.sh`](../bin/fm-teardown.sh)'s header owns the landed-work proofs, PR-discovery fallback, and stale-lock recovery procedure. diff --git a/docs/configuration.md b/docs/configuration.md index 8e85e56fed5..3667ef9be4d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -35,7 +35,7 @@ This preference is local to each Firstmate home and is not part of secondmate in ## Cipher/Hermes bridge -The optional local Cipher/Hermes bridge emits only a genuine keyed decision and an exact-head checks-green PR event for the two canonical iinvy repositories. +The optional local Cipher/Hermes bridge emits only a genuine keyed decision and an exact-head checks-green PR event for the gated iinvy repositories listed in [`bin/fm-cipher-hook-repositories`](../bin/fm-cipher-hook-repositories). Gated membership is decided by the literal case-insensitive `FM_CIPHER_GATED_REPOSITORIES` list in [`bin/fm-pr-lib.sh`](../bin/fm-pr-lib.sh), which needs no file or configuration read, and `bin/fm-cipher-hook-repositories` carries the same list for the Python payload owner with a test holding the two in step. It is not a generic notification channel, does not copy Firstmate supervision state, and never sends worker prose. GitHub remains the durable decision and review ledger, Firstmate remains coding-only, and Cipher owns the narrow iinvy production-outage inspection and merge action. @@ -73,7 +73,7 @@ A first delivery succeeds only on HTTP 202 with exactly `{"status":"accepted","r An idempotent retry succeeds only on HTTP 200 with exactly `{"status":"duplicate","delivery_id":""}` and no additional fields. Every other HTTP status or response shape is invalid and holds the event. -`bin/fm-cipher-hook.sh` validates genuine current state before delivery - reconciled local state, or for the PR-ready event GitHub's own open-and-CLEAN answer - and `bin/fm-cipher-hook.py` owns the payload, HMAC, retry, response, and private-record mechanics. +`bin/fm-cipher-hook.sh` validates genuine current state before delivery - reconciled local state, or for the PR-ready event GitHub's own answer that the pull request is open, CLEAN, and carries a passed check rollup - and `bin/fm-cipher-hook.py` owns the payload, HMAC, retry, response, and private-record mechanics. Requests are written before network delivery under mode-0700 `state/cipher-hooks/`, with separate mode-0600 request, sent, acknowledgement, hold, diagnostic, and authenticated-return records. An acknowledged logical event is not sent again after restart, while a transiently held event retries the same exact body and request ID with a fresh V2 timestamp. Timeouts, connection failures, transient HTTP failures, authentication failures, malformed responses, unsafe local files, and schema failures never print a response body or secret. @@ -87,7 +87,8 @@ Configuration-class holds - a missing or invalid route configuration, a bad secr Checks-green reconciliation is likewise automatic and durable rather than agent-driven. On the same slow check cadence, the watcher runs `bin/fm-cipher-hook.sh reconcile`, which re-registers every recorded gated pull request that is currently checks-green through `bin/fm-pr-check.sh`, the one canonical trigger that refreshes the exact head and re-enters the idempotent PR-ready path. -Checks-green itself is decided by local current-state reconciliation or by GitHub's own open-and-CLEAN answer, whichever reports it first, so a wedged or stale local CI monitor cannot silently keep a forge-green pull request from ever emitting its event. +Checks-green itself is decided by local current-state reconciliation or by GitHub's own answer, whichever reports it first, so a wedged or stale local CI monitor cannot silently keep a forge-green pull request from ever emitting its event. +The forge answer is deliberately strict: the pull request must be open and CLEAN and its own check rollup must carry at least one passed check with nothing still running or unsuccessful, because GitHub reports CLEAN for a pull request that has no checks at all - before CI registers its first run, and permanently in a repository that requires none - and such a pull request has verified nothing. A green transition reached only after registration - a rebase or sync onto the current default branch, a repair or recovery, or a manual coordinator reconciliation run directly in the task's local copy - therefore still emits its exact-head event even though the registration-time trigger saw the pull request before it was green. The sweep wakes Firstmate with a `cipher-reconcile` check notification only when a new event is acknowledged; a task that is not green, an identity that is already acknowledged, and a held identity awaiting retry or configuration repair all stay silent, so repeated reconciliation never redelivers. diff --git a/tests/fm-cipher-hook.test.sh b/tests/fm-cipher-hook.test.sh index 13a3d92e6e4..620ea667869 100755 --- a/tests/fm-cipher-hook.test.sh +++ b/tests/fm-cipher-hook.test.sh @@ -137,12 +137,19 @@ make_case() { # [ -z "${FM_TEST_CREW_STATE_MARKER:-}" ] || : > "$FM_TEST_CREW_STATE_MARKER" printf '%s\n' "${FM_TEST_CREW_STATE:-state: unknown · source: none}" SH + # The forge snapshot query resolves state, mergeability, head, and the check + # rollup verdict in gh's own jq, so the fake answers with that one line: + # " ". A pull request with no + # check run of its own is the default, and is never green. cat > "$dir/fakebin/gh" <<'SH' #!/usr/bin/env bash case "${1:-} ${2:-}" in "pr view") case "$*" in - *mergeStateStatus*) printf '%s\n' "${FM_TEST_FORGE_GREEN:-}" ;; + *statusCheckRollup*) + printf '%s %s %s\n' "${FM_TEST_FORGE_GREEN:-CLOSED BLOCKED}" \ + "${FM_TEST_HEAD:-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}" \ + "${FM_TEST_FORGE_CHECKS:-0}" ;; *) printf '%s\n' "${FM_TEST_HEAD:-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa}" ;; esac ;; @@ -1056,6 +1063,22 @@ EOF count=$(wc -l < "$dir/server.log" | tr -d ' ') [ "$count" = 1 ] || fail "duplicate reconcile reached the gateway" + # An announcement is durable, not in-process. A sweep killed by the watcher's + # check timeout after the acknowledgement landed leaves the announcement + # owed; the next sweep still reports it rather than losing the wake forever, + # and reports it without spending a second gateway delivery. + rm -f "$dir/state/cipher-hooks/announced/$request_id" + : > "$dir/state/cipher-hooks/announced/sync-task.pending" + out=$(FM_TEST_HEAD=$HEAD_B \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "interrupted-announcement sweep failed" + [ "$out" = "delivered $request_id iinvy-pr-ready sync-task" ] \ + || fail "an interrupted announcement was lost instead of reported: $out" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 1 ] || fail "recovered announcement reached the gateway again" + assert_absent "$dir/state/cipher-hooks/announced/sync-task.pending" \ + "recovered announcement left its pending marker behind" + # A further head advance while still green emits exactly one fresh # exact-head event under a new request identity. out=$(FM_TEST_HEAD=$head_c \ @@ -1150,9 +1173,26 @@ EOF "registration did not arm the merge poll" assert_absent "$dir/state/cipher-hooks" "an armed poll was mistaken for a delivery" + # GitHub answers open and CLEAN for a pull request that has no check run of + # its own - CI has not started, or the repository requires no checks. That is + # mergeability, never proof that anything was verified, so the sweep must not + # treat it as checks-green truth. + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + FM_TEST_CREW_STATE='state: working · source: run-step · validating (running)' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "checkless reconcile sweep failed" + [ -z "$out" ] || fail "a pull request with no checks was reported delivered: $out" + assert_absent "$dir/state/cipher-hooks" "a pull request with no checks spent a Cipher event" + + # A rollup whose only check is still running is equally not green. + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' FM_TEST_FORGE_CHECKS=0 \ + FM_TEST_CREW_STATE='state: working · source: run-step · validating (running)' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "pending-check reconcile sweep failed" + [ -z "$out" ] || fail "a pending check rollup was reported delivered: $out" + assert_absent "$dir/state/cipher-hooks" "a pending check rollup spent a Cipher event" + # GitHub reaches green/CLEAN while the pipeline's own CI monitor stays # silently wedged in a validating state: forge-side truth delivers anyway. - out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' FM_TEST_FORGE_CHECKS=1 \ FM_TEST_CREW_STATE='state: working · source: run-step · validating (running)' \ run_hook "$dir" reconcile 2> "$dir/reconcile.err") || fail "forge-green reconcile sweep failed" request_id=$(request_id_for_kind "$dir" iinvy-pr-ready) @@ -1165,7 +1205,7 @@ EOF [ "$count" = 1 ] || fail "forge-green reconcile delivered $count times" # Repeating the sweep with the monitor still wedged does not redeliver. - out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + out=$(FM_TEST_HEAD=$HEAD_A FM_TEST_FORGE_GREEN='OPEN CLEAN' FM_TEST_FORGE_CHECKS=1 \ FM_TEST_CREW_STATE='state: working · source: run-step · validating (running)' \ run_hook "$dir" reconcile 2>/dev/null) || fail "repeated forge-green sweep failed" [ -z "$out" ] || fail "repeated forge-green sweep redelivered: $out" @@ -1175,7 +1215,7 @@ EOF # When GitHub is already green at registration time, the registration-time # trigger itself delivers despite the wedged local monitor. set +e - FM_TEST_FORGE_GREEN='OPEN CLEAN' \ + FM_TEST_FORGE_GREEN='OPEN CLEAN' FM_TEST_FORGE_CHECKS=1 \ prepare_pr_case "$dir" wedged-at-arm-task morris2spears/iinvy "$HEAD_B" \ 'state: working · source: run-step · validating (running)' \ > "$dir/register2.out" 2> "$dir/register2.err" From f1737c2eac44fc5889f1c67f5eed018216317d51 Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 18:24:46 -0400 Subject: [PATCH 3/7] no-mistakes(review): clear stale pending announcement markers in reconcile sweep --- bin/fm-cipher-hook.sh | 22 +++++++++----- tests/fm-cipher-hook.test.sh | 59 ++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/bin/fm-cipher-hook.sh b/bin/fm-cipher-hook.sh index 41dcbe80258..01a37aae199 100755 --- a/bin/fm-cipher-hook.sh +++ b/bin/fm-cipher-hook.sh @@ -261,11 +261,13 @@ EOF # be killed by the watcher's check timeout after the acknowledgement is # already written, and diffing the acks directory in memory would then # leave that PR-ready silently unannounced forever. A task the sweep is - # about to register is marked pending first, so an acknowledgement that - # appears for a pending task is announced on a later cadence even if the - # sweep that produced it never got to print. An acknowledgement the sweep - # never registered was already reported by its own registration, so it is - # recorded as announced without a wake and steady state stays silent. + # about to register is marked pending first and unmarked as soon as that + # iteration reaches any outcome of its own, so the marker outlives the + # iteration only when the sweep was killed inside the delivery window and + # a later cadence still owes the announcement. An acknowledgement the + # sweep never registered was already reported by its own registration or + # by the retry-held sweep, so it is recorded as announced without a wake + # and steady state stays silent. marker_path() { # case "$1" in *[!A-Za-z0-9._-]*|''|.|..) return 1 ;; @@ -331,14 +333,18 @@ EOF continue fi if [ -n "$RID" ] && [ -f "$HOLDS/$RID.json" ]; then + clear_marker "$ID.pending" continue fi fi mark_announced "$ID.pending" "$SCRIPT_DIR/fm-pr-check.sh" "$ID" "$URL" >/dev/null 2>&1 || true - RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || continue - [ -f "$ACKS/$RID.json" ] || continue - announce_reconciled "$RID" "$ID" + RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || RID= + if [ -n "$RID" ] && [ -f "$ACKS/$RID.json" ]; then + announce_reconciled "$RID" "$ID" + else + clear_marker "$ID.pending" + fi done exit 0 ;; diff --git a/tests/fm-cipher-hook.test.sh b/tests/fm-cipher-hook.test.sh index 620ea667869..da7027b965c 100755 --- a/tests/fm-cipher-hook.test.sh +++ b/tests/fm-cipher-hook.test.sh @@ -1096,6 +1096,64 @@ EOF pass "reconcile delivers a post-registration checks-green transition once with the fresh exact head" } +test_reconcile_hold_leaves_no_stale_announcement() { + local dir port request_id out count + dir=$(make_case reconcile-hold) + cat > "$dir/data/backlog.md" <<'EOF' +- [ ] held-task - held reconciliation https://github.com/morris2spears/iinvy/issues/21 (kind: ship) +EOF + port=$(start_server "$dir" accepted) + write_config "$dir" "$port" enabled enabled + stop_server "$(cat "$dir/server.pid")" + + # Registration while the task is red spends no event even with the gateway + # already down. + set +e + prepare_pr_case "$dir" held-task morris2spears/iinvy "$HEAD_A" \ + 'state: working · source: run-step · ci running' >/dev/null 2>&1 + set -e + assert_absent "$dir/state/cipher-hooks" "red registration emitted a Cipher event" + + # The sweep reaches green while the gateway is unavailable: the delivery + # holds fail-closed, the sweep stays silent, and it owes no announcement. + out=$(FM_TEST_HEAD=$HEAD_A FM_CIPHER_RETRIES=1 FM_CIPHER_RETRY_DELAY_SECS=0 \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "held reconcile sweep failed" + [ -z "$out" ] || fail "a held reconcile delivery was announced: $out" + request_id=$(request_id_for_kind "$dir" iinvy-pr-ready) + [ -n "$request_id" ] || fail "held reconcile did not record the PR-ready request" + assert_present "$dir/state/cipher-hooks/holds/$request_id.json" \ + "held reconcile did not record its durable hold" + assert_absent "$dir/state/cipher-hooks/announced/held-task.pending" \ + "a held reconcile iteration left a stale pending announcement" + + # A repeated sweep against the same held identity is silent and still owes + # nothing, so the hold stays with retry-held. + out=$(FM_TEST_HEAD=$HEAD_A \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "repeated held reconcile sweep failed" + [ -z "$out" ] || fail "a repeated held reconcile sweep produced output: $out" + assert_present "$dir/state/cipher-hooks/holds/$request_id.json" \ + "repeated held reconcile dropped the transient hold" + assert_absent "$dir/state/cipher-hooks/announced/held-task.pending" \ + "a repeated held reconcile iteration left a stale pending announcement" + + # retry-held owns the recovery and reports the delivery once. The next + # reconcile sweep must not announce that same acknowledgement again. + start_server "$dir" accepted "$port" >/dev/null + out=$(FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" retry-held 2>/dev/null) || fail "retry sweep failed after gateway recovery" + [ "$out" = "delivered $request_id iinvy-pr-ready held-task" ] \ + || fail "recovered retry did not report the delivered outcome: $out" + out=$(FM_TEST_HEAD=$HEAD_A \ + FM_TEST_CREW_STATE='state: done · source: run-step · checks green: PR ready for review' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "post-recovery reconcile sweep failed" + [ -z "$out" ] || fail "reconcile re-announced a retry-held delivery: $out" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 1 ] || fail "post-recovery sweeps reached the gateway $count times" + pass "a held reconcile delivery owes no announcement and is never re-announced after retry-held" +} + test_watcher_reconciles_post_registration_green() { local dir port request_id out wpid i dir=$(make_case watcher-reconcile) @@ -1244,5 +1302,6 @@ test_retry_held_after_gateway_recovery test_retry_supersedes_obsolete_holds test_watcher_retries_held_delivery test_reconcile_delivers_post_registration_green +test_reconcile_hold_leaves_no_stale_announcement test_watcher_reconciles_post_registration_green test_forge_green_overrides_wedged_local_monitor From f8fbb2d21a83201ebe72f072746236c182dc9af9 Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 18:33:17 -0400 Subject: [PATCH 4/7] no-mistakes(document): document reconcile sweep and forge checks-green answer --- AGENTS.md | 2 +- docs/configuration.md | 2 +- docs/scripts.md | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 339e28ad539..881208b4cb5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,7 +111,7 @@ state/ volatile runtime signals; gitignored tg-away-digest/ away-mode private 0600 working copies of the escalation lines an accepted Telegram notice carried; read these when a delivery receipt points at them, folded into return catch-up, and retired with the away session (bin/fm-away-ledger-lib.sh) tg-away-versions/ away-mode immutable batch versions, one complete v./ copy of the escalation buffer, ledger sidecar, wedge marker, and digests per ledger transition, plus the single active/active.applied pointer and the .owner.lock every transition holds; the live artifacts are that pointer's projection, and retained versions fold into return catch-up and retire whole once it is acknowledged (bin/fm-away-ledger-lib.sh) pending-replies/ parent-owned secondmate pending-reply records (correlation id, delivery vs reply, recovery, escalation); fm-pending-reply-lib.sh - cipher-hooks/ private request, sent, acknowledgement, hold, diagnostic, and authenticated-return records for the optional Cipher/Hermes bridge; bin/fm-cipher-hook.sh + cipher-hooks/ private request, sent, acknowledgement, hold, announcement, diagnostic, and authenticated-return records for the optional Cipher/Hermes bridge; bin/fm-cipher-hook.sh cipher-receive.turn-ended append-only content-free monitoring edge for authenticated Cipher return records already in the durable wake queue; bin/fm-cipher-receive.sh x-inbox/ generated X-mode pending mention payloads; fmx-respond drains it (section 14) x-context/ generated X-mode durable per-request reply context and one-wake offer markers, keyed by request_id; survives inbox cleanup and expires within seven days (section 14; bin/fm-x-lib.sh) diff --git a/docs/configuration.md b/docs/configuration.md index 3667ef9be4d..19f818c7248 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -74,7 +74,7 @@ An idempotent retry succeeds only on HTTP 200 with exactly `{"status":"duplicate Every other HTTP status or response shape is invalid and holds the event. `bin/fm-cipher-hook.sh` validates genuine current state before delivery - reconciled local state, or for the PR-ready event GitHub's own answer that the pull request is open, CLEAN, and carries a passed check rollup - and `bin/fm-cipher-hook.py` owns the payload, HMAC, retry, response, and private-record mechanics. -Requests are written before network delivery under mode-0700 `state/cipher-hooks/`, with separate mode-0600 request, sent, acknowledgement, hold, diagnostic, and authenticated-return records. +Requests are written before network delivery under mode-0700 `state/cipher-hooks/`, with separate mode-0600 request, sent, acknowledgement, hold, announcement, diagnostic, and authenticated-return records. An acknowledged logical event is not sent again after restart, while a transiently held event retries the same exact body and request ID with a fresh V2 timestamp. Timeouts, connection failures, transient HTTP failures, authentication failures, malformed responses, unsafe local files, and schema failures never print a response body or secret. Only the first unchanged hold emits its bounded actionable diagnostic. diff --git a/docs/scripts.md b/docs/scripts.md index 9725ebff8f7..390d76a69b5 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -79,12 +79,12 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-peek.sh` | Print a bounded tail of a crewmate endpoint | | `fm-check-register.sh` | Bind an intentional custom watcher check to its current bytes | | `fm-check-lib.sh` | Validate custom-check registrations and prepare private execution snapshots | -| `fm-pr-lib.sh` | Own canonical task and PR validation, the literal Cipher-gated repository list, plus private atomic PR-poll publication and identity-bound retirement | +| `fm-pr-lib.sh` | Own canonical task and PR validation, the literal Cipher-gated repository list, the forge's live-head and checks-green answer, plus private atomic PR-poll publication and identity-bound retirement | | `fm-pr-poll.sh` | Provide the byte-static watcher program for validated PR/MR-poll sidecars | | `fm-pr-check-migrate.sh` | Quarantine older task polls without execution and rebuild only canonical polls | | `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, enforce any exact-head Cipher boundary, merge a task's canonical full GitHub URL, then best-effort close its backlog-linked issue | -| `fm-cipher-hook.sh` | Deliver authenticated decision and iinvy exact-head events, retry transiently held deliveries after gateway recovery, durably close a keyed decision answered by an authenticated Cipher comment, and expose Cipher's guarded merge entrypoint | +| `fm-cipher-hook.sh` | Deliver authenticated decision and iinvy exact-head events, retry transiently held deliveries after gateway recovery, reconcile gated checks-green pull requests back through the canonical trigger, durably close a keyed decision answered by an authenticated Cipher comment, and expose Cipher's guarded merge entrypoint | | `fm-cipher-receive.sh` | Queue one authenticated GitHub decision or blocker pointer by task identity without selecting a terminal pane | | `fm-promote.sh` | Promote a scout task in place to a protected ship task | | `fm-teardown.sh` | Fail-closed teardown: return landed ship worktrees, require completed scout deliverables, retire secondmate homes | From 3edbb3e3b0927d06bf943b7680d0b8d05f454802 Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 19:52:55 -0400 Subject: [PATCH 5/7] no-mistakes(review): bound reconcile sweep, keep pr_head, test forge-green query --- bin/fm-cipher-hook.sh | 96 +++++++++++++++++++++++++++++++----- bin/fm-pr-check.sh | 10 ++++ bin/fm-pr-lib.sh | 19 ++++--- docs/configuration.md | 1 + tests/fm-cipher-hook.test.sh | 52 +++++++++++++++++++ 5 files changed, 160 insertions(+), 18 deletions(-) diff --git a/bin/fm-cipher-hook.sh b/bin/fm-cipher-hook.sh index 01a37aae199..84998b2905e 100755 --- a/bin/fm-cipher-hook.sh +++ b/bin/fm-cipher-hook.sh @@ -296,15 +296,32 @@ EOF clear_marker "$id.pending" return 0 } - for META in "$STATE"/*.meta; do - [ -f "$META" ] && [ ! -L "$META" ] || continue - ID=$(basename "$META" .meta) - fm_task_id_creation_valid "$ID" || continue - URL=$(grep '^pr=' "$META" | tail -1 | cut -d= -f2-) - [ -n "$URL" ] || continue - fm_pr_url_parse "$URL" || continue - [ "$FM_PR_PROVIDER" = github ] || continue - fm_cipher_repo_gated "$FM_PR_PATH" || continue + # Markers outlive nothing they describe. A request marker is only ever + # written beside a durable acknowledgement, and a pending marker only ever + # for a live task, so a marker whose acknowledgement or task metadata is + # gone - a rebased head, a torn-down task - is orphaned and pruned here. + # Without this the directory grows one permanent entry per gated head. + prune_markers() { + local file name + [ -d "$ANNOUNCED" ] || return 0 + for file in "$ANNOUNCED"/*; do + [ -f "$file" ] || continue + name=$(basename "$file") + case "$name" in + *.pending) + [ -f "$STATE/${name%.pending}.meta" ] || rm -f -- "$file" 2>/dev/null || true + ;; + *) + [ -f "$ACKS/$name.json" ] || rm -f -- "$file" 2>/dev/null || true + ;; + esac + done + return 0 + } + reconcile_task() { # + local ID=$1 URL=$2 META="$STATE/$1.meta" + local WORKTREE LIVE_HEAD STATE_LINE RECORDED_HEAD RID + [ -f "$META" ] && [ ! -L "$META" ] || return 0 # One forge round-trip answers both questions this sweep asks of a gated # pull request - is it green, and where is its head now - inside a check # budget shared with every other task in the glob. @@ -314,7 +331,7 @@ EOF STATE_LINE=$(current_state "$ID") case "$STATE_LINE" in "state: done"*"checks green"*) ;; - *) [ "$FM_PR_GITHUB_GREEN" = 1 ] || continue ;; + *) [ "$FM_PR_GITHUB_GREEN" = 1 ] || return 0 ;; esac RECORDED_HEAD=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2-) RID=$(run_python request-id "$ID" "$URL" 2>/dev/null) || RID= @@ -330,11 +347,11 @@ EOF else mark_announced "$RID" fi - continue + return 0 fi if [ -n "$RID" ] && [ -f "$HOLDS/$RID.json" ]; then clear_marker "$ID.pending" - continue + return 0 fi fi mark_announced "$ID.pending" @@ -345,7 +362,62 @@ EOF else clear_marker "$ID.pending" fi + return 0 + } + # Selecting the gated pull requests costs no forge call, so the whole + # inventory is always known; only the per-task forge work is bounded. The + # watcher runs this sweep under a check timeout, and an unbounded sweep + # killed by it would restart at the same alphabetical head every cadence + # and never reach the later tasks - the very class of missed transition + # this sweep exists to close. So the sweep resumes where the last one + # stopped and takes at most a fixed number of tasks per cadence, and it + # records each task as taken before spending the round trip, so even a task + # whose own iteration is killed cannot pin the cursor and starve the rest. + RECONCILE_IDS=() + RECONCILE_URLS=() + for META in "$STATE"/*.meta; do + [ -f "$META" ] && [ ! -L "$META" ] || continue + ID=$(basename "$META" .meta) + fm_task_id_creation_valid "$ID" || continue + URL=$(grep '^pr=' "$META" | tail -1 | cut -d= -f2-) + [ -n "$URL" ] || continue + fm_pr_url_parse "$URL" || continue + [ "$FM_PR_PROVIDER" = github ] || continue + fm_cipher_repo_gated "$FM_PR_PATH" || continue + RECONCILE_IDS+=("$ID") + RECONCILE_URLS+=("$URL") done + TOTAL=${#RECONCILE_IDS[@]} + if [ "$TOTAL" -gt 0 ]; then + # Sweep bookkeeping, never a Cipher record, so it lives beside the task + # state rather than inside the private cipher-hooks record tree, which + # exists only once a real event does. + CURSOR="$STATE/.cipher-reconcile-cursor" + BUDGET=${FM_CIPHER_RECONCILE_BUDGET:-8} + case "$BUDGET" in + ''|*[!0-9]*|0) BUDGET=8 ;; + esac + LAST= + [ ! -f "$CURSOR" ] || LAST=$(head -1 "$CURSOR" 2>/dev/null) || LAST= + START=0 + INDEX=0 + while [ "$INDEX" -lt "$TOTAL" ]; do + if [ "${RECONCILE_IDS[$INDEX]}" = "$LAST" ]; then + START=$(( (INDEX + 1) % TOTAL )) + break + fi + INDEX=$((INDEX + 1)) + done + TAKEN=0 + while [ "$TAKEN" -lt "$TOTAL" ] && [ "$TAKEN" -lt "$BUDGET" ]; do + INDEX=$(( (START + TAKEN) % TOTAL )) + TAKEN=$((TAKEN + 1)) + ID=${RECONCILE_IDS[$INDEX]} + (umask 077 && printf '%s\n' "$ID" > "$CURSOR") 2>/dev/null || true + reconcile_task "$ID" "${RECONCILE_URLS[$INDEX]}" + done + fi + prune_markers exit 0 ;; verify-merge) diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index 0360de355f9..f542d81e870 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -69,9 +69,19 @@ fi # bin/fm-teardown.sh reads the head from the forge at teardown rather than from # metadata and falls back to its provider-agnostic content check, and # bin/fm-review-diff.sh resolves the head from the remote when none is recorded. +# +# A forge that cannot answer right now is head-unknown, never head-changed, so +# an already recorded head is kept rather than erased: dropping it would break +# the exact-head request identity of an event already in flight and leave every +# later re-registration comparing against nothing. A head that genuinely moved +# is still refreshed, because the forge answered in that case. PR_HEAD= if [ "$PROVIDER" = github ]; then PR_HEAD=$(fm_pr_github_live_head "$META" "$URL") + if [ -z "$PR_HEAD" ]; then + PR_HEAD=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2-) || true + fm_pr_head_valid "$PR_HEAD" || PR_HEAD= + fi fi META_TMP= diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index a970dcb5a49..cc091300490 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -250,7 +250,7 @@ FM_PR_GITHUB_SNAPSHOT_QUERY=' | ($checks | map( (.status == "COMPLETED" and .conclusion == "SUCCESS") or .state == "SUCCESS")) as $passed - | [(.state // ""), (.mergeStateStatus // ""), (.headRefOid // "-"), + | [(.state // "-"), (.mergeStateStatus // "-"), (.headRefOid // "-"), (if ($passed | any) and ($settled | all) then "1" else "0" end)] | join(" ")' @@ -283,14 +283,21 @@ EOF } # Best-effort live GitHub head for a task's recorded pull request, read through -# gh from the recorded task worktree. Prints the validated SHA or nothing, and -# never fails, so a missing worktree, absent gh, or forge error reads as "head -# unknown" rather than an error a caller could mistake for state. +# gh from the recorded task worktree when there still is one and from the +# ordinary environment when there is not - the pull request URL identifies the +# repository on its own, and a task whose worktree is already gone still has a +# head worth recording. Prints the validated SHA or nothing, and never fails, +# so an absent gh or a forge error reads as "head unknown" rather than an error +# a caller could mistake for state. fm_pr_github_live_head() { # local meta=$1 url=$2 wt head wt=$(grep '^worktree=' "$meta" | tail -1 | cut -d= -f2-) || true - [ -n "$wt" ] && [ -d "$wt" ] && command -v gh >/dev/null 2>&1 || return 0 - head=$(cd "$wt" && gh pr view "$url" --json headRefOid -q .headRefOid 2>/dev/null) || return 0 + command -v gh >/dev/null 2>&1 || return 0 + if [ -n "$wt" ] && [ -d "$wt" ]; then + head=$(cd "$wt" && gh pr view "$url" --json headRefOid -q .headRefOid 2>/dev/null) || return 0 + else + head=$(gh pr view "$url" --json headRefOid -q .headRefOid 2>/dev/null) || return 0 + fi fm_pr_head_valid "$head" || return 0 printf '%s\n' "$head" } diff --git a/docs/configuration.md b/docs/configuration.md index 19f818c7248..64d894ec635 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -90,6 +90,7 @@ On the same slow check cadence, the watcher runs `bin/fm-cipher-hook.sh reconcil Checks-green itself is decided by local current-state reconciliation or by GitHub's own answer, whichever reports it first, so a wedged or stale local CI monitor cannot silently keep a forge-green pull request from ever emitting its event. The forge answer is deliberately strict: the pull request must be open and CLEAN and its own check rollup must carry at least one passed check with nothing still running or unsuccessful, because GitHub reports CLEAN for a pull request that has no checks at all - before CI registers its first run, and permanently in a repository that requires none - and such a pull request has verified nothing. A green transition reached only after registration - a rebase or sync onto the current default branch, a repair or recovery, or a manual coordinator reconciliation run directly in the task's local copy - therefore still emits its exact-head event even though the registration-time trigger saw the pull request before it was green. +Selecting the gated pull requests costs no forge call, but each selected one does, so a single sweep spends at most `FM_CIPHER_RECONCILE_BUDGET` (default 8) forge round trips and the next sweep resumes at the task after the last one it took; every gated task is therefore still reached across cadences even when a home has more gated pull requests than one watcher check timeout can serve. The sweep wakes Firstmate with a `cipher-reconcile` check notification only when a new event is acknowledged; a task that is not green, an identity that is already acknowledged, and a held identity awaiting retry or configuration repair all stay silent, so repeated reconciliation never redelivers. An absent bridge or `decision_route=disabled` leaves the existing Firstmate decision authority unchanged. diff --git a/tests/fm-cipher-hook.test.sh b/tests/fm-cipher-hook.test.sh index da7027b965c..dcf052f7382 100755 --- a/tests/fm-cipher-hook.test.sh +++ b/tests/fm-cipher-hook.test.sh @@ -1287,6 +1287,58 @@ EOF pass "GitHub-side green truth delivers the PR-ready event despite a wedged local CI monitor" } +# The gh fake answers the snapshot query's own output shape, so the jq program +# that decides whether an unverified pull request may spend a merge-boundary +# event is exercised here directly, against real gh-shaped rollups. +test_forge_green_query_classifies_check_rollup() { + local query verdict rollup expected line fields + query=$(bash -uc '. "$1"; printf "%s" "$FM_PR_GITHUB_SNAPSHOT_QUERY"' _ "$ROOT/bin/fm-pr-lib.sh") \ + || fail "could not read the forge snapshot query" + [ -n "$query" ] || fail "the forge snapshot query is empty" + snapshot_line() { # + printf '{"state":"OPEN","mergeStateStatus":"CLEAN","headRefOid":"%s","statusCheckRollup":%s}' \ + "$HEAD_A" "$1" | jq -r "$query" + } + while IFS='|' read -r rollup expected; do + [ -n "$rollup" ] || continue + line=$(snapshot_line "$rollup") || fail "the snapshot query failed on rollup: $rollup" + fields=$(printf '%s\n' "$line" | awk '{print NF}') + [ "$fields" = 4 ] || fail "the snapshot query answered $fields fields for rollup $rollup: $line" + verdict=$(printf '%s\n' "$line" | awk '{print $4}') + [ "$verdict" = "$expected" ] \ + || fail "rollup $rollup answered green=$verdict, expected $expected" + done <<'EOF' +null|0 +[]|0 +[{"__typename":"CheckRun","status":"QUEUED"}]|0 +[{"__typename":"CheckRun","status":"IN_PROGRESS"}]|0 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"FAILURE"}]|0 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"CANCELLED"}]|0 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SKIPPED"}]|0 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"NEUTRAL"}]|0 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SUCCESS"}]|1 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SUCCESS"},{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SKIPPED"}]|1 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SUCCESS"},{"__typename":"CheckRun","status":"IN_PROGRESS"}]|0 +[{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SUCCESS"},{"__typename":"CheckRun","status":"COMPLETED","conclusion":"FAILURE"}]|0 +[{"__typename":"StatusContext","state":"SUCCESS"}]|1 +[{"__typename":"StatusContext","state":"PENDING"}]|0 +[{"__typename":"StatusContext","state":"FAILURE"}]|0 +[{"__typename":"StatusContext","state":"SUCCESS"},{"__typename":"CheckRun","status":"COMPLETED","conclusion":"SUCCESS"}]|1 +EOF + + # Every column keeps its place when GitHub answers without a state, a + # mergeability, or a head, so a caller never reads one field's value as + # another's. + line=$(printf '{"statusCheckRollup":[{"status":"COMPLETED","conclusion":"SUCCESS"}]}' | jq -r "$query") \ + || fail "the snapshot query failed on a field-less answer" + fields=$(printf '%s\n' "$line" | awk '{print NF}') + [ "$fields" = 4 ] || fail "a field-less answer collapsed to $fields fields: $line" + [ "$(printf '%s\n' "$line" | awk '{print $4}')" = 1 ] \ + || fail "a field-less answer misplaced the green verdict: $line" + pass "the forge snapshot query calls only a genuinely passed check rollup green" +} + +test_forge_green_query_classifies_check_rollup test_v2_decision_and_pr_delivery_dedupe test_note_keyed_decision_single_hook_and_park test_resolve_decision_requires_and_follows_authenticated_answer From 42b224ba4fbfed418043a9be7b0bf3ef1643f94a Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 20:08:16 -0400 Subject: [PATCH 6/7] no-mistakes(review): test reconcile budget and head preservation, bind fallback --- AGENTS.md | 1 + bin/fm-cipher-hook.sh | 12 ++-- bin/fm-pr-check.sh | 7 ++- tests/fm-cipher-hook.test.sh | 111 +++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 881208b4cb5..67cd17e11f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,6 +112,7 @@ state/ volatile runtime signals; gitignored tg-away-versions/ away-mode immutable batch versions, one complete v./ copy of the escalation buffer, ledger sidecar, wedge marker, and digests per ledger transition, plus the single active/active.applied pointer and the .owner.lock every transition holds; the live artifacts are that pointer's projection, and retained versions fold into return catch-up and retire whole once it is acknowledged (bin/fm-away-ledger-lib.sh) pending-replies/ parent-owned secondmate pending-reply records (correlation id, delivery vs reply, recovery, escalation); fm-pending-reply-lib.sh cipher-hooks/ private request, sent, acknowledgement, hold, announcement, diagnostic, and authenticated-return records for the optional Cipher/Hermes bridge; bin/fm-cipher-hook.sh + .cipher-reconcile-cursor private per-cadence resume point for the Cipher checks-green reconciliation sweep, naming the last gated task it took; never touch (docs/configuration.md "Cipher/Hermes bridge") cipher-receive.turn-ended append-only content-free monitoring edge for authenticated Cipher return records already in the durable wake queue; bin/fm-cipher-receive.sh x-inbox/ generated X-mode pending mention payloads; fmx-respond drains it (section 14) x-context/ generated X-mode durable per-request reply context and one-wake offer markers, keyed by request_id; survives inbox cleanup and expires within seven days (section 14; bin/fm-x-lib.sh) diff --git a/bin/fm-cipher-hook.sh b/bin/fm-cipher-hook.sh index 84998b2905e..5837b99caea 100755 --- a/bin/fm-cipher-hook.sh +++ b/bin/fm-cipher-hook.sh @@ -296,11 +296,13 @@ EOF clear_marker "$id.pending" return 0 } - # Markers outlive nothing they describe. A request marker is only ever - # written beside a durable acknowledgement, and a pending marker only ever - # for a live task, so a marker whose acknowledgement or task metadata is - # gone - a rebased head, a torn-down task - is orphaned and pruned here. - # Without this the directory grows one permanent entry per gated head. + # The announcement set is bounded by the bridge's own durable record set + # rather than pruned on a schedule of its own: a request marker is written + # only beside an acknowledgement, so the markers can never outnumber the + # acknowledgements they mirror and they retire with them. A marker whose + # acknowledgement or whose task metadata is already gone describes nothing + # and is dropped here, which is what keeps an interrupted announcement from + # outliving its task. prune_markers() { local file name [ -d "$ANNOUNCED" ] || return 0 diff --git a/bin/fm-pr-check.sh b/bin/fm-pr-check.sh index f542d81e870..032cdbfb1a7 100755 --- a/bin/fm-pr-check.sh +++ b/bin/fm-pr-check.sh @@ -74,11 +74,14 @@ fi # an already recorded head is kept rather than erased: dropping it would break # the exact-head request identity of an event already in flight and leave every # later re-registration comparing against nothing. A head that genuinely moved -# is still refreshed, because the forge answered in that case. +# is still refreshed, because the forge answered in that case. The recorded +# head is only ever carried forward for the pull request it was recorded +# against, so registering a different pull request while the forge is silent +# records no head at all rather than the previous one's. PR_HEAD= if [ "$PROVIDER" = github ]; then PR_HEAD=$(fm_pr_github_live_head "$META" "$URL") - if [ -z "$PR_HEAD" ]; then + if [ -z "$PR_HEAD" ] && grep -qxF "pr=$URL" "$META"; then PR_HEAD=$(grep '^pr_head=' "$META" | tail -1 | cut -d= -f2-) || true fm_pr_head_valid "$PR_HEAD" || PR_HEAD= fi diff --git a/tests/fm-cipher-hook.test.sh b/tests/fm-cipher-hook.test.sh index dcf052f7382..19f2c1dea02 100755 --- a/tests/fm-cipher-hook.test.sh +++ b/tests/fm-cipher-hook.test.sh @@ -143,6 +143,9 @@ SH # check run of its own is the default, and is never green. cat > "$dir/fakebin/gh" <<'SH' #!/usr/bin/env bash +# FM_TEST_GH_FAIL is a forge that cannot answer right now: gh exits non-zero +# with no output, the way an auth, network, or rate-limit failure does. +[ -z "${FM_TEST_GH_FAIL:-}" ] || exit 1 case "${1:-} ${2:-}" in "pr view") case "$*" in @@ -1338,7 +1341,115 @@ EOF pass "the forge snapshot query calls only a genuinely passed check rollup green" } +test_reconcile_budget_reaches_every_gated_task_in_turn() { + local dir port out id delivered count cursor sweep number + dir=$(make_case reconcile-budget) + cat > "$dir/data/backlog.md" <<'EOF' +- [ ] alpha-task - first gated pull request https://github.com/morris2spears/iinvy/issues/31 (kind: ship) +- [ ] bravo-task - second gated pull request https://github.com/morris2spears/iinvy/issues/32 (kind: ship) +- [ ] charlie-task - third gated pull request https://github.com/morris2spears/iinvy/issues/33 (kind: ship) +EOF + port=$(start_server "$dir" accepted) + write_config "$dir" "$port" enabled enabled + + # Three gated pull requests, none green at registration, so no event is + # spent before the sweeps run. + number=31 + for id in alpha-task bravo-task charlie-task; do + number=$((number + 1)) + fm_write_meta "$dir/state/$id.meta" \ + "window=fm-$id" "worktree=$dir/wt" "project=$dir/wt" "kind=ship" "mode=no-mistakes" + FM_TEST_HEAD=$HEAD_A \ + FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_pr_check "$dir" "$id" "https://github.com/morris2spears/iinvy/pull/$number" \ + > "$dir/$id.register.out" 2> "$dir/$id.register.err" \ + || fail "registering $id failed" + done + assert_absent "$dir/state/cipher-hooks" "a not-green registration spent a Cipher event" + + # A budget of one task per cadence must still reach all three, one per + # sweep, resuming after the task the previous sweep took. + delivered= + for sweep in 1 2 3; do + out=$(FM_CIPHER_RECONCILE_BUDGET=1 FM_TEST_HEAD=$HEAD_A \ + FM_TEST_FORGE_GREEN='OPEN CLEAN' FM_TEST_FORGE_CHECKS=1 \ + FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_hook "$dir" reconcile 2> "$dir/budget-$sweep.err") \ + || fail "budgeted reconcile sweep $sweep failed" + count=$(printf '%s\n' "$out" | grep -c 'iinvy-pr-ready' || true) + [ "$count" = 1 ] || fail "budgeted sweep $sweep delivered $count events: $out" + id=${out##* } + cursor=$(cat "$dir/state/.cipher-reconcile-cursor") + [ "$cursor" = "$id" ] \ + || fail "budgeted sweep $sweep announced $id but resumes after $cursor" + case " $delivered " in + *" $id "*) fail "budgeted sweep $sweep repeated $id instead of advancing" ;; + esac + delivered="$delivered $id" + done + [ "$delivered" = " alpha-task bravo-task charlie-task" ] \ + || fail "budgeted sweeps did not reach every gated task in turn:$delivered" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 3 ] || fail "budgeted sweeps delivered $count events" + + # A fourth sweep wraps back to the first task, which is already acknowledged + # and announced, so the wrap is silent and spends nothing. + out=$(FM_CIPHER_RECONCILE_BUDGET=1 FM_TEST_HEAD=$HEAD_A \ + FM_TEST_FORGE_GREEN='OPEN CLEAN' FM_TEST_FORGE_CHECKS=1 \ + FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_hook "$dir" reconcile 2>/dev/null) || fail "wrapped reconcile sweep failed" + [ -z "$out" ] || fail "the wrapped sweep re-announced a delivered event: $out" + [ "$(cat "$dir/state/.cipher-reconcile-cursor")" = alpha-task ] \ + || fail "the sweep did not wrap back to the first gated task" + count=$(wc -l < "$dir/server.log" | tr -d ' ') + [ "$count" = 3 ] || fail "the wrapped sweep reached the gateway" + pass "a budgeted reconcile sweep reaches every gated task in turn across cadences" +} + +test_recorded_head_survives_a_silent_forge() { + local dir head + dir=$(make_case head-preservation) + cat > "$dir/data/backlog.md" <<'EOF' +- [ ] silent-forge-task - head preservation https://github.com/morris2spears/iinvy/issues/34 (kind: ship) +EOF + fm_write_meta "$dir/state/silent-forge-task.meta" \ + "window=fm-silent-forge-task" "worktree=$dir/wt" "project=$dir/wt" "kind=ship" "mode=no-mistakes" + FM_TEST_HEAD=$HEAD_A FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_pr_check "$dir" silent-forge-task https://github.com/morris2spears/iinvy/pull/34 \ + > "$dir/register.out" 2> "$dir/register.err" || fail "registration failed" + head=$(grep '^pr_head=' "$dir/state/silent-forge-task.meta" | cut -d= -f2-) + [ "$head" = "$HEAD_A" ] || fail "registration did not record the exact head: $head" + + # Re-registering the same pull request while the forge cannot answer is + # head-unknown, never head-changed, so the recorded exact head survives. + FM_TEST_GH_FAIL=1 FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_pr_check "$dir" silent-forge-task https://github.com/morris2spears/iinvy/pull/34 \ + > "$dir/silent.out" 2> "$dir/silent.err" || fail "re-registration under a silent forge failed" + head=$(grep '^pr_head=' "$dir/state/silent-forge-task.meta" | cut -d= -f2-) + [ "$head" = "$HEAD_A" ] || fail "a silent forge erased or changed the recorded head: $head" + + # Registering a different pull request while the forge is silent records no + # head at all: the recorded one belongs to the previous pull request. + FM_TEST_GH_FAIL=1 FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_pr_check "$dir" silent-forge-task https://github.com/morris2spears/iinvy/pull/35 \ + > "$dir/replaced.out" 2> "$dir/replaced.err" || fail "replacement registration failed" + grep -q '^pr_head=' "$dir/state/silent-forge-task.meta" \ + && fail "a replaced pull request inherited the previous pull request's head" + grep -qxF 'pr=https://github.com/morris2spears/iinvy/pull/35' "$dir/state/silent-forge-task.meta" \ + || fail "the replacement pull request was not recorded" + + # Once the forge answers again the head is refreshed from it. + FM_TEST_HEAD=$HEAD_B FM_TEST_CREW_STATE='state: working · source: run-step · ci running' \ + run_pr_check "$dir" silent-forge-task https://github.com/morris2spears/iinvy/pull/35 \ + > "$dir/recovered.out" 2> "$dir/recovered.err" || fail "recovered registration failed" + head=$(grep '^pr_head=' "$dir/state/silent-forge-task.meta" | cut -d= -f2-) + [ "$head" = "$HEAD_B" ] || fail "a recovered forge did not refresh the head: $head" + pass "a silent forge preserves the recorded exact head only for the same pull request" +} + test_forge_green_query_classifies_check_rollup +test_reconcile_budget_reaches_every_gated_task_in_turn +test_recorded_head_survives_a_silent_forge test_v2_decision_and_pr_delivery_dedupe test_note_keyed_decision_single_hook_and_park test_resolve_decision_requires_and_follows_authenticated_answer From ec1b1593e3ec3a2663518984b710f5c9b9969d97 Mon Sep 17 00:00:00 2001 From: Morris Alromhein Date: Mon, 31 Aug 2026 20:15:58 -0400 Subject: [PATCH 7/7] no-mistakes(document): list reconcile budget tunable in env inventory --- docs/configuration.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/configuration.md b/docs/configuration.md index 64d894ec635..490db70912e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -561,6 +561,7 @@ FM_HEARTBEAT=600 # base seconds between heartbeat scans; no-change heartb FM_HEARTBEAT_MAX=7200 # heartbeat backoff cap FM_CHECK_INTERVAL=300 # seconds between slow checks (authenticated merge polls, custom checks, or X-mode/Telegram-mode dispatch) FM_CHECK_TIMEOUT=30 # seconds allowed per slow check script +FM_CIPHER_RECONCILE_BUDGET=8 # forge round trips one Cipher checks-green reconciliation sweep may spend; the next sweep resumes after the last task it took ("Cipher/Hermes bridge") FM_CODEX_WATCH_CHECKPOINT=180 # seconds per foreground watcher checkpoint in Codex primary supervision FM_CREW_STATE_NM_TIMEOUT=10 # seconds allowed per no-mistakes query inside fm-crew-state.sh FM_CREW_STATE_RUNS_LIMIT=200 # recent no-mistakes run rows scanned when axi status cannot be attributed to the current code