|
| 1 | +# Fork & contribution discipline |
| 2 | + |
| 3 | +> The generic rules for contributing a fix upstream through a fork — the **Check |
| 4 | +> closing / contribution** discipline (the counterpart to [`principles.md`](principles.md), |
| 5 | +> which governs Plan-time solution design). These rules are project-agnostic; **your |
| 6 | +> project's concrete answers** — which branches, which remotes, which validation |
| 7 | +> runners — live in [`INTEGRATION.md`](INTEGRATION.md) (§2 branch targets, §5 |
| 8 | +> upstream-isn't-ahead, §8 committing/PR, §10 maintainer). The cycle consults this |
| 9 | +> file: the planner, builder, reviewer, and publisher are each pointed here for the |
| 10 | +> rules they act on. Maintained as a generic reference — record a project specific in |
| 11 | +> `INTEGRATION.md`, not here. |
| 12 | + |
| 13 | +The split this file assumes: **the rules are generic and live here; the answers are |
| 14 | +per-project and live in `INTEGRATION.md`.** Where a rule needs a concrete value |
| 15 | +(a branch name, a remote, a runner), it says *"(instance: INTEGRATION.md §N)"* — fill |
| 16 | +it there, not by editing this file (so `copier update` keeps the rules current). |
| 17 | + |
| 18 | +## 1. Fork mechanics |
| 19 | + |
| 20 | +- **Remote layout.** `upstream` = the canonical project (read-only, the contribution |
| 21 | + target); `origin` = **your fork** (where branches are pushed). The fork owner is |
| 22 | + distinct from the upstream owner. *(instance: the actual repos/owner — INTEGRATION.md §5.)* |
| 23 | +- **Branch from `upstream/<base>`, never the fork's tracking branch.** A fork's |
| 24 | + `maintenance/*` / default branches **drift** — they carry local CI/tooling commits |
| 25 | + that must not ride into the contribution. Always `fetch upstream` and branch off |
| 26 | + `upstream/<base>` so the diff is exactly the fix. *(A stray tooling file riding in on |
| 27 | + a fork-based branch is the classic symptom.)* |
| 28 | +- **A fork-based PR's `--head` is `OWNER:BRANCH`.** The branch lives on the fork |
| 29 | + (origin); `gh pr create --head <bare-branch>` resolves it against the *base* repo, |
| 30 | + where it doesn't exist, and fails. The publish mechanics handle this deterministically. |
| 31 | +- **Keep the fork checkout clean.** Publish refuses on a dirty checkout. A |
| 32 | + patch-apply-then-commit op must stage **added** files (a new test), not only modified |
| 33 | + ones — `git apply` + `git add --all` + `git commit`, never `commit -a` (which silently |
| 34 | + drops the new file). The same applies to a patch-and-revert validation gate: revert |
| 35 | + *modified* files and **remove** *added* ones, or run in a throwaway worktree. |
| 36 | + |
| 37 | +## 2. Draft-only, and STOP |
| 38 | + |
| 39 | +- The automation **commits, pushes a branch, and opens a *draft* PR — then stops.** It |
| 40 | + never marks a PR ready and never merges. Marking ready is the human's disposition |
| 41 | + after a fresh-eyes re-read. |
| 42 | +- No push / PR-open / ready-mark happens without explicit instruction. This is |
| 43 | + **mechanically enforced** by the builder/publisher PreToolUse hook (`builder_guard.py`), |
| 44 | + not left to prompt discipline. |
| 45 | +- If a brief seems to require marking a PR ready, that is a brief defect — surface it |
| 46 | + and stop; do not work around the block. |
| 47 | + |
| 48 | +## 3. Contribution targeting |
| 49 | + |
| 50 | +For a project with a single line of development this is one rule (target the default |
| 51 | +branch). For a project with **parallel maintenance lines**, the targeting matters: |
| 52 | + |
| 53 | +- **Fixes ride the current maintenance line and forward-merge** to the development |
| 54 | + line; only genuinely new features target the development/`main` line. A fix sent to |
| 55 | + the dev line alone doesn't reach released users. *(instance: the per-area branch map |
| 56 | + — INTEGRATION.md §2.)* |
| 57 | +- **A maintainer's explicit base-branch request on the PR overrides the default.** |
| 58 | +- **Cross-version cherry-pick is a *correctness* check, not a conflict check.** |
| 59 | + "Applies cleanly" is **not** "remains correct": verify a cherry-picked fix against the |
| 60 | + *target branch's* related code — **including files the patch doesn't touch** — not just |
| 61 | + that `git cherry-pick`/`git apply` succeeds. |
| 62 | +- **Where the test/artifact ships can differ by target.** Different branches or target |
| 63 | + kinds (e.g. a core vs a plugin/addon contribution) may use different test locations or |
| 64 | + naming conventions; the brief's **Test file** field must name the right one *for the |
| 65 | + target branch*. Getting it wrong is a recurring Do error. *(instance: the per-target |
| 66 | + conventions — INTEGRATION.md §3.)* |
| 67 | + |
| 68 | +## 4. Validation against upstream |
| 69 | + |
| 70 | +- **Validate against the *clean upstream* contribution target**, not the fork's |
| 71 | + tooling-polluted branches and not a developer's working clone. Use a pinned checkout |
| 72 | + of `upstream/<base>` (a per-version `git worktree` when the project has several target |
| 73 | + versions) so "verified" means "verified against what reviewers will see." *(instance: |
| 74 | + the runners + how the pinned targets are built — INTEGRATION.md §3.)* |
| 75 | + - *Docker note:* a `git worktree`'s `.git` is a **file** pointing at the primary |
| 76 | + gitdir; if you bind-mount a worktree into a container, also mount that gitdir at its |
| 77 | + own absolute path or in-container git breaks. |
| 78 | +- **A fix may legitimately depend on an unmerged upstream fix.** When the contribution |
| 79 | + target lacks a not-yet-merged prerequisite, **record the dependency** (e.g. retry |
| 80 | + against an "essential" line = upstream + a minimal, evidence-based set of enabling |
| 81 | + fixes, and write a dependency note into the bundle) rather than silently failing the |
| 82 | + gate. Keep that enabling set minimal — add a fix only once a bundle demonstrably needs |
| 83 | + it ([`principles.md`](principles.md) §8). *(instance: the essential set + manifest — |
| 84 | + INTEGRATION.md §2.)* |
| 85 | +- **The patch must be commit-ready for the *target* repo.** The publish commit runs the |
| 86 | + *target's own* pre-commit hooks (formatter/linters), which no PDCA gate models — so |
| 87 | + "all gates green" ≠ committable. Run the project's configured formatter / commit hooks |
| 88 | + before declaring done. |
| 89 | + |
| 90 | +## 5. Upstream-isn't-ahead (prior art) |
| 91 | + |
| 92 | +Before contributing, confirm the fix isn't already upstream or already rejected: |
| 93 | + |
| 94 | +- **Search by affected file *path*, not bug-id or keyword.** Anchoring on the path |
| 95 | + catches side-effect fixes that an id/keyword search misses. (Mind host tokenization |
| 96 | + quirks — a substring search may not match a compound word.) |
| 97 | +- **Check merged history *and* closed/rejected PRs** on the upstream repo(s) for that |
| 98 | + path before opening a contribution. *(instance: the exact branches + search commands — |
| 99 | + INTEGRATION.md §5.)* |
| 100 | + |
| 101 | +--- |
| 102 | + |
| 103 | +**Who consults this.** Plan resolves the branch target and runs the prior-art check |
| 104 | +(§3, §5); Do writes the patch against the target branch, honoring cherry-pick |
| 105 | +correctness and test placement (§1, §3, §4); Check (reviewer) judges cherry-pick |
| 106 | +correctness, validation-against-upstream, and prior-art (§3–§5) and routes the |
| 107 | +unmechanizable ones to NEEDS-HUMAN; Publish performs the fork push under §1–§2. The |
| 108 | +deterministic git/PR mechanics are in `pdca_harness.publish`; this file is the *why*. |
0 commit comments