From a8fcea8871d2ae32037e1616f5e633e5559c817c Mon Sep 17 00:00:00 2001 From: sramkrishna Date: Wed, 23 Sep 2026 22:33:16 +0000 Subject: [PATCH] docs(contributing): add self-collision preflight draft for intra-lane duplicate PRs The hold-gated preflight lists open PRs so a lane can prove its new filing is disjoint, yet the quality lane opened two duplicate pairs covering identical file clusters within three days (finpilot#285/#303, finpilot#296/#299). All four are resolved as of 2026-09-23 -- three merged, one closed -- so the queue absorbed the collision; no rule prevented the second filing. Re-auditing the 31 open app-authored PRs across the ten repositories tracked in common#1058 on 2026-09-23 found no confirmed intra-lane duplicate cluster. Every file-cluster intersection that exists is legitimate work: distinct Renovate digest bumps sharing image-versions.yml (bluefin#1227/#1310) and build.yml (server#238/#239), a cosign fix and an SSOT refactor both touching justfile (dakota-iso#142/#146), and two human PRs closing different issues that share Justfile (common#1186/#1187). The prohibition as filed -- no PR whose file cluster intersects the lane's own open PRs -- would therefore have blocked four legitimate filings while catching neither real duplicate. This draft proposes the evidenced alternative: intersect the lane's own open PRs before filing, compare hunks and closing-issue sets rather than shared paths, and either comment on the existing PR or disclose the overlap in the new body. Drafted rather than applied because it binds filing behavior org-wide, matching the precedent set for common#1043 (hold-gate-rubric.md) and common#1052 (agent-lane-throttle.md). Closes #1060 Hive-Run: projectbluefin/common#1060 Hive-Plan: docs/contributing/self-collision-preflight.md Hive-Spec: common#1060#proposed-next-step Assisted-by: deepseek/deepseek-v4.1-flash via Hive Signed-off-by: sramkrishna --- CONTRIBUTING.md | 3 + docs/contributing/self-collision-preflight.md | 165 ++++++++++++++++++ docs/factory/agentic-model.md | 5 + .../pr-review/references/duplicate-cluster.md | 12 ++ 4 files changed, 185 insertions(+) create mode 100644 docs/contributing/self-collision-preflight.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2be6f560a..cc690d304 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,3 +70,6 @@ Full layer validation (the `common` behave suite from - [`docs/contributing/agent-lane-throttle.md`](docs/contributing/agent-lane-throttle.md) — draft proposal for demand-side throttling of agent-filed PRs under review backlog; unadopted until a maintainer decision. +- [`docs/contributing/self-collision-preflight.md`](docs/contributing/self-collision-preflight.md) — + draft proposal for a preflight step that checks a lane's own open PRs for file + cluster overlap before it opens another; unadopted until a maintainer decision. diff --git a/docs/contributing/self-collision-preflight.md b/docs/contributing/self-collision-preflight.md new file mode 100644 index 000000000..e39500c2e --- /dev/null +++ b/docs/contributing/self-collision-preflight.md @@ -0,0 +1,165 @@ +# Self-Collision Preflight — Contribution Strategy (Planning Draft) + +> **Status**: hold-gated planning artifact. Human review required before adoption. +> Filed by the strategist agent. Tracks [common#1060](https://github.com/projectbluefin/common/issues/1060); +> pairs with the hold-gate prioritization rubric +> ([common#1043](https://github.com/projectbluefin/common/issues/1043), +> [`hold-gate-rubric.md`](hold-gate-rubric.md)) and the demand-side throttle +> ([common#1052](https://github.com/projectbluefin/common/issues/1052), +> [`agent-lane-throttle.md`](agent-lane-throttle.md)). +> +> Nothing below changes agent filing behavior, workflow behavior, or hold-gate +> enforcement until a maintainer adopts it. It proposes one preflight step an +> agent evaluates at developer time; it adds no CI check, no label, and no +> automation. + +## Vocabulary + +- **Agent lane** — as defined in + [`agent-lane-throttle.md`](agent-lane-throttle.md#vocabulary): a class of agent + work identified by who files it and what it produces, not by a workflow label. +- **Self-collision** — two open PRs from the *same* lane whose file clusters + intersect. Distinct from a cross-lane competing pair, where two different lanes + file against the same ground. +- **File cluster** — the set of paths a PR changes, read from the PR itself: + `gh pr view --json files`. + +## Problem + +The hold-gated protocol lists open PRs at kick time precisely so a lane can prove +its new filing is disjoint from work already in flight. Within one lane that +check failed: the quality lane opened duplicate PRs covering identical clusters +while its own earlier PR sat in the hold queue. + +| Earlier PR | Duplicate PR | Shared cluster | Gap | +|---|---|---|---| +| finpilot#285 (2026-08-27) | finpilot#303 (2026-08-30) | `build/clean-stage.sh` + `build/copr-helpers.sh` BATS tests, both touch `Justfile` | 3 days | +| finpilot#296 (2026-08-30 07:48Z) | finpilot#299 (2026-08-30 09:52Z) | `Justfile` `tag-images` recipe BATS coverage | 2 hours | + +Each pair cost four review slots for two pieces of work. A lane that re-claims +ground it already holds makes queue depth overstate real work-in-flight, and +forces a reviewer to adjudicate which duplicate to keep — a comparison the lane +had the evidence to make at kick time and the reviewer does not. + +### What the queue has already absorbed + +All four PRs are resolved as of 2026-09-23, so the named collisions are no +longer open work: + +| PR | Outcome | +|---|---| +| finpilot#285 | merged 2026-09-04 | +| finpilot#296 | merged 2026-09-05 | +| finpilot#299 | merged 2026-09-06 | +| finpilot#303 | closed 2026-09-06 without merge | + +The queue did adjudicate them — at the cost this proposal exists to avoid, not +through any rule that prevented the second filing. + +### Re-audit: the naive rule would catch the wrong thing + +[common#1060](https://github.com/projectbluefin/common/issues/1060) proposes +"a lane may not open a PR whose file cluster intersects its own open hold-gated +PRs." Re-auditing every open app-authored PR across the ten repositories tracked +in [common#1058](https://github.com/projectbluefin/common/issues/1058) on +2026-09-23 (**31 open app-authored PRs**) found **no confirmed intra-lane +duplicate cluster**. Every file-cluster intersection that exists is legitimate +work that a hard prohibition would have blocked: + +| Intersecting pair | Same lane? | Why it is not a duplicate | +|---|---|---| +| bluefin#1227 / bluefin#1310 | `app/mergeraptor` | Distinct dependency digest bumps (`projectbluefin/common:latest` vs `ublue-os/brew:latest`) that share `image-versions.yml` | +| server#238 / server#239 | `app/mergeraptor` | Distinct dependency digest bumps (`projectbluefin/actions` vs `taiki-e/install-action`) that share `.github/workflows/build.yml` | +| dakota-iso#142 / dakota-iso#146 | `app/hivecommons-hive` | A cosign-verification security fix and a host-side config SSOT refactor that both touch `justfile` | +| common#1186 / common#1187 | human (`repires`) | Two human PRs closing different issues (#1161, #1160) that both touch `Justfile` and `.github/workflows/unit-tests.yml` | + +The discriminator in the real finpilot duplicates was not the shared file — it +was the shared *intent and closing issue*. This matches the rule the factory +already applies at review time: the same file is not the same work, and a +competing pair is a candidate for resolution only after the actual hunks are +compared +([`common-rationalizations.md`](../skills/pr-review/references/common-rationalizations.md), +[`duplicate-cluster.md`](../skills/pr-review/references/duplicate-cluster.md)). + +A prohibition keyed on file intersection alone would therefore have blocked four +legitimate filings while catching zero of the two real duplicates. The gap is +not the absence of a prohibition — it is the absence of a *required, evidenced +comparison* before the second PR is opened. + +## Proposal: one preflight step + +Before opening a PR, a lane must intersect the files it intends to change +against the open PRs it already holds on the target repository, and record the +result. + +**1. List this lane's own open PRs with their file clusters.** + +```bash +gh pr list --repo projectbluefin/ --state open --author \ + --json number,title,files \ + --jq '.[] | "\(.number)\t\(.title)\t\([.files[].path] | join(","))"' +``` + +**2. Intersect** that output against the paths in the new change. + +**3. On intersection, compare the hunks and the closing-issue sets** — not the +titles, and not the shared path. `gh pr diff ` works for fork heads. + +- **Same work already open** → comment on that PR. Do not open a second one. +- **Different work, overlapping files** → name the other PR and the shared path + in the new PR body, so the reviewer can order the two instead of discovering + the overlap mid-sweep. + +**4. Record the result in the new PR body** — either the explicit "no overlap +with this lane's open PRs" statement or the named overlap from step 3. An +unrecorded check is indistinguishable from a skipped one. + +This is deliberately a *disclosure* rule, not a prohibition. It leaves the +"is this the same work?" judgment with the lane, which is where the evidence is, +and it makes that judgment visible to the reviewer rather than silently +duplicated. + +## Compliance with the seven-label contract + +The preflight is a **filing-time convention, not a workflow state**: + +- It adds no label, removes no label, and changes no transition in + [`label-workflow.md`](../skills/label-workflow.md). +- It introduces no new overlay and no new queue. +- It is evaluated by the agent at the moment it decides whether to open a PR — + the same class as the attribution trailers in + [`agentic-model.md`](../factory/agentic-model.md) — not by a CI check. Per that + same contract, a process convention must not become a bespoke blocking CI job. + +## Operating metrics + +Draft measures, to be baselined at adoption: + +- **Self-collision pairs opened per week** — baseline: 2 pairs in 3 days + (finpilot, 2026-08-27 → 2026-08-30); 0 confirmed org-wide on 2026-09-23. +- **Review slots spent adjudicating self-collisions** — baseline: 4 slots for 2 + work items in the finpilot pairs. Target 0. +- **Preflight disclosure rate** — share of new agent PRs whose body records the + intersection result. A rule that is never recorded is not being run. + +## What this document deliberately does not do + +- No closure, relabelling, or reverting of any currently open PR. The four + finpilot PRs were dispositioned by review; nothing here reopens that. +- No change to the hard rules in + [`agentic-model.md`](../factory/agentic-model.md), and no change to any lane's + runtime behavior, until a maintainer adopts it. +- No new script, workflow, CI gate, or label. +- No duplication of the review-time resolution procedure — that stays + [`duplicate-cluster.md`](../skills/pr-review/references/duplicate-cluster.md), + which remains human-gated. + +## Related + +- [common#1060](https://github.com/projectbluefin/common/issues/1060) — self-collision finding (this doc's tracker) +- [common#1043](https://github.com/projectbluefin/common/issues/1043) — hold-gate prioritization rubric ([`hold-gate-rubric.md`](hold-gate-rubric.md)) +- [common#1052](https://github.com/projectbluefin/common/issues/1052) — agent-lane output throttle ([`agent-lane-throttle.md`](agent-lane-throttle.md)) +- [common#1054](https://github.com/projectbluefin/common/issues/1054) — obsolescence detection for superseded hold-gated PRs +- [common#1058](https://github.com/projectbluefin/common/issues/1058) — reviewer coverage map / repo-concentration data +- [`duplicate-cluster.md`](../skills/pr-review/references/duplicate-cluster.md) — review-time resolution procedure +- [`common-rationalizations.md`](../skills/pr-review/references/common-rationalizations.md) — why a shared file is not proof of duplication diff --git a/docs/factory/agentic-model.md b/docs/factory/agentic-model.md index b4990dd18..9c3aef960 100644 --- a/docs/factory/agentic-model.md +++ b/docs/factory/agentic-model.md @@ -33,6 +33,11 @@ cross-repository breakage, merge, or production human gates. - **Check for existing PRs before opening.** Before creating a branch for any issue, run: `gh pr list --repo projectbluefin/ --state open --search ""` If an open PR already covers the work, comment on it rather than opening a duplicate. + The topic search does not cover the lane's *own* open PRs — for the proposed + self-collision preflight that intersects this lane's file clusters before it + files again, see the draft + [`self-collision-preflight.md`](../contributing/self-collision-preflight.md) + ([common#1060](https://github.com/projectbluefin/common/issues/1060)). - **Ask before opening PRs.** Do not open PRs autonomously. Present the plan and the diff, get explicit human approval, then open. Exception: Renovate bot PRs are pre-approved. - **`just check` before every commit** in repos that have a Justfile. - **`pre-commit run --all-files` before every commit** in repos with `.pre-commit-config.yaml`. diff --git a/docs/skills/pr-review/references/duplicate-cluster.md b/docs/skills/pr-review/references/duplicate-cluster.md index 59dd4c052..15a7885da 100644 --- a/docs/skills/pr-review/references/duplicate-cluster.md +++ b/docs/skills/pr-review/references/duplicate-cluster.md @@ -8,6 +8,18 @@ confirms that it is the same work and names the survivor. Resolve a confirmed cluster as a unit, halting on the first failure. +## Prevention — the author-time check + +Everything below resolves a collision a reviewer has already paid for. The +cheaper fix is one step earlier: before opening a PR, a lane intersects the +files it intends to change against the open PRs it *already holds* on that +repository, and either comments on the existing PR or names the overlap in the +new PR body. That step is drafted, not yet adopted, in +[`self-collision-preflight.md`](../../../contributing/self-collision-preflight.md) +([common#1060](https://github.com/projectbluefin/common/issues/1060)); the +prohibition it replaces was rejected because a shared file is not proof of +duplication. Nothing in this section changes the human gate on this page. + ## Procedure **1. The human confirms the duplicate and names the survivor.**