Skip to content

feat: detect fleet-wide pull request conflicts - #3044

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

feat: detect fleet-wide pull request conflicts#3044
bingb0t5 wants to merge 84 commits into
kunchenguid:mainfrom
bingb0t5:fm/fm-pr-conflict-watch-fork

Conversation

@bingb0t5

Copy link
Copy Markdown

Intent

Build automatic detection and routing of pull-request merge conflicts across every repository this fleet works in. The captain asked for this after conflicts hit several PRs twice in one day; it is a proven repeated need.

A registered check polls open GitHub pull requests of every repository this home works in, detects genuinely conflicted ones, and wakes firstmate with one routed actionable line. Repositories are derived from data/projects.md project clones plus this firstmate checkout's origin remote; owning teams come from data/secondmates.md. Do not hardcode a repository list.

Hard requirements still in force:

  1. Dedupe by repository, PR number, AND head SHA. Same conflict on the same head stays silent after the first wake; a force-updated head that conflicts again is a new event. Keys are recorded only for conflicts that reached the printed line, so line-cap omissions wake later instead of being lost.
  2. GitHub lazy mergeability: UNKNOWN is never treated as clean or conflicted. Reread only those PRs until the state settles or stays unknown.
  3. Print nothing when there is nothing new; one line when firstmate should wake; finish inside FM_CHECK_TIMEOUT.
  4. Cheap: bound API calls, use gh-axi as the fleet standard. gh-axi has no --json flag and answers with an axi envelope, not raw JSON. One GraphQL read per repository carries the open set with mergeability; do not REST-read each PR (that round-2 approach exhausted the sweep budget and created the coverage cascade). A truncated envelope is a failed read, not a short list. A GraphQL null repository is a failed read, not an empty one.
  5. Include drafts; mark draft=yes rather than skipping them.
  6. Register with bin/fm-check-register.sh. Detection and routing only: never resolve conflicts, force-push, rebase, or call GitHub's update-branch.

Coverage is a separate ledger from conflicts (Sol diagnosis 2026-08-25). A conflict is a positive observation keyed by repo + number + head. A coverage gap is an absence of trustworthy observation keyed by a stable target defined BEFORE any GitHub read: repo:<owner/repo> for a valid slug, project: when a clone or origin cannot resolve to a valid repo, and source:projects-registry or source:firstmate-origin when discovery fails before a repo can be named. Never persist or interpolate an unvalidated raw origin slug.

Typed outcomes are observed vs unobserved with cause discovery | invalid-origin | budget | github | truncated. Sweep completeness is derived from those outcomes: complete only when every expected target was observed AND discovery itself completed. One continuous gap record per target keeps opened_at and disclosed across cause flaps; changing cause never resets age and never opens a second gap. Disclose once after FM_PR_CONFLICT_UNREAD_GRACE_SECS (0 = first sweep) as coverage-hole target=... latest-cause=...; local identity refusal, budget cut, and truncated envelopes must not be worded as GitHub reads failing. Recovery then a later outage is a new gap. Typed notification queue acks by identity (conflict key / coverage disclosed), never by substring-scanning the rendered line. Prefix remains pr-conflict:.

PR transport is GraphQL --jq to a JSON array, not TSV. Sanitize tabs and newlines only at format time so titles keep a literal backslash.

This rebuild started from validated pipeline head 46b6866b (GraphQL batching already landed). Do not revert to c382a2d. Do not patch the four parked coverage findings one by one; the coverage subsystem was rebuilt as a ledger instead of hole-kinds.

Delivery: keep the fork-targeted route. The pipeline must open the PR against bingb0t5/firstmate base main. This home cannot merge the upstream kunchenguid PR 3010 and its workflows need an admin approval we do not have, so that is not delivery. Do not hand-create a fork PR and do not forge an attestation.

PR body must state: detection is a safety net not a cure (conflicts happen because PRs wait unmerged while main moves); round-2 REST-per-PR created the budget cascade; arm/shim duplication with fm-tool-update-check.sh is a noted follow-up, not this PR.

Tests must exercise behavior through the executable and must never assert implementation-source bytes. One sentence per line in tracked Markdown. No agent name as a commit co-author.

What Changed

  • Add an armable fleet-wide PR conflict watcher that discovers repositories and owners from fleet records, batches open PR mergeability through GraphQL, rereads UNKNOWN states, and deduplicates notifications by repository, PR number, and head SHA.
  • Track unobserved repositories in a separate coverage ledger with typed causes, grace-period disclosure, recovery handling, bounded sweeps, and safe GitHub origin parsing shared with bearings.
  • Document that detection is a safety net, not a cure: conflicts arise while PRs wait as main moves; the round-2 REST-per-PR design caused the budget cascade, while consolidating arm/shim duplication with fm-tool-update-check.sh remains a follow-up.

Risk Assessment

✅ Low: The implementation is well-bounded and no material correctness defect was found, but one changed global instruction still overstates the dedupe pruning guarantee.

Testing

Diff inspection, focused executable suites, and a direct origin-parser demonstration passed, exercising safe slug derivation, typed refusals, conflict routing and deduplication, lazy mergeability, coverage-ledger notifications, dependency recovery, registration, and the Bearings integration. No visual artifact was captured because this is a shell/CLI change with no rendered UI.

Evidence: Structural origin parser demonstration

Source: Structural origin parser demonstration

credential-https status=ok slug=acme/alpha ssh-explicit-port status=ok slug=acme/alpha spoofed-host status=unsupported-host slug=<none> unsupported-forge status=unsupported-host slug=<none>

credential-https status=ok slug=acme/alpha
ssh-explicit-port status=ok slug=acme/alpha
spoofed-host status=unsupported-host slug=<none>
unsupported-forge status=unsupported-host slug=<none>

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

🔧 **Rebase** - 1 issue found → auto-fixed ✅
  • ⚠️ docs/scripts.md - merge conflict rebasing onto origin/main

🔧 Fix applied.
✅ Re-checked - no issues remain.

⚠️ **Review** - 1 info
  • 🚨 bin/fm-pr-conflict-watch.sh:1069 - Notifications are acknowledged before stdout is written. If printf fails, for example because the watcher output pipe closes, conflict keys are still persisted and coverage gaps marked disclosed, so the notification never retries despite not reaching the printed line. Print successfully before calling ack_delivered; otherwise leave identities unacknowledged.
  • ⚠️ bin/fm-pr-conflict-watch.sh:964 - An UNKNOWN list result reread uses the settled head but retains the stale list-time draft flag. Concrete case: the list says isDraft=false, mergeable=UNKNOWN, then the reread says isDraft=true, mergeable=CONFLICTING; the wake reports the judged head with draft=no. Carry the reread's isDraft through pr_mergeable_resolved alongside the settled head.
  • ⚠️ bin/fm-repo-slug-lib.sh:18 - The parser matches github.com anywhere instead of validating the URL host. For example, https://notgithub.com/acme/alpha.git resolves to acme/alpha, causing the watcher to query and potentially report an unrelated public GitHub repository rather than record an invalid-origin gap. Anchor accepted HTTPS/SSH forms to the exact GitHub host at this shared parsing boundary.

🔧 Fix: Fix conflict delivery, reread metadata, and repository parsing
2 issues (1 warning, 1 info) still open:

  • ⚠️ bin/fm-pr-conflict-watch.sh:465 - The required diagnostic invariant says a local identity refusal must use latest-cause=invalid-origin, but an invalid firstmate origin such as https://gitlab.com/acme/firstmate.git makes fm_repo_slug return empty and is classified here as discovery. Preserve whether git remote get-url failed versus whether parsing/validation rejected the returned URL, then assign discovery or invalid-origin accordingly.
  • ℹ️ docs/configuration.md:471 - This states that a cut-budget notice is the only output without a conflict, but the newly documented coverage-hole notifications are also emitted independently of conflicts. Correct the sentence so the documentation matches the implemented notification model.

🔧 Fix: Distinguish origin failures and correct coverage documentation
1 warning still open:

  • ⚠️ bin/fm-pr-conflict-watch.sh:483 - The required typed-outcome contract distinguishes discovery from invalid-origin, but project origin lookup failures are collapsed into invalid-origin. Concrete path: a registered project's clone exists but has no origin; resolve_project_repo fails, line 480 replaces the result with an empty slug, and line 483 records latest-cause=invalid-origin. This contradicts the requirement that lookup/discovery failure use discovery, while only a present rejected URL uses invalid-origin. Return a typed resolution outcome from resolve_project_repo so the target remains project:&lt;name&gt; with the correct cause.

🔧 Fix: Preserve typed project origin failure causes
1 warning still open:

  • ⚠️ bin/fm-repo-slug-lib.sh:21 - The intent requires coverage across every repository derived from clone origins, but this SSH parser only accepts the no-port form. A valid remote such as ssh://git@github.com:22/acme/alpha.git produces no slug and is permanently classified as project:&lt;name&gt; with latest-cause=invalid-origin, so that repository is never polled. Accept an optional SSH port while continuing to require the exact github.com host.

🔧 Fix: Accept port-qualified GitHub SSH origins
1 error still open:

  • 🚨 bin/fm-pr-conflict-watch.sh:1005 - The required typed-outcome model says targets must become observed or unobserved with an allowed cause, but an armed watcher returns silently before discovery when jq or gh-axi later becomes unavailable. This produces no coverage gap, never advances the ledger, and can remain indistinguishable from a clean fleet forever. Decide which stable source target and permitted cause should represent a missing runtime dependency, then route these guards through that coverage boundary instead of returning success.

🔧 Fix: Track missing runtime dependencies as coverage gaps
2 errors still open:

  • 🚨 bin/fm-pr-conflict-watch.sh:501 - A recovered target-specific discovery gap is only pruned when every other discovery also succeeds. Concrete path: projects alpha and beta both have disclosed project:* gaps; alpha's origin recovers while beta remains invalid, so DISCOVERY_COMPLETE stays 0 and coverage_prune retains alpha's old disclosed gap. If alpha fails again before beta recovers, it reuses that disclosed gap and never notifies, violating the recovery-then-new-gap invariant. Close each source/project gap when that exact source or project resolves, while retaining unrelated gaps.
  • 🚨 bin/fm-pr-conflict-watch.sh:1112 - Runtime dependency recovery is cleared only in memory before the interval throttle. If jq returns less than FM_PR_CONFLICT_INTERVAL after a missing-dependency run, line 1117 returns without persisting the cleared state; a second jq outage reloads the old disclosed gap and remains silent indefinitely. Persist dependency recovery before this early return, and do the same for gh-axi.

🔧 Fix: Persist independent coverage recovery lifecycle
1 error still open:

  • 🚨 bin/fm-repo-slug-lib.sh:19 - The required coverage of "every repository this fleet works in" is incomplete for valid GitHub HTTPS origins containing userinfo. For example, https://x-access-token:TOKEN@github.com/acme/alpha.git has the exact github.com host and is accepted by Git, but this pattern returns no slug, so discovery permanently records project:&lt;name&gt; with latest-cause=invalid-origin and never polls that repository. Parse HTTPS URLs by validating the actual host while allowing optional userinfo, without persisting the raw credential-bearing URL.

🔧 Fix: Build structural GitHub origin parser
1 info still open:

  • ℹ️ AGENTS.md:116 - This global instruction says the dedupe record is always cut back to conflicts each sweep still observes, but repositories whose open set reaches FM_PR_CONFLICT_PR_LIMIT intentionally retain older keys because their page is incomplete. Correct the sentence or include that exception so future agents do not rely on a false record-size invariant.
✅ **Test** - passed

✅ No issues found.

  • Inspected the changed-file scope with git diff --stat 6a2cd6cd2c642c9422bb43dccedbd91103285fb0..a0c4c7aaeac73c0ac6682d167a579566173ac59e
  • tests/fm-repo-slug.test.sh
  • tests/fm-pr-conflict-watch.test.sh
  • tests/fm-bearings-snapshot.test.sh
  • Invoked fm_repo_slug_parse directly with credential-bearing HTTPS, explicit-port SSH, spoofed-host, and unsupported-forge origins
  • Verified cleanup with git status --short
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Fix ShellCheck declarations and intentional lint annotations
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

…er check

The adjacent-follow-up E2E case captured the tmux pane for its
duplicate-captain-answer assertion immediately after the session file
confirmed processing, with no settle wait, unlike every other
readiness check in this test. Sending two followUp deliveries queues
more Calm presentation work (an extra operational-user row plus its
hiding invalidation) than a single one, so the already-settled captain
answer's redraw could still be in flight at that instant, making the
check flaky. Poll the pane the same way the session-file wait already
does, and track the peak count seen along the way so a captain answer
that is genuinely rendered twice for even one frame still fails even
if a later redraw were to self-correct.
…ss gates

Adds the two triggers from data/fm-auto-stow/report.md so the captain no
longer has to type /stow to keep memory current, without a new daemon,
watcher, or cascade:

- bin/fm-session-start.sh prepends a STOW DUE line to a compact/clear
  session-start re-emit when state/.last-stow is missing or older than
  FM_AUTO_STOW_INTERVAL_SECS (default ~24h), silent when current.
- AGENTS.md section 8 rule 4 now also checks that same marker on a
  heartbeat wake, using the same larger-than-heartbeat interval, so a
  pass runs at most once per interval rather than on every wake.
- The stow skill touches state/.last-stow only at the end of a pass it
  can call reset-safe, mirroring state/.last-heartbeat's bare-mtime
  marker.

Away-mode heartbeats stay bash-only and unaffected: they never reach an
LLM turn to run /stow in, per the existing away-daemon design.
Vendor the lalo-admin assessor already proven on mrbeanz-brains, with the
same drift pin and live SoT comparison, so firstmate PRs cannot skip the
required overview, decision, module-boundary, and validation sections.
Promote the last-stow marker contract out of the cascade heading so a
secondmate home still throttles automatic /stow. Pin the stale re-emit
age assertion to a 9xxxx band instead of the prefix 900. Document that
default Pi branch supervision does not run heartbeat /stow, and leave
that wiring as follow-up (kunchenguid#2944) rather than
editing fm-branch-prompt.sh.
Adds a stdlib-only Python server that shells quota-axi --json per
request and serves one self-contained page (fleet summary, one card
per provider, live/signed-out/error states in quota-axi's own words,
30s client refresh), matching the design in
data/fm-quota-dashboard/report.md. Binds only to this host's own
Tailscale IPv4 address, confirmed via `tailscale ip -4`, and refuses
to start otherwise - never 0.0.0.0, never a public interface.
Raise the shell-out to 45s so one slow provider degrades instead of
blanking the page. Prove the listener is address-scoped via 127.0.0.2,
and keep the scripts index voice cluster intact.
Checking out the base branch cannot execute the new checker until it
lands, so restore pull_request and assess the proposed head instead.
fix(test): settle Pi follow-up pane before the duplicate-captain-answer check
feat(bin): serve quota dashboard on the tailnet
Vendor the lalo-admin assessor already proven on mrbeanz-brains, with the
same drift pin and live SoT comparison, so firstmate PRs cannot skip the
required overview, decision, module-boundary, and validation sections.
Conflict detection already worked; four review rounds patched the same
coverage-disclosure hole. A gap is now keyed by a stable target with a
frozen opened_at, cause is metadata that never resets age, and wake
lines ack by identity rather than by scanning the rendered text.
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: first look on current main 6a2cd6cd2c64 (#3024). Never messaged the captain.

class=opt-in. Fleet-wide conflict detection does not poll until bin/fm-pr-conflict-watch.sh arm writes state/pr-conflict-watch.check.sh and binds it with fm-check-register.sh. Absent the shim, the watcher never runs this check. check is operator-invoked; disarm removes shim, trust, and dedupe record. Same arming shape as fm-tool-update-check.sh. Not default-behavior. Never auto this pass.

VISION.md (inspected bin/fm-pr-conflict-watch.sh action_arm / action_disarm / GraphQL list+reread, bin/fm-repo-slug-lib.sh fm_repo_slug_parse host+userinfo, bin/fm-bearings-snapshot.sh slug swap, docs/configuration.md arm docs, tests/fm-pr-conflict-watch.test.sh / tests/fm-repo-slug.test.sh, AGENTS.md shim note):

  • One captain, one interface: aligns (one routed pr-conflict: / coverage-hole line; silence when nothing new; detection not a captain-facing cure).
  • Authority is explicit: aligns as opt-in (arm is the grant; no bootstrap auto-arm in this diff).
  • Scripts own the mechanics: aligns (mergeability is GitHub state, not meaning; UNKNOWN is never inferred clean or conflicted).
  • A restart is a non-event: aligns (dedupe + coverage ledger on disk, keyed by repo+number+head / stable target).
  • Delegation with a spine: aligns (detection and routing only; never rebase, force-push, or update-branch).
  • The fleet outlives any vendor: aligns (gh-axi GraphQL; github.com host only).
  • Scope: aligns (command-layer check, not forge merge policy).

This HEAD: 084ef75937c4f8bd050d08c94ae96a6605bf0d23. MERGEABLE / UNSTABLE, ahead 19 / behind 0.
Attestation matches THIS HEAD (no-mistakes-pipeline-attestation:v1 084ef759…).
CI: fork Actions on THIS HEAD are action_required (CI 32833422163, Require no-mistakes 32833422154) — not green. Greptile 5/5 is not a merge gate. Author body does not use a GitHub closing link.

Not first-time: bingb0t5 has had Behavior/Lint/CI actually run on prior PRs. No workflow approval this pass.

Security: none that blocks. After arm, polls open PRs via existing gh-axi GraphQL with charset-validated slugs; origin userinfo is parsed then discarded (not persisted, not sent); GIT_TERMINAL_PROMPT=0; never resolves conflicts. Tests pin bearings must not send origin userinfo.

Overlap: successor of open #3010 (same author, earlier watcher; this body says keep the fork route). AGENTS.md / docs/configuration.md also sit on #3042 / #2801. Shared fm-repo-slug-lib.sh used from bearings. Not lock / spawn-freshen / herdr / #2804.

Land-eligible rec: NO (CI not green). Captain-flag NOW: no.

This is waiting-on-author for green Actions on THIS HEAD. Not a captain-decision. Not a merge I will recommend.

@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