Skip to content

feat(bin): add fleet-wide PR merge conflict watcher - #3010

Closed
bingb0t5 wants to merge 7 commits into
kunchenguid:mainfrom
bingb0t5:fm/fm-pr-conflict-watch
Closed

bingb0t5 wants to merge 7 commits into
kunchenguid:mainfrom
bingb0t5:fm/fm-pr-conflict-watch

Conversation

@bingb0t5

Copy link
Copy Markdown

Intent

Build automatic detection and routing of pull-request merge conflicts across every repository this fleet works in. Add a registered watcher check that polls open PRs for repos derived from data/projects.md clones, data/secondmates.md ownership, and the firstmate checkout origin (no hardcoded repo list). Route each wake with owner-team from secondmates.md (firstmate repo -> main). Dedupe on repo+number+head SHA so the same conflict wakes once and a force-updated head wakes again. Poll GitHub lazy mergeability: never treat UNKNOWN as clean or conflicted. Print one wake line only when firstmate should wake; stay silent otherwise; finish inside FM_CHECK_TIMEOUT; use gh-axi; include drafts; register with fm-check-register.sh via arm/disarm. Detection only - no conflict resolution, rebase, or update-branch. One script, one registration. Prove with fixture tests covering newly conflicted wakes, same-head silence, new-head re-wake, unknown never reported, and empty output when clean. Document what this does NOT solve (safety net, not cure).

What Changed

  • Added bin/fm-pr-conflict-watch.sh, a registered watcher check (check/arm/disarm via fm-check-register.sh) that sweeps open GitHub PRs - drafts included - across repositories derived from data/projects.md clone origins and this firstmate checkout's own origin, routes each wake to an owner-team resolved from data/secondmates.md (firstmate repo and unmapped projects fall back to main), and prints a single pr-conflict: line only when a new conflict appears. It polls lazy mergeable state so a persistent UNKNOWN is never reported as clean or conflicted, dedupes on repo + PR number + head SHA so a force-updated head wakes again, bounds each probe and the whole sweep inside FM_CHECK_TIMEOUT, discloses conflicts cut by the line cap as N more omitted without recording them, and prunes the dedupe record back to the conflicts each sweep still observes. Detection and routing only - no rebase, update-branch, or conflict resolution.
  • Extracted the GitHub remote/PR URL to owner/repo parse out of bin/fm-bearings-snapshot.sh into a new shared bin/fm-repo-slug-lib.sh (fm_repo_slug) so the bearings snapshot and the new watcher name the same repository the same way.
  • Added tests/fm-pr-conflict-watch.test.sh (12 fixture cases covering new-conflict wakes, same-head silence, new-head re-wake, unknown never reported, unknown-reread head attribution and deadline, line-cap deferral, record pruning, draft reporting, clean-fleet silence, and arm registration), and documented the watcher's routing, dedupe, tunables, and explicit non-goals in docs/configuration.md, docs/scripts.md, and AGENTS.md.

Risk Assessment

⚠️ Medium: The three prior substantive defects are genuinely fixed and backed by regression tests that fail against the pre-fix code, and the change satisfies every source-verifiable intent criterion, but it is an 842-line new watcher whose dedupe record still grows without bound for repos at PR_LIMIT and can be wiped wholesale on an empty discovery - both safe to merge and address as follow-ups.

Testing

I ran the change's own fixture suite (12 cases, twice, no flakiness) plus the bearings-snapshot suite that shares the newly extracted repo-slug library, then built and ran an end-to-end demo that drives the real bin/fm-watch.sh against an armed pr-conflict-watch.check.sh in a throwaway FM_HOME with only GitHub faked, so repository discovery, owner routing, the trust-bound check sweep, the durable wake queue, and the drain are all production code. The captured transcript shows a clean fleet producing no wake at all, three conflicts across two project clones and the firstmate repo surfacing as one wake line routed to team-a, team-b and main with the draft flagged, silence on subsequent sweeps with unchanged heads, a fresh wake after a force-updated head, an UNKNOWN pull request re-read and never reported either way, and disarm leaving no artifacts behind. The change is a shell/CLI watcher with no rendered surface, so the reviewer-visible evidence is the operator transcript rather than a screenshot. Everything passed and I found no issues.

Evidence: End-to-end operator transcript: arm, eight watcher sweeps, drained wake lines, disarm

Source: End-to-end operator transcript: arm, eight watcher sweeps, drained wake lines, disarm

=== 3. main moves: two PRs conflict (one a draft) plus one in the firstmate repo === watcher exited to wake firstmate. what firstmate reads out of the durable wake queue: | check: <home>/state/pr-conflict-watch.check.sh: pr-conflict: owner-team=team-a repo=acme/alpha number=7 head=1111111111111111111111111111111111111111 draft=no url=https://github.com/acme/alpha/pull/7 title=Add retry budget to the worker; owner-team=team-b repo=acme/beta number=3 head=1111111111111111111111111111111111111111 draft=yes url=https://github.com/acme/beta/pull/3 title=WIP: split the ingest queue; owner-team=main repo=kunchenguid/firstmate number=2942 head=1111111111111111111111111111111111111111 draft=no url=#2942 title=bound remote job worker supervisor restarts === 4. the same heads are still conflicting: the fleet stays quiet === no wake: the watcher kept supervising for 12s and printed nothing. === 5. PR 7's head is force-updated and still conflicts: wake again === | check: <home>/state/pr-conflict-watch.check.sh: pr-conflict: owner-team=team-a repo=acme/alpha number=7 head=2222222222222222222222222222222222222222 draft=no url=https://github.com/acme/alpha/pull/7 title=Add retry budget to the worker === 6. GitHub has not computed mergeability yet (UNKNOWN): never guessed either way === no wake: the watcher kept supervising for 12s and printed nothing. | 11x gh-axi pr view --repo acme/alpha 9


=== 1. arm the watcher check (an operator runs this once per home) ===
armed: state/pr-conflict-watch.check.sh
  total 8
  -rw------- 1 rich rich  84 Aug 25 10:23 pr-conflict-watch.check-trust
  -rwx------ 1 rich rich 335 Aug 25 10:23 pr-conflict-watch.check.sh
  trust binding verifies against the armed shim bytes
  repositories this fleet works in (projects.md clones + firstmate origin): acme/alpha, acme/beta, kunchenguid/firstmate

=== 2. clean fleet: nothing conflicts, so firstmate is never woken ===
no wake: the watcher kept supervising for 12s and printed nothing.
GitHub reads this round (gh-axi, bounded by the sweep budget):
  | 5x gh-axi pr list --repo acme/alpha 
  | 5x gh-axi pr list --repo acme/beta 
  | 5x gh-axi pr list --repo kunchenguid/firstmate 

=== 3. main moves: two PRs conflict (one a draft) plus one in the firstmate repo ===
watcher exited to wake firstmate. reason printed to the supervision loop:
  | check: /tmp/pr-conflict-e2e.Np3xIt/home/state/pr-conflict-watch.check.sh: pr-conflict: 
  | owner-team=team-a repo=acme/alpha number=7 head=1111111111111111111111111111111111111111 draft=no 
  | url=https://github.com/acme/alpha/pull/7 title=Add retry budget to the worker; owner-team=team-b 
  | repo=acme/beta number=3 head=1111111111111111111111111111111111111111 draft=yes 
  | url=https://github.com/acme/beta/pull/3 title=WIP: split the ingest queue; owner-team=main 
  | repo=kunchenguid/firstmate number=2942 head=1111111111111111111111111111111111111111 draft=no 
  | url=https://github.com/kunchenguid/firstmate/pull/2942 title=bound remote job worker supervisor 
  | restarts
what firstmate reads out of the durable wake queue:
  | check: /tmp/pr-conflict-e2e.Np3xIt/home/state/pr-conflict-watch.check.sh: pr-conflict: owner-team=team-a repo=acme/alpha number=7 head=1111111111111111111111111111111111111111 draft=no url=https://github.com/acme/alpha/pull/7 title=Add retry budget to the worker; owner-team=team-b repo=acme/beta number=3 head=1111111111111111111111111111111111111111 draft=yes url=https://github.com/acme/beta/pull/3 title=WIP: split the ingest queue; owner-team=main repo=kunchenguid/firstmate number=2942 head=1111111111111111111111111111111111111111 draft=no url=https://github.com/kunchenguid/firstmate/pull/2942 title=bound remote job worker supervisor restarts
GitHub reads this round (gh-axi, bounded by the sweep budget):
  | 1x gh-axi pr list --repo acme/alpha 
  | 1x gh-axi pr list --repo acme/beta 
  | 1x gh-axi pr list --repo kunchenguid/firstmate 

=== 4. the same heads are still conflicting: the fleet stays quiet ===
no wake: the watcher kept supervising for 12s and printed nothing.
GitHub reads this round (gh-axi, bounded by the sweep budget):
  | 7x gh-axi pr list --repo acme/alpha 
  | 7x gh-axi pr list --repo acme/beta 
  | 7x gh-axi pr list --repo kunchenguid/firstmate 

=== 5. PR 7's head is force-updated and still conflicts: wake again ===
watcher exited to wake firstmate. reason printed to the supervision loop:
  | check: /tmp/pr-conflict-e2e.Np3xIt/home/state/pr-conflict-watch.check.sh: pr-conflict: 
  | owner-team=team-a repo=acme/alpha number=7 head=2222222222222222222222222222222222222222 draft=no 
  | url=https://github.com/acme/alpha/pull/7 title=Add retry budget to the worker
what firstmate reads out of the durable wake queue:
  | check: /tmp/pr-conflict-e2e.Np3xIt/home/state/pr-conflict-watch.check.sh: pr-conflict: owner-team=team-a repo=acme/alpha number=7 head=2222222222222222222222222222222222222222 draft=no url=https://github.com/acme/alpha/pull/7 title=Add retry budget to the worker
GitHub reads this round (gh-axi, bounded by the sweep budget):
  | 1x gh-axi pr list --repo acme/alpha 
  | 1x gh-axi pr list --repo acme/beta 
  | 1x gh-axi pr list --repo kunchenguid/firstmate 

=== 6. GitHub has not computed mergeability yet (UNKNOWN): never guessed either way ===
no wake: the watcher kept supervising for 12s and printed nothing.
GitHub reads this round (gh-axi, bounded by the sweep budget):
  | 4x gh-axi pr list --repo acme/alpha 
  | 3x gh-axi pr list --repo acme/beta 
  | 3x gh-axi pr list --repo kunchenguid/firstmate 
  | 11x gh-axi pr view --repo acme/alpha 9

=== 7. every conflict is resolved upstream: the fleet is quiet again ===
no wake: the watcher kept supervising for 12s and printed nothing.
GitHub reads this round (gh-axi, bounded by the sweep budget):
  | 9x gh-axi pr list --repo acme/alpha 
  | 9x gh-axi pr list --repo acme/beta 
  | 9x gh-axi pr list --repo kunchenguid/firstmate 

=== 8. disarm removes the shim, the trust binding, and the dedupe record ===
disarmed: state/pr-conflict-watch.check.sh
  pr-conflict-watch artifacts left in state/: 0
Evidence: Reproducible end-to-end script (fake GitHub only; real watcher, wake queue and drain)

Source: Reproducible end-to-end script (fake GitHub only; real watcher, wake queue and drain)

#!/usr/bin/env bash
# End-to-end demo of the fleet PR merge-conflict watcher as an operator sees it:
# arm the check, let the REAL bin/fm-watch.sh poll it on its own cadence, and
# show the wake text that reaches firstmate through the durable wake queue.
#
# GitHub is the only thing faked (a canned gh-axi). Repository discovery,
# owner routing, the check shim + trust binding, the watcher check sweep, the
# wake queue, and the drain are all the production code paths.
set -u
ROOT=${FM_E2E_ROOT:?set FM_E2E_ROOT to the firstmate checkout}
WORK=$(mktemp -d "${TMPDIR:-/tmp}/pr-conflict-e2e.XXXXXX")
HOME_DIR="$WORK/home"
FIXTURE="$HOME_DIR/fixture"
FAKEBIN="$HOME_DIR/fakebin"
TANGLE="$WORK/tangle"
REPO_A=acme/alpha
REPO_B=acme/beta
FM_SLUG=$(git -C "$ROOT" remote get-url origin | sed -n 's#.*github\.com[:/]\([^/]*/[^/]*\)#\1#p' | sed 's#\.git$##')
HEAD_ONE=1111111111111111111111111111111111111111
HEAD_TWO=2222222222222222222222222222222222222222

mkdir -p "$HOME_DIR/state" "$HOME_DIR/data" "$HOME_DIR/projects/alpha" "$HOME_DIR/projects/beta" \
  "$FIXTURE/lists" "$FIXTURE/views" "$FAKEBIN" "$TANGLE"

cat > "$HOME_DIR/data/projects.md" <<'MD'
# Projects

- alpha - alpha service (added 2026-08-25)
- beta - beta service (added 2026-08-25)
MD
cat > "$HOME_DIR/data/secondmates.md" <<'MD'
# Second mates

- team-a - owns alpha (home: /tmp/team-a; scope: alpha; projects: alpha; added 2026-08-25)
- team-b - owns beta (home: /tmp/team-b; scope: beta; projects: beta; added 2026-08-25)
MD

for p in alpha beta; do git -C "$HOME_DIR/projects/$p" init -q; done
git -C "$HOME_DIR/projects/alpha" remote add origin "https://github.com/$REPO_A.git"
git -C "$HOME_DIR/projects/beta" remote add origin "https://github.com/$REPO_B.git"

cat > "$FAKEBIN/gh-axi" <<'SH'
#!/usr/bin/env bash
set -u
fixture="$FM_E2E_FIXTURE"
repo=; number=; mode=
while [ "$#" -gt 0 ]; do
  case "$1" in
    pr) mode=$2; shift 2 ;;
    --repo) repo=$2; shift 2 ;;
    --json|--state) shift ;;
    --limit) shift 2 ;;
    [0-9]*) number=$1; shift ;;
    *) shift ;;
  esac
done
printf 'pr %s --repo %s %s\n' "$mode" "$repo" "$number" >> "$FM_E2E_GHLOG"
slug=${repo//\//__}
case "$mode" in
  list) f="$fixture/lists/${slug}.json"; if [ -f "$f" ]; then cat "$f"; else printf '[]\n'; fi ;;
  view) f="$fixture/views/${slug}-${number}.json"; [ -f "$f" ] && cat "$f" ;;
esac
exit 0
SH
chmod +x "$FAKEBIN/gh-axi"
printf '#!/usr/bin/env bash\nprintf "state: unknown - source: none - e2e fake\\n"\n' > "$FAKEBIN/fm-crew-state.sh"
chmod +x "$FAKEBIN/fm-crew-state.sh"
export FM_E2E_FIXTURE="$FIXTURE"
export FM_E2E_GHLOG="$WORK/gh-calls.log"
: > "$FM_E2E_GHLOG"

list() { local slug=${1//\//__}; printf '%s\n' "$2" > "$FIXTURE/lists/${slug}.json"; }
list "$REPO_A" '[]'; list "$REPO_B" '[]'; list "$FM_SLUG" '[]'

hr() { printf '\n=== %s ===\n' "$1"; }

drain_payload() {  # print what firstmate reads, then acknowledge it
  local err="$WORK/drain.err" seq gen
  FM_ROOT_OVERRIDE="$TANGLE" FM_STATE_OVERRIDE="$HOME_DIR/state" \
    "$ROOT/bin/fm-wake-drain.sh" 2> "$err" | cut -f5- | sed 's/^/  | /'
  seq=$(sed -n 's/.*--ack-through \([0-9][0-9]*\) .*/\1/p' "$err")
  gen=$(sed -n 's/.*--recovery-generation \([A-Za-z0-9._-]*\)$/\1/p' "$err")
  [ -n "$seq" ] && [ -n "$gen" ] && FM_ROOT_OVERRIDE="$TANGLE" FM_STATE_OVERRIDE="$HOME_DIR/state" \
    "$ROOT/bin/fm-wake-drain.sh" --ack-through "$seq" --recovery-generation "$gen" >/dev/null 2>&1
  return 0
}

run_watcher() {  # <out> <max-wait-secs>; 0 = woke, 1 = kept supervising
  local out=$1 limit=$2 pid i=0
  ( PATH="$FAKEBIN:$PATH" FM_HOME="$HOME_DIR" FM_STATE_OVERRIDE="$HOME_DIR/state" \
      FM_CREW_STATE_BIN="$FAKEBIN/fm-crew-state.sh" FM_POLL=1 FM_CHECK_INTERVAL=1 \
      FM_HEARTBEAT=999999 FM_SIGNAL_GRACE=1 FM_PR_CONFLICT_INTERVAL=0 \
      bash "$ROOT/bin/fm-watch.sh" > "$out" 2>&1 ) &
  pid=$!
  while [ "$i" -lt $((limit * 4)) ]; do
    kill -0 "$pid" 2>/dev/null || { wait "$pid" 2>/dev/null; return 0; }
    sleep 0.25; i=$((i + 1))
  done
  kill "$pid" 2>/dev/null; wait "$pid" 2>/dev/null
  return 1
}

# One observation round. A watcher restarted after downtime first announces
# "check: rearm-resurface" (a watcher-lifecycle wake, nothing to do with PRs);
# that is consumed and the round is then observed on a warm watcher.
watch_round() {  # <label> <max-wait-secs>
  local label=$1 limit=$2 out before after
  out="$WORK/$label.watch"
  before=$(wc -l < "$FM_E2E_GHLOG")
  if run_watcher "$out" "$limit" && [ "$(cat "$out")" = "check: rearm-resurface" ]; then
    drain_payload >/dev/null
    before=$(wc -l < "$FM_E2E_GHLOG")
    run_watcher "$out" "$limit" || true
  fi
  after=$(wc -l < "$FM_E2E_GHLOG")
  if [ -s "$out" ]; then
    printf 'watcher exited to wake firstmate. reason printed to the supervision loop:\n'
    fold -s -w 100 "$out" | sed 's/^/  | /'
    printf 'what firstmate reads out of the durable wake queue:\n'
    drain_payload
  else
    printf 'no wake: the watcher kept supervising for %ss and printed nothing.\n' "$limit"
  fi
  printf 'GitHub reads this round (gh-axi, bounded by the sweep budget):\n'
  sed -n "$((before + 1)),${after}p" "$FM_E2E_GHLOG" | sort | uniq -c \
    | sed 's/^ *\([0-9][0-9]*\) /  | \1x gh-axi /'
}

hr "1. arm the watcher check (an operator runs this once per home)"
PATH="$FAKEBIN:$PATH" FM_HOME="$HOME_DIR" "$ROOT/bin/fm-pr-conflict-watch.sh" arm
ls -l "$HOME_DIR/state" | sed 's/^/  /'
FM_STATE_OVERRIDE="$HOME_DIR/state" "$ROOT/bin/fm-check-register.sh" pr-conflict-watch >/dev/null \
  && printf '  trust binding verifies against the armed shim bytes\n'
printf '  repositories this fleet works in (projects.md clones + firstmate origin): %s, %s, %s\n' \
  "$REPO_A" "$REPO_B" "$FM_SLUG"

hr "2. clean fleet: nothing conflicts, so firstmate is never woken"
watch_round clean 12

hr "3. main moves: two PRs conflict (one a draft) plus one in the firstmate repo"
list "$REPO_A" "[{\"number\":7,\"title\":\"Add retry budget to the worker\",\"url\":\"https://github.com/$REPO_A/pull/7\",\"headRefOid\":\"$HEAD_ONE\",\"isDraft\":false,\"mergeable\":\"CONFLICTING\"}]"
list "$REPO_B" "[{\"number\":3,\"title\":\"WIP: split the ingest queue\",\"url\":\"https://github.com/$REPO_B/pull/3\",\"headRefOid\":\"$HEAD_ONE\",\"isDraft\":true,\"mergeable\":\"CONFLICTING\"}]"
list "$FM_SLUG" "[{\"number\":2942,\"title\":\"bound remote job worker supervisor restarts\",\"url\":\"https://github.com/$FM_SLUG/pull/2942\",\"headRefOid\":\"$HEAD_ONE\",\"isDraft\":false,\"mergeable\":\"CONFLICTING\"}]"
watch_round conflict 20

hr "4. the same heads are still conflicting: the fleet stays quiet"
watch_round repeat 12

hr "5. PR 7's head is force-updated and still conflicts: wake again"
list "$REPO_A" "[{\"number\":7,\"title\":\"Add retry budget to the worker\",\"url\":\"https://github.com/$REPO_A/pull/7\",\"headRefOid\":\"$HEAD_TWO\",\"isDraft\":false,\"mergeable\":\"CONFLICTING\"}]"
watch_round forced 20

hr "6. GitHub has not computed mergeability yet (UNKNOWN): never guessed either way"
list "$REPO_A" "[{\"number\":9,\"title\":\"Lazy mergeability\",\"url\":\"https://github.com/$REPO_A/pull/9\",\"headRefOid\":\"$HEAD_TWO\",\"isDraft\":false,\"mergeable\":\"UNKNOWN\"}]"
list "$REPO_B" '[]'; list "$FM_SLUG" '[]'
printf '%s\n' "{\"mergeable\":\"UNKNOWN\",\"number\":9,\"title\":\"Lazy mergeability\",\"url\":\"https://github.com/$REPO_A/pull/9\",\"headRefOid\":\"$HEAD_TWO\",\"isDraft\":false}" \
  > "$FIXTURE/views/acme__alpha-9.json"
watch_round unknown 12

hr "7. every conflict is resolved upstream: the fleet is quiet again"
list "$REPO_A" '[]'
watch_round resolved 12

hr "8. disarm removes the shim, the trust binding, and the dedupe record"
FM_HOME="$HOME_DIR" "$ROOT/bin/fm-pr-conflict-watch.sh" disarm
printf '  pr-conflict-watch artifacts left in state/: %s\n' \
  "$(ls "$HOME_DIR/state" 2>/dev/null | grep -c 'pr-conflict-watch')"

rm -rf "$WORK"
Evidence: Fixture-test transcript naming the behaviours the intent requires

Source: Fixture-test transcript naming the behaviours the intent requires

ok - newly conflicted PR wakes once with routing fields
ok - same conflicted head stays silent on the next poll
ok - new head after force-update wakes again
ok - persistent UNKNOWN is never reported as conflicted or clean
ok - UNKNOWN that settles on CONFLICTING wakes once
ok - a reread wake is labelled and deduped by the head it judged
ok - conflicts cut by the line cap wake on a later sweep instead of being lost
ok - keys for conflicts the sweep no longer observes leave the record
ok - UNKNOWN rereads stop at the sweep deadline
ok - clean fleet output is empty
ok - conflicted draft PRs are reported
ok - arm writes and registers the watcher check

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 5 infos
  • 🚨 bin/fm-pr-conflict-watch.sh:469 - Every emitted finding is recorded as reported (record_add_key) even when it never made it into the printed line. emit_finding concatenates all findings into one string and action_check cuts it to MAX_LINE=1000 with fm_cap_line_var, but nothing ties the recorded keys to what survived the cut. Concrete sequence: main moves and 8 open PRs go CONFLICTING in one sweep. A realistic finding is ~220 chars (owner-team=main repo=kunchenguid/firstmate number=2942 head=&lt;40 hex&gt; draft=no url=https://github.com/kunchenguid/firstmate/pull/2942 title=...), so after the 13-char pr-conflict: prefix only ~4 findings fit; findings 5-8 are replaced by [truncated]. All 8 keys are still persisted by record_write, so on the next sweep record_has_key skips them and those 4 conflicts never wake firstmate at all - the heads are unchanged, so nothing ever re-triggers them. This defeats the intent's "the same conflict wakes once" for exactly the mass-conflict scenario the watcher exists for. Fix: only record keys for findings actually present in the printed line, or cap the finding count and disclose "N more omitted" while leaving the omitted keys unrecorded (the sibling bin/fm-tool-update-check.sh:735 handles the same hazard by comparing the whole finding set rather than the cut line).
  • ⚠️ bin/fm-pr-conflict-watch.sh:500 - When the sweep runs out of budget the loop just breaks, and repos never reached are indistinguishable from clean in the output. discover_repos emits a fixed order (projects.md order, with the firstmate repo appended last), so the same tail repos starve on every sweep, forever. With BUDGET_SECS=20 and one gh pr list per repo (plus up to UNKNOWN_ATTEMPTS extra pr view calls per UNKNOWN PR, and PR_LIMIT=30 PRs per repo), a fleet with more than ~15 repos - or one busy repo early in the list - permanently silences everything after the cut point, including the firstmate repo itself. Intent requires detection "across every repository this fleet works in"; the established sibling contract in bin/fm-tool-update-check.sh:216 (budget_allows) emits "check incomplete: the time budget ran out before <name>" precisely so an unfinished sweep says so instead of reading as clean. Same applies to the other silent-failure paths that return without a word: gh pr list failure or timeout (:432), missing jq (:477), missing gh-axi (:480). Since the fix adds a user-visible line, confirm the desired shape.
  • ⚠️ bin/fm-pr-conflict-watch.sh:399 - pr_mergeable_resolved never checks budget_exhausted and its total cost is not bounded by DEADLINE. Once the deadline passes, probe_bound floors at PROBE_MIN_SECS=1, so the loop can still spend UNKNOWN_ATTEMPTS gh calls plus (UNKNOWN_ATTEMPTS-1) sleeps of UNKNOWN_WAIT past the deadline. BUDGET_MAX only reserves PROBE_MIN_SECS+CLOCK_ROUNDING+KILL_GRACE = 3s of headroom. Concrete: FM_PR_CONFLICT_UNKNOWN_ATTEMPTS=10 and FM_PR_CONFLICT_UNKNOWN_WAIT=5 are both inside the documented valid ranges (:121-143) and yield up to 101 + 95 = 55s of overshoot on top of the 20s budget; FM_PR_CONFLICT_BUDGET_SECS=30 with the default FM_CHECK_TIMEOUT=30 gives 27+6=33s at default knobs. Either exceeds the watcher's CHECK_TIMEOUT (bin/fm-watch.sh:153), which kills the process group - and because the script only prints and records at the very end (:509-512), a killed run prints nothing and records nothing, so the same sweep dies identically on every poll and no conflict is ever reported. Fix: cut the retry loop short on budget_exhausted, and/or fold UNKNOWN_ATTEMPTS*(PROBE_MIN_SECS+UNKNOWN_WAIT) into the BUDGET_MAX reserve.
  • ⚠️ bin/fm-pr-conflict-watch.sh:492 - The budget-cut notice is pushed into FINDINGS on every sweep but is never deduped against the record (REPORTED_KEYS only holds conflict keys), so a non-empty line is printed every cadence even when no PR is conflicted. Concrete: FM_PR_CONFLICT_BUDGET_SECS=30 passes validation (1..120 at :109-119); with the default FM_CHECK_TIMEOUT=30, BUDGET_MAX=27, so BUDGET_CUT_FROM is set and every poll prints pr-conflict: sweep budget 30s cut to 27s to stay inside the watcher check timeout of 30s, waking firstmate every FM_CHECK_INTERVAL indefinitely with no new information. This contradicts the intent's "Print one wake line only when firstmate should wake; stay silent otherwise" and "empty output when clean". The sibling avoids this by gating the print on [ &#34;$FINDINGS&#34; != &#34;$RECORD_REPORTED&#34; ] (bin/fm-tool-update-check.sh:735). Fix: report the cut once (record it), or only attach it to a line that already carries a conflict.
  • ⚠️ tests/fm-pr-conflict-watch.test.sh:133 - write_view_sequence writes the three view payloads as newline-delimited JSON objects, but the fake gh-axi reads them with jq -c &#34;.[$((idx - 1))]&#34; &#34;$file&#34; (:98), which is array indexing. I verified: printf &#39;%s\n&#39; &#39;{&#34;a&#34;:1}&#39; &#39;{&#34;a&#34;:2}&#39; | jq -c &#39;.[0]&#39; fails with "Cannot index object with number" and exit 5. The fake then falls through to exit 0 with empty stdout, so gh_bounded pr view succeeds with no output, json_field on empty input exits 4, and pr_mergeable_resolved takes its return 2 probe-failure branch at :402 - the UNKNOWN retry loop, the attempt counter, and the "persistent UNKNOWN stays unknown" decision at :406-413 are never executed. test_unknown_never_reported therefore passes because empty gh output is silent, not because UNKNOWN is handled, leaving the intent-required "never treat UNKNOWN as clean or conflicted" behavior unproven (and the whole pr_mergeable_resolved path, including UNKNOWN -> CONFLICTING resolution, untested). Fix: write the sequence as a single JSON array so jq &#39;.[i]&#39; resolves, and add a case where the retries settle on CONFLICTING.
  • ⚠️ bin/fm-pr-conflict-watch.sh:378 - IFS=&#39;;&#39; read -r -a _parts &lt;&lt;&lt; &#34;$REPORTED_KEYS&#34; followed by for part in &#34;${_parts[@]}&#34; expands a declared-but-empty array under set -u (:29). On bash before 4.4 (including macOS's system bash 3.2, which this repo supports elsewhere - see the gtimeout/shasum-first fallbacks in bin/fm-timeout-lib.sh and bin/fm-check-lib.sh) that is an "unbound variable" fatal error and the non-interactive shell exits, so the check dies before printing or recording anything. Reachable on the most common path: a clean fleet on the first run has REPORTED_KEYS empty, a MERGEABLE PR calls record_remove_key (:449), and the array is empty. Same pattern at :282 in owner_for_project when a secondmate's projects: field is empty. The repo already uses the guard idiom for exactly this (&#34;${arr[@]+\&#34;${arr[@]}\&#34;}&#34; in bin/fm-pr-merge.sh:254, bin/fm-test-run.sh:664); apply it here or return early when the source string is empty.
  • ℹ️ bin/fm-pr-conflict-watch.sh:366 - REPORTED_KEYS only ever shrinks when an open PR is observed MERGEABLE (:449). Keys for PRs that are merged, closed, or drop out of the PR_LIMIT window are never removed, and keys for repos the sweep no longer visits persist forever, so state/.pr-conflict-watch grows without bound (~60 bytes per conflicted head, forever). Unlike the sibling, whose record is the current finding set rebuilt each sweep, this record is accumulate-only. Consider dropping keys whose repo was fully evaluated and whose PR no longer appeared, or aging entries out via the recorded epoch.
  • ℹ️ bin/fm-pr-conflict-watch.sh:217 - repo_slug is a byte-for-byte copy of repo_slug in bin/fm-bearings-snapshot.sh:197-199, including the identical two-stage sed pipeline. Two independent copies of the same GitHub URL -> owner/repo parser will drift (e.g. when GHES hosts or a new URL form need handling), and the repo's own convention for shared shape is a single owner (see the "ONE OWNER" headers in bin/fm-timeout-lib.sh and bin/fm-line-cap-lib.sh). Move it into a sourced lib - bin/fm-pr-lib.sh already owns provider/path parsing - and have both callers use it.
  • ℹ️ bin/fm-pr-conflict-watch.sh:295 - owner_for_repo is called once per repo inside the sweep loop, and each call re-runs firstmate_repo_slug (one git invocation) plus resolve_project_repo for every project until it matches (one git invocation each), re-deriving work discover_repos already did. For N projects that is O(N^2) git spawns per sweep, all charged against the same BUDGET_SECS the GitHub probes need, and none of it is bounded by fm_run_timed. Build the repo->owner map once while discovering repos and look it up, which also removes the second firstmate_repo_slug call site.
  • ℹ️ bin/fm-pr-conflict-watch.sh:455 - On the UNKNOWN path the dedupe key and the reported head= come from the pr list response, but the resolution re-reads the PR with pr view and already requests headRefOid (:401) without using it. If the head is force-updated between the list and the view, the mergeability that gets reported belongs to the new head while the wake line labels it with the old SHA, and the key is recorded against the old head - a wrong head= value routed to the owning team, with no error. Use the view's headRefOid when the view is what resolved the state.
  • ℹ️ docs/configuration.md:422 - The new configuration section is thorough and does document what this does NOT solve, but the repo's two maintained inventories were not updated for the new artifacts: AGENTS.md's state/ catalog has a line for the sibling (tool-updates.check.sh ... its report record .tool-updates ... at AGENTS.md:113) and needs the equivalent for pr-conflict-watch.check.sh, its .check-trust binding, and the .pr-conflict-watch record; docs/scripts.md's bin/ table has a row for fm-tool-update-check.sh (docs/scripts.md:111) and none for fm-pr-conflict-watch.sh.

🔧 Fix: fix PR conflict watch cap suppression, budget overrun, record pruning
5 infos still open:

  • ℹ️ bin/fm-pr-conflict-watch.sh:639 - SWEPT_REPOS only gains a repo when count &lt; PR_LIMIT, so pruning is disabled entirely for any repo whose open-PR page filled the limit, and record_prune then keeps every one of that repo's keys (the repo is still in DISCOVERED_REPOS, so the else branch preserves them). Concrete: a repo with 30+ open PRs (default FM_PR_CONFLICT_PR_LIMIT=30); PR ci: enforce contributor guardrails #5 conflicts at head A -> key owner/repo#5#A recorded; the author force-pushes, it conflicts again at head B -> key #5#B recorded; A is never removed, and every subsequent force-push adds another permanent key. state/.pr-conflict-watch grows one line-entry per historical head forever, which contradicts the claim the fix round added at bin/fm-pr-conflict-watch.sh:24-25 ("cannot grow without bound") and docs/configuration.md:445 ("stays the size of the live conflict set"). The conservative gate is right for absence-based pruning, but a per-number rule is safe even on a truncated page: for any repo+number the page did return, a recorded key with the same repo+number and a different head is definitionally stale (a PR has exactly one head), so those can be dropped regardless of whether the page was complete.
  • ℹ️ bin/fm-pr-conflict-watch.sh:508 - When discover_repos resolves no repositories, the for repo in $DISCOVERED_REPOS loop body never runs, so SWEEP_COMPLETE stays 1 while SWEPT_REPOS and SEEN_KEYS are both empty. record_prune then sends every recorded key down the else branch, finds its repo absent from the empty DISCOVERED_REPOS, and drops it - the whole dedupe record is wiped and rewritten as reported=. The next sweep re-wakes every still-conflicted PR in the fleet, contradicting the intent's "the same conflict wakes once". Reachable whenever discovery fails wholesale rather than per repo: action_check probes only jq (:647) and $GH_AXI (:650), not git, so a watcher PATH without git makes firstmate_repo_slug and every resolve_project_repo return 1; likewise if $PROJECTS is transiently unavailable (stale mount, home being relocated). An unread fleet is not a resolved fleet - gate the DISCOVERED_REPOS-based drop on [ -n &#34;$DISCOVERED_REPOS&#34; ], or set SWEEP_COMPLETE=0 when discovery yields nothing.
  • ℹ️ tests/fm-pr-conflict-watch.test.sh:324 - test_unknown_rereads_stop_at_the_sweep_deadline sets FM_PR_CONFLICT_BUDGET_SECS=1, but DEADLINE is computed at bin/fm-pr-conflict-watch.sh:661 before discover_repos runs, and discover_repos spawns several unbounded git remote get-url calls plus two file reads. If that takes over one second (plausible on a loaded CI box), budget_exhausted fires at the top of the repo loop, evaluate_repo is never entered, the reread loop is never reached, and the test's only assertions - elapsed &lt; 15 and empty output - both hold even if the entire deadline guard were reverted. Unlike test_unknown_never_reported, which pins behaviour with the fake's .seq cursor, this test has no evidence the loop was entered at all. Use write_view_sequence here too and assert the cursor advanced but stayed below FM_PR_CONFLICT_UNKNOWN_ATTEMPTS=10, so the test proves the loop ran and was cut short by the deadline rather than never running.
  • ℹ️ bin/fm-pr-conflict-watch.sh:472 - record_remove_key is called for every MERGEABLE PR (:629) and unconditionally runs while ... done &lt; &lt;(list_parts &#34;$REPORTED_KEYS&#34; &#39;;&#39;), which forks a subshell even when REPORTED_KEYS is empty - the normal state of a clean fleet. With the default PR_LIMIT=30 across a multi-repo fleet that is one wasted fork per open PR on every sweep, charged against the same BUDGET_SECS the GitHub probes need and inside the deadline the fix round just tightened. record_has_key &#34;$key&#34; || return 0 at the top makes the no-op case free and leaves behaviour identical.
  • ℹ️ bin/fm-pr-conflict-watch.sh:302 - DEADLINE is set at :661 and discover_repos runs after it, but the git -C &#34;$dir&#34; remote get-url origin call it makes per project (and the one in firstmate_repo_slug, :289) is the only external command in the sweep not wrapped in fm_run_timed, and there is no budget_exhausted check between projects. The fix round's own rationale for bounding the UNKNOWN loop applies identically here: this check prints and records only at the very end (:699-702), so a discovery phase that stalls past FM_CHECK_TIMEOUT gets the process group killed and the sweep loses everything, repeating identically on every poll. A clone on a hung or stale mount is enough. The script already sources bin/fm-timeout-lib.sh, so wrapping the two origin reads in fm_run_timed &#34;$(probe_bound)&#34; (and breaking discovery on budget_exhausted) closes it with no new machinery.
✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-pr-conflict-watch.test.sh (12 fixture cases: newly conflicted wake with routing fields, same-head silence, new-head re-wake, persistent UNKNOWN never reported, UNKNOWN settling on CONFLICTING, reread-head labelling/dedupe, line-cap conflicts waking on a later sweep, record pruning, UNKNOWN rereads stopping at the sweep deadline, clean-fleet silence, draft conflicts, arm registration) - run twice to check for flakiness, all pass both times
  • bash tests/fm-bearings-snapshot.test.sh (31 cases) - the other consumer of the newly extracted bin/fm-repo-slug-lib.sh, all pass
  • Manual end-to-end: /home/rich/.no-mistakes/evidence/01M0VCRA29HFS9SR262BP9YW00/pr-conflict-watch-e2e.sh - arms the check in a throwaway FM_HOME, then runs the real bin/fm-watch.sh check sweep against a fake gh-axi over eight scenarios (clean fleet, three routed conflicts including a draft and the firstmate repo, same-head silence, force-updated head, UNKNOWN, resolved, disarm), draining each wake through bin/fm-wake-drain.sh
  • FM_STATE_OVERRIDE=&lt;home&gt;/state bin/fm-check-register.sh pr-conflict-watch - verified the armed shim bytes against the trust binding
⚠️ **Document** - 1 info
  • ℹ️ docs/fm-test-portable-shards.md:76 - This change adds tests/fm-pr-conflict-watch.test.sh, which lands in the portable serial lane (it appears in portable-serial-2of4) with no duration hint, so it takes PORTABLE_SERIAL_DEFAULT_WEIGHT_MS. The doc's shard table still reads 29/28/30/30 scripts while the lanes now hold 30/32/32/32, and the doc's own rule says "Refresh the hints whenever the serial lane gains scripts". I did not fix it: the table was already stale by eight scripts at the base commit (this change accounts for one of them), and a correct refresh needs measured per-script timings downloaded from a green CI run plus an edit to the portable_serial_weight_hints table in bin/fm-test-run.sh, which is executable code this documentation phase must not touch. Coverage is unaffected (the coverage guard keeps the partition complete and disjoint); only shard balance drifts. Worth a follow-up that runs the refresh procedure the doc already spells out.
✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Detect newly conflicted open PRs from registry-derived repos, dedupe by head SHA, poll lazy GitHub mergeability safely, and arm through the standard registered check path.
@greptile-apps

greptile-apps Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the current discovery-completeness tracking and pruning guards address the previously reported empty, partial, and missing-registry dedupe-state loss paths.

Reviews (5): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile

Comment thread bin/fm-pr-conflict-watch.sh Outdated
Comment thread bin/fm-pr-conflict-watch.sh Outdated
Comment thread bin/fm-pr-conflict-watch.sh
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: this account has been flagged as attempting malicious activity and can no longer contribute to any of Kun's repos. Closing this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants