From 5efda076b340f11cb2a6b158a667b6b084c22431 Mon Sep 17 00:00:00 2001 From: Carlo van Driesten Date: Fri, 23 Oct 2026 10:43:00 +0200 Subject: [PATCH 1/3] chore(fork): define the ASCS-eV fork process ASCS-eV/PufferDrive is consumed as a git submodule internally while feeding features back to Emerge-Lab/PufferDrive. This commit carries everything that exists only in the fork, and is the single place fork-only content lives. Branch model: 3.0 verbatim, fast-forward-only mirror of upstream/3.0 integration/3.0 default branch; this commit plus a linear stack of single-commit features; what internal consumers pin feature/ one feature, exactly one commit mirror/ cherry-picked onto pure upstream/3.0 for upstream PRs integration/3.0 is the default branch because GitHub reads the PR template, the contributing guidelines, the agent instructions and scheduled workflows only from the default branch, and none of those files may exist on 3.0 without destroying the mirror. A clone checks out the default branch, so the copy of the instructions here is the process as far as every agent and every submodule consumer is concerned; a rule that lives only in an open PR has no effect. Nothing reaches integration/3.0 until three gates pass: 1. CI green on the final commit. 2. Adversarial review by someone other than the author, with the findings written down -- including the probes that found nothing. 3. Upstream-readiness proven rather than asserted: the commit cherry-picks onto a branch cut from pure upstream/3.0 and produces an upstream-clean diff. We stack only commits we could contribute today. chore(fork): commits are exempt from gate 3, since fork-only infrastructure can never be upstream-ready. They carry two orthogonal markers: do-not-mirror (must never reach upstream) and do-not-merge (the merge button is never pressed -- merging would create a second chore(fork): commit and break the single-commit invariant that every sync asserts). Such a PR is folded into this commit by fixup and closed unmerged. Contents: - .github/copilot-instructions.md: normative process for coding agents - CONTRIBUTING.md: human orientation, same rules - .github/pull_request_template.md: testing and stack-hygiene gates - .github/workflows/sync-mirror.yml: fast-forward-only mirror sync that refuses any non-fast-forward instead of rewriting the upstream base - AGENTS.md / CLAUDE.md: fenced pointer so agents find the fork process without colliding with upstream-owned content - ci.yml / install.yml: run CI for PRs targeting integration/3.0, which upstream's branch filters would otherwise skip A standing draft PR from integration/3.0 into 3.0 gives a continuous CI verdict on the combined stack; it must never be merged. Everything here is fork-only and must never be cherry-picked upstream. --- .github/copilot-instructions.md | 930 ++++++++++++++++++++++++++++++ .github/pull_request_template.md | 84 +++ .github/workflows/ci.yml | 2 +- .github/workflows/install.yml | 2 +- .github/workflows/sync-mirror.yml | 132 +++++ AGENTS.md | 10 + CLAUDE.md | 10 + CONTRIBUTING.md | 170 ++++++ 8 files changed, 1338 insertions(+), 2 deletions(-) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/pull_request_template.md create mode 100644 .github/workflows/sync-mirror.yml create mode 100644 CONTRIBUTING.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..d8c52dd4f0 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,930 @@ +# ASCS-eV/PufferDrive — fork contribution rules + +**Audience:** every coding agent (Copilot CLI/IDE, Claude Code, Codex, …) and every human working in +`ASCS-eV/PufferDrive`. + +**This file is fork-only.** It does not exist upstream and must never reach `Emerge-Lab/PufferDrive`. + +## 0. Precedence + +1. The human's explicit instruction in the current turn. +2. **This file** — fork branching, PR, and release process. +3. `AGENTS.md` / `CLAUDE.md` (upstream-owned) — code style, trust boundaries, guardrails. + +Rules 2 and 3 do not overlap: this file never redefines coding standards, and `AGENTS.md` never +defines fork process. Where `AGENTS.md` speaks about issues and PRs, it is describing +**upstream's** process; for PRs raised inside this fork, section 5 below applies instead. + +## 1. Why this fork exists + +`ASCS-eV/PufferDrive` is a fork of `Emerge-Lab/PufferDrive`, consumed as a **git submodule** by +internal BMW projects. It has two jobs: + +- Provide a **stable, integrated branch** that internal consumers can pin. +- Prepare **small, well-tested, independently reviewable features** that we offer back upstream as + mirror PRs once we consider them ready. + +Everything below exists to keep those two jobs from fighting each other. + +## 2. Branch model + +| Branch | Role | Who writes | History | +| --- | --- | --- | --- | +| `3.0` | Verbatim mirror of `upstream/3.0`; base for every upstream PR | automation only | fast-forward only | +| `integration/3.0` | **Default branch.** Combined feature stack; what BMW consumes | feature PRs only | linear, rebased | +| `feature/` | One feature, **exactly one commit** | contributor/agent | rebased onto `integration/3.0` | +| `chore/` | Fork-only change, **exactly one commit** | contributor/agent | rebased onto `integration/3.0` | +| `mirror/` | Upstream PR source | generated by cherry-pick | cut from `upstream/3.0` | +| `2.0` | Frozen legacy line | nobody | archived, do not touch | + +Remotes: `origin` = `ASCS-eV/PufferDrive`, `upstream` = `Emerge-Lab/PufferDrive`. + +`integration/3.0` is the default branch because GitHub reads the PR template, the contributing +guidelines, and scheduled workflows **only** from the default branch — and none of those fork-only +files may exist on `3.0` without destroying the mirror. A welcome side effect: new PRs default to +the correct base. + +The same reasoning governs this file. A fresh `git clone` checks out the default branch, so the copy +of `.github/copilot-instructions.md`, `AGENTS.md`, and `CLAUDE.md` **on `integration/3.0` is the +process** as far as every agent — and every BMW submodule consumer — is concerned. A process change +that exists only in an open PR does not exist. That is precisely why fork-only changes are folded +into the base commit on the default branch (section 3.3) instead of being parked on a long-lived +branch, and why section 4.1 opens by checking whether the rules are mid-change. + +``` +upstream/3.0 ──► 3.0 (mirror) + └─► integration/3.0 = 3.0 + [fork scaffolding] + [feat A] + [feat B] + … + └─► feature/ (1 commit, PR'd into integration/3.0) +3.0 ──► mirror/ (cherry-pick of that same 1 commit → upstream PR) +``` + +`upstream/2.0` and `upstream/3.0` have **no common ancestor**. Never attempt to merge, rebase, or +cherry-pick between the `2.0` and `3.0` lines. + +### Hard branch rules + +- **Nothing reaches `integration/3.0` that has not cleared the merge gate in section 4.8** — green + CI, adversarial review, and proven upstream-readiness. The stack is a line of + already-contributable commits, not a staging area for work in progress. +- **Never commit to `3.0`.** It is a pure mirror; any local commit there breaks the clean base for + every mirror PR. +- **Never create a merge commit on `integration/3.0`.** The stack stays linear so that each feature + remains a single, cherry-pickable commit. +- **Never branch a feature off another unmerged feature.** Each feature must apply to pure `3.0`. +- `integration/3.0` is maintained by rebase and is therefore **force-pushed** (`--force-with-lease`). + Contributors re-sync with `git fetch origin && git reset --hard origin/integration/3.0` after + confirming `git status` is clean — **not** `git pull --rebase`, which replays your local copies of + commits that were rewritten or dropped during the rebase and silently resurrects them. Consumers + pin an immutable tag (section 7), not the branch tip. + +**How `integration/3.0` is protected, and the deliberate hole in it.** All 12 required checks are +enforced, and "require branches to be up to date" is on, so a PR cannot be merged green against a +stale base and land an untested combination. Admin enforcement is deliberately **off**: with it on, +GitHub rejects *any* direct ref update to the branch (verified — it returns +`8 of 12 required status checks are failing`), which would make the fixup flow in section 3.3 and +the stack rebase in section 7 impossible. Those two procedures bypass the gate by design, and the +invariant checks written into them are the compensating control. Do not force-push this branch +outside those two procedures. + +### The standing integration draft PR + +A permanent **draft PR `integration/3.0` → `3.0`** titled `DO NOT MERGE — integration stack CI`, +carrying the `do-not-merge` label, gives us a continuous, always-visible CI verdict on *all combined +features at once*, which no individual feature PR can provide. + +- **Never merge it. Never mark it ready for review.** Merging would push the fork's stack into the + mirror and destroy the clean base that every upstream PR is cut from. +- It is force-pushed whenever the stack is rebased; its checks simply re-run. A red verdict means + the combined stack is broken even if every feature PR was green — fix it before tagging a release. +- Close and reopen it only if it is accidentally merged or the branch is recreated. + +## 3. Fork-only content — never send this upstream + +There are **two kinds**, and they are checked differently. Both must stay out of every feature +commit and every mirror PR. + +### 3.1 Whole files that exist only here + +If any of these appears in a feature diff, the feature is contaminated: + +- `.github/copilot-instructions.md` (this file) +- `CONTRIBUTING.md` +- `.github/pull_request_template.md` +- `.github/workflows/sync-mirror.yml` + +### 3.2 Fork-only edits inside files that also exist upstream + +These files are shared with upstream, so the **file** appearing in a diff is fine and often +correct — only the **fork-only hunk** must stay put: + +| Shared file | Fork-only part | +| --- | --- | +| `AGENTS.md`, `CLAUDE.md` | the `` … `` block | +| `.github/workflows/ci.yml`, `.github/workflows/install.yml` | the `'integration/3.0'` entry in the **`pull_request`** `branches:` filter | + +A file-name check cannot catch 3.2 — only a content check can (section 4.5). Conversely, do not +treat a shared file as untouchable: a PR that rewrites the install steps of `ci.yml` and +`install.yml` is fully upstream-contributable, because it leaves the branch filter alone. + +Do **not** add `integration/3.0` to those workflows' `push:` filters. The standing draft PR +(section 2) already runs the full suite on every update of the branch, so a `push` trigger only +duplicates every job and doubles runner consumption. + +Fork-only content lives in **one dedicated commit** (`chore(fork): …`) sitting directly on top of +`3.0` at the base of the integration stack. That commit is never cherry-picked. + +### 3.3 Changing fork-only content + +The "exactly one `chore(fork):` commit" rule is what keeps the stack trivially rebasable onto new +upstream, and keeps "is this commit contributable?" an obvious yes for every *other* commit on the +stack. Maintaining that invariant has consequences: + +1. Branch `chore/` from `origin/integration/3.0`. Make the change as one commit. +2. Open the PR **as a draft**, titled `[do-not-mirror][do-not-merge] chore(fork): …`, and apply + **both** the `do-not-mirror` and `do-not-merge` labels. +3. These are two independent axes and both must be marked, because they fail in different + directions: + - **`do-not-mirror`** — the change must never reach upstream. Marked four times over, deliberately + redundantly: title prefix, label, `chore(fork):` commit type, and the "Never upstream" box in + the PR template. + - **`do-not-merge`** — the merge button must never be pressed on it. Merging would put a *second* + `chore(fork):` commit on the stack, and section 7's invariant asserts there is exactly one, so + **every future sync would abort** until someone unpicked it by hand. Marked twice: title prefix + and label. + + An agent that misses all of these is not reading. +4. Gates 1 and 2 (section 4.8) apply in full. Gate 3 does not — see the exemption there. +5. When the gates pass, mark it ready for review, then **do not press the merge button.** Apply it + by folding it into the base commit instead: + +```bash +git switch integration/3.0 +git fetch origin +test -z "$(git status --porcelain)" || { echo "ABORT: dirty tree"; exit 1; } + +START=$(git rev-parse origin/integration/3.0) # capture the lease target before rewriting +git reset --hard "$START" + +# The base commit is the oldest commit on the stack. Verify that, don't assume it. +BASE=$(git rev-list origin/3.0..HEAD | tail -1) +git log -1 --format=%s "$BASE" | grep -q '^chore(fork)' \ + || { echo "ABORT: oldest stack commit is not chore(fork)"; exit 1; } + +git cherry-pick --no-commit +git commit --fixup "$BASE" +GIT_SEQUENCE_EDITOR=: git rebase -i --autosquash origin/3.0 +make test +``` + +Note `git cherry-pick` takes **one** commit — which is safe only because a `chore/` branch is +required to be exactly one commit (section 4.1). Confirm that before you start, or you will silently +apply only its tip. + +Never use an interactive `git rebase -i` here. Agents run without an editor, where it silently +becomes a no-op — and the next command force-pushes the default branch. The `GIT_SEQUENCE_EDITOR=:` +form above is deterministic and needs no editor. + +Verify the invariant actually held **before** you push: + +```bash +n=$(git log --format=%s origin/3.0..HEAD | grep -c '^chore(fork)' || true) +test "$n" = 1 || { echo "ABORT: $n chore(fork) commits on the stack, expected 1"; exit 1; } +git log --format=%s origin/3.0..HEAD | grep -q '^fixup!' \ + && { echo "ABORT: fixup! commit was not folded in"; exit 1; } + +git push --force-with-lease=refs/heads/integration/3.0:"$START" origin integration/3.0 +``` + +(`grep -c` exits non-zero when it finds nothing, hence the `|| true`. Use the explicit +`--force-with-lease=:` form — see section 7 for why the bare form is unsafe.) + +This rewrites the default branch after only `make test`, which is narrower than the required check +set. Watch the standing draft PR afterwards and be ready to fix forward quickly. + +The PR then ends up **closed, not merged**. Post the resulting `integration/3.0` SHA in the PR +thread so the trail survives; a closed-unmerged PR with no explanation looks like abandoned work. + +**Keep the window short.** Between opening a `chore/` PR and folding it in, the default branch +still serves the *old* rules, and every agent that clones the fork in that window follows them — see +section 2. A fork-only PR is a **review vehicle, not a delivery mechanism**: nothing in it is in +force until the force-push above lands. So do not park one open across unrelated work, and when one +has been open a while, assume agents have already acted on the superseded text and check their +output against the new rules rather than assuming compliance. + +If a feature genuinely needs to change one of these files, make it a **separate** `chore(fork):` +commit — do not fold it into the feature commit. + +## 4. Feature workflow + +### 4.1 Start + +One-time setup in a fresh clone — `upstream` is not configured automatically, and several commands +below (and Gate 3) fail without it: + +```bash +git remote get-url upstream >/dev/null 2>&1 \ + || git remote add upstream https://github.com/Emerge-Lab/PufferDrive.git +gh repo set-default ASCS-eV/PufferDrive # else bare `gh pr …` can resolve to upstream +``` + +The Makefile targets run `.venv/bin/python`, so none of the test recipes below work until the +environment exists: + +```bash +uv venv && source .venv/bin/activate +uv pip install -e . +test -x .venv/bin/python || { echo "ABORT: no .venv — see README.md"; exit 1; } +``` + +Before relying on anything below, check whether the process itself is mid-change. An open PR +carrying **both** fork-only labels is rewriting the rules you are about to follow: + +```bash +gh pr list --repo ASCS-eV/PufferDrive --state open --label do-not-mirror --label do-not-merge \ + --json number,title --jq '.[] | "#\(.number) \(.title)"' +``` + +`gh` ANDs repeated `--label` flags, so this matches fork-only process PRs and nothing else — the +standing draft PR of section 2 carries `do-not-merge` alone and is correctly excluded. Anything +listed supersedes this file in the areas it touches: read it before you start, and rebase onto the +result rather than racing it. + +Then: + +```bash +git fetch origin && git fetch upstream +git switch -c feature/ origin/integration/3.0 +``` + +`` is short, lowercase, hyphenated, and describes the change (`lane-heading-obs`, not +`fix-stuff`). + +For fork-only work that will become a `chore(fork):` commit, name the branch `chore/` +instead. Everything else in section 4 applies unchanged, except Gate 3 (section 4.8), from which +such commits are exempt. + +### 4.2 Scope + +One feature = one reviewable idea. If the PR description needs the word "and" to list unrelated +changes, split it. Refactors that merely enable a feature go in their own earlier PR. + +### 4.3 Build and test + +Makefile targets assume a POSIX shell and `.venv/bin/python` — use Linux, macOS, or WSL. + +| Situation | Required before pushing | +| --- | --- | +| any change | `pre-commit run --all-files` | +| `.c` / `.h` touched | `make rebuild` (zero warnings — warnings are bugs) | +| Python / env / eval | `make test-unit`, `make test-eval` | +| C sim logic | `make test-c` | +| notebooks or their deps | `make test-notebooks` | +| **sim dynamics, rewards, observations, or training loop** | `make test-docker-smoke` (golden train/rollout smoke run) | +| perf-sensitive hot paths | `make -C tests/drive perf` | +| everything in the default suite, before marking ready | `make test` | + +`make test` is **not** everything: it runs `test-unit test-eval test-c test-notebooks` and +deliberately excludes `test-docker-smoke` (slow, needs Docker) and the perf target. If the table +above says you need those, run them explicitly — `make test` passing does not cover them. + +**Regenerating smoke goldens.** If your change legitimately moves the golden train/rollout values +(observation layout, dynamics, rewards, RNG draw order), regenerate them with +`tests/smoke_tests/update_goldens.sh`, which runs inside the pinned QEMU image so the numbers match +CI. Review the diff, then stage both files **in the same single commit as the change that caused +them** — the rollout golden is gitignored and needs `-f`: + +```bash +tests/smoke_tests/update_goldens.sh +# tests/smoke_tests/data/ matches .gitignore, so BOTH goldens need -f even though +# both are already tracked — a plain `git add` on them exits 1. +git add -f tests/smoke_tests/data/drive_smoke_golden.json \ + tests/smoke_tests/data/drive_rollout_golden.json +``` + +Goldens are **base-dependent**: values regenerated on `integration/3.0` are not necessarily valid on +pure `upstream/3.0`. If Gate 3 (section 4.8) shows the smoke test failing on `mirror/`, +regenerate there and carry the corrected values back, rather than assuming our stack's numbers are +upstream's. + +**New behaviour requires a new test.** A bug fix requires a test that fails before the fix and +passes after it. "Existing tests still pass" is not sufficient evidence for a feature PR. + +Paste the commands you actually ran, and their outcome, into the PR's *How tested* section. Do not +claim a suite passed unless it ran to completion in this environment. + +### 4.4 Shape the commit + +Squash to **exactly one commit** before requesting review: + +```bash +git fetch origin +git rebase origin/integration/3.0 # replay onto the current base +git reset --soft origin/integration/3.0 # collapse the branch into the index +git commit -F # or -m; never rely on an editor opening +test "$(git rev-list --count origin/integration/3.0..HEAD)" = 1 \ + || { echo "ABORT: not exactly one commit"; exit 1; } +``` + +If the rebase conflicts: resolve, `git add` the files, then +`GIT_EDITOR=true git rebase --continue` — plain `--continue` opens an editor and fails outright when +none is available. To back out, `git rebase --abort`. + +Use the non-interactive form above rather than `git rebase -i`: agents run without an editor, where +the interactive rebase either stalls or silently does nothing. + +Conventional Commits, imperative mood, ≤ 72-char subject: + +``` +feat(drive): add lane-heading error to the observation vector + +Why the change is needed, what the approach is, and anything a reviewer +would otherwise have to reverse-engineer. +``` + +Types: `feat`, `fix`, `perf`, `refactor`, `test`, `docs`, `build`, `ci`, `chore`. +Common scopes: `drive`, `obs`, `reward`, `train`, `eval`, `render`, `config`, `fork`. + +`chore(fork): …` marks a commit that **must never go upstream**. + +Do not add AI/agent authorship trailers or "generated by" notes to commits, or to **fork** PR text. + +The single exception is the upstream authorship disclosure required by section 6.1, which is +**mandatory** on mirror PR bodies. That is a disclosure to upstream, not an authorship trailer, and +this rule does not license omitting it. + +### 4.5 Verify upstream-cleanliness before opening the PR + +```bash +test "$(git rev-list --count origin/integration/3.0..HEAD)" = 1 \ + || { echo "ABORT: not exactly one commit"; exit 1; } + +git diff --name-only origin/integration/3.0...HEAD \ + | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror' \ + && { echo "ABORT: fork-only file in the feature diff"; exit 1; } + +git diff origin/integration/3.0...HEAD -- AGENTS.md CLAUDE.md \ + .github/workflows/ci.yml .github/workflows/install.yml \ + | grep -Eq '^[+-].*integration/3\.0|^[+-].*fork:(begin|end)' \ + && { echo "ABORT: fork-only hunk touched in a shared file"; exit 1; } + +echo "pre-flight OK" +``` + +These are written as guards, not as commands whose output you eyeball: a check that prints +contamination and exits `0` is not a check. Changing the *rest* of a shared file is allowed and is +sometimes the whole point of the PR — only the fork-only hunks of section 3.2 are protected. + +This is the cheap pre-flight check. The authoritative proof is Gate 3 in section 4.8. + +**`chore/` branches are exempt from this subsection entirely** — their whole purpose is to +change the files in section 3.1, so these checks are expected to "fail". Run the first command only +(exactly one commit), and state in the PR that the branch is fork-only. + +### 4.6 Open the fork PR + +Base **`integration/3.0`** (the default, so it is pre-selected). Fill in +`.github/pull_request_template.md` completely and let CI finish. + +Opening the PR is **not** the end of your job — a PR that is merely open is not stacked. It must +clear section 4.8 first. + +### 4.7 Adversarial review + +Every feature PR is adversarially reviewed before it may be stacked. The reviewer's job is to +**try to break the change**, not to confirm that it looks reasonable. "LGTM" is not a review. + +**The reviewer is not the author.** When an agent wrote the change, the review runs as a *separate +task with fresh context* — a reviewer still holding the author's reasoning reproduces the author's +blind spots. + +Work through all of these, and answer them in writing: + +- **Does the test actually test it?** Revert the source change, keep the new test, confirm the test + fails. A test that passes against unfixed code proves nothing. +- **What input breaks it?** Empty scenario, single agent, max agents, zero-length trajectory, NaN, + out-of-range action, a controlled agent removed mid-episode. +- **Determinism.** Same seed, same result? Any new iteration over an unordered container, + uninitialised read, or change to float accumulation order? +- **Trust boundaries and memory.** Every rule in `AGENTS.md` that touches the changed code — + bounds, ownership, lifetime, null handling. +- **Layout and compatibility.** Observation vector, action space, config keys, checkpoint format. + Any change here breaks submodule consumers and must be called out explicitly in the PR. +- **Performance.** Did a hot path gain an allocation, a branch, or a cache miss? If plausible, + require `make -C tests/drive perf` numbers rather than an opinion. +- **Scope.** Anything in the diff that is not the one stated idea: drive-by renames, unrelated + reformatting, leftover debug code, commented-out blocks. +- **Upstream reception.** Would an upstream maintainer request changes? Naming, file placement, + config surface, a knob we want but they would not. Fix it now, not after they ask. + +Post the outcome as a PR review listing what was probed and what was found — **including the probes +that found nothing**, so the next reader knows the coverage. Each finding is either fixed or +explicitly accepted by a human, with the reason in the thread. + +### 4.8 The merge gate — nothing stacks until it is proven + +`integration/3.0` is what BMW pins *and* the base that every later feature rebases onto, so a defect +merged here is multiplied across every downstream consumer and every subsequent PR. Merge a feature +PR **only** when all applicable gates are green: + +**Gate 1 — CI is green.** Every required check passes *on the final commit*. Not "green apart from a +known flake", not "green before the last force-push". This is enforced by GitHub, not by good +intentions: the required set is configured as branch protection on `integration/3.0`, so the merge +button stays disabled until all of it is green. Confirm with: + +```bash +gh pr checks --repo ASCS-eV/PufferDrive +``` + +If a check is broken for reasons unrelated to the feature, fix that first as its own PR — a CI fix +is itself a perfectly good feature PR — instead of merging past it. + +**Gate 2 — adversarial review passed.** Section 4.7, findings written down and resolved. + +**Gate 3 — upstream-readiness is proven, not asserted.** Build the mirror branch *before* merging +and let the cherry-pick prove the commit is contributable as-is. Every check below **aborts**; a +command that merely prints contamination is not a gate: + +```bash +git fetch upstream +git switch --detach upstream/3.0 # lets the branch be deleted even if checked out +git branch -D mirror/ 2>/dev/null || true # always recreate — this is what makes it retryable +git switch -c mirror/ upstream/3.0 + +git cherry-pick || { + echo "ABORT: entangled with fork-only state"; git cherry-pick --abort; exit 1; } + +git diff --name-only upstream/3.0...HEAD \ + | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror' \ + && { echo "ABORT: fork-only FILE in mirror diff"; exit 1; } + +git diff upstream/3.0...HEAD \ + | grep -Eq '^\+.*integration/3\.0|^\+.*fork:begin' \ + && { echo "ABORT: fork-only HUNK in mirror diff"; exit 1; } + +echo "Gate 3 PASS" +git switch feature/ # name the branch; `git switch -` breaks after --detach above +``` + +If the cherry-pick conflicts, the feature is entangled with fork-only state: fix it on the feature +branch and repeat. Deleting and recreating `mirror/` every time is deliberate — section 7 +requires re-proving this gate after a rebase, and `git switch -c` fails if the branch already +exists. + +**If the change can move the smoke goldens** (observation layout, dynamics, rewards, RNG order), +Gate 3 must also *run* the relevant suites from section 4.3 on the mirror branch, not just diff it. +Goldens generated on our stack are not evidence about pure upstream. A commit whose goldens depend +on an earlier feature that upstream does not have is **not** upstream-ready: it does not stack until +that dependency is resolved. + +Actually *opening* the upstream PR stays a separate, human-approved step (sections 5 and 6) — the +gate is only that we could open it today. Record the resulting `integration/3.0` SHA on the PR when +the feature is stacked, so section 6 can rebuild the mirror later from a known commit. + +**Exemption — `chore(fork):` commits.** Fork-only infrastructure cannot be upstream-ready by +definition, so Gate 3 does not apply, and the fork-only *file* and *hunk* checks in section 4.5 are +expected to trip — touching those files is the entire point. **Every other rule still holds**, in +particular "exactly one commit": section 3.3 cherry-picks a single SHA, so a two-commit chore branch +silently loses work. Gates 1 and 2 apply in full, and the reviewer additionally confirms the change +is genuinely fork-only and contains nothing upstream would have wanted. Such a PR is a draft titled +`[do-not-mirror][do-not-merge] …` and carries both the `do-not-mirror` and `do-not-merge` labels; +it is applied by fixup into the base +commit rather than merged (section 3.3), so it ends **closed, not merged** — the "Rebase and merge" +step below does not apply to it. + +Only once the applicable gates pass, **a human presses "Rebase and merge"** (never squash-merge a +branch that is already one commit, never create a merge commit). Squash and merge commits are +disabled on this repository so the wrong button cannot be pressed by accident. + +If a feature is good but not yet upstream-ready, it does **not** go on the stack. Leave the PR open +and say why. An unmergeable-but-correct feature is a smaller problem than a stack nobody can +contribute from. + +## 5. Agent rules for issues and PRs + +- Agents **may** open, update, and comment on PRs and issues **in `ASCS-eV/PufferDrive`**. +- An agent **must not** adversarially review (section 4.7) a change it authored in the same session. + Start a fresh review task with no authoring context, or hand it to a human. +- Agents **must not** merge into `integration/3.0` on their own initiative. Report the gate status + from section 4.8 and let a human decide. +- **A human must verify the final PR title, body, and diff before an agent opens any PR** — fork or + upstream. The agent may press the button; it may not decide that the content is fit to publish. + Show the exact text and the diff, get explicit approval, then open it. For upstream PRs that + approval is **per PR, in the current conversation**, and covers the `mirror/` content too. +- Never open a PR targeting the `3.0` mirror. The only PR that may ever target it is the standing + draft from section 2, which exists already and must never be merged. +- When a PR targets upstream, **upstream's `AGENTS.md` "Issue and PR Guidelines" apply**, with one + documented exception handled in section 6.1. Do not import them into fork PRs. +- Never force-push a branch you did not create. Never force-push `integration/3.0` without being + asked to. +- Never commit secrets, internal BMW identifiers, customer data, internal URLs, or proprietary + scenario data. This fork is public. + +## 6. Mirror PR workflow (fork → upstream) + +Gate 3 (section 4.8) already proved this commit cherry-picks cleanly. Do **not** assume the branch +it produced still exists: it is local-only, the feature branch is deleted from `origin` on merge, +and the agent opening the upstream PR is usually not the one that ran the gate. **Always rebuild the +mirror branch from scratch.** + +Locate the commit **by subject, not by the SHA recorded on the feature PR**. Every sync rebase +rewrites the whole stack, so a recorded SHA stops resolving as soon as `3.0` advances. Treat it as a +hint for a human reader, never as an input to a command. + +```bash +git fetch origin && git fetch upstream +SUBJ="fix(scope): the exact commit subject" +n=$(git log --format='%s' origin/3.0..origin/integration/3.0 | grep -Fc -- "$SUBJ") +test "$n" = 1 || { echo "ABORT: subject matches $n stack commits, expected 1"; exit 1; } +FEATURE_SHA=$(git log --format='%H %s' origin/3.0..origin/integration/3.0 \ + | grep -F -- "$SUBJ" | cut -d' ' -f1) +git log -1 --oneline "$FEATURE_SHA" + +git switch --detach upstream/3.0 # so the branch can be deleted even if checked out +git branch -D mirror/ 2>/dev/null || true +git switch -c mirror/ upstream/3.0 +git cherry-pick "$FEATURE_SHA" || { + echo "ABORT: no longer applies to current upstream"; git cherry-pick --abort; exit 1; } + +git diff --name-only upstream/3.0...HEAD \ + | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror' \ + && { echo "ABORT: fork-only FILE in mirror diff"; exit 1; } +git diff upstream/3.0...HEAD \ + | grep -Eq '^\+.*integration/3\.0|^\+.*fork:begin' \ + && { echo "ABORT: fork-only HUNK in mirror diff"; exit 1; } +echo "mirror is upstream-clean" +``` + +The `--detach` is what makes `git branch -D` work even when `mirror/` is the branch you are +currently on. It also means **`git switch -` will not take you back afterwards** — it resolves to a +detached commit, not a branch. Name the branch you want explicitly, e.g. `git switch +integration/3.0`. + +If a guard aborts, the feature commit was contaminated — fix it on `integration/3.0` and regenerate. +Otherwise push `mirror/` to `origin` and open the upstream PR from it **after a human has +verified the title, body and diff** (section 5). + +Re-run the relevant suites from section 4.3 on the mirror branch: it is a different base from the +one CI tested, and "it passed on our stack" is not evidence that it passes on pure upstream. + +Keep the upstream PR description in upstream's **What / Why / Notes** structure. Give them the +evidence, not our internal process vocabulary — upstream does not know or care what our integration +stack is. + +**Match upstream's title convention, not ours.** This fork uses Conventional Commits; upstream does +not. Their merged PRs read as imperative sentence case — *"Remove native Drive rendering in favor of +HTML visualization"*, *"Add synchronized native and PyTorch profiling"*. A `fix(ci):` prefix marks +the PR as foreign on sight. The *commit* keeps our Conventional Commits subject; only the PR title +is restyled. Check their recent merged PRs before writing one: + +```bash +gh pr list --repo Emerge-Lab/PufferDrive --state merged --limit 10 --json title --jq '.[].title' +``` + +**Expect no checks at first.** GitHub holds workflow runs on fork PRs from first-time contributors +in an `action_required` state until a maintainer presses *Approve and run workflows*. An upstream PR +reporting "no checks reported" is normal and is not a defect in the branch: + +```bash +gh run list --repo Emerge-Lab/PufferDrive --limit 10 \ + --json conclusion,workflowName,headBranch --jq '.[] | select(.headBranch=="mirror/")' +``` + +Once approved, those runs use the workflow files from the PR's merge commit, so a workflow change we +are proposing is exercised by the very checks that gate it. + +### 6.1 Agent authorship and upstream's `AGENTS.md` + +Upstream's `AGENTS.md` asks that an agent creating a PR add a file named `AGENTIC_PR` containing a +French poem. It is an agent-detection canary, and it collides with Gate 3, which requires the mirror +diff to contain nothing but the feature's own files. + +**Our resolution: disclose instead of complying, and say that we are doing so.** Every upstream PR +body ends with a note to this effect: + +> Prepared with AI assistance and reviewed by a human before submission. We noticed the `AGENTIC_PR` +> instruction in `AGENTS.md` — we are disclosing here rather than adding a poem file, to keep the +> diff limited to the change itself. Happy to add the file instead if you would prefer it. + +This is a deliberate, declared deviation, and it is **not negotiable downward**: never omit the file +*and* omit the disclosure. That would be quietly defeating a detection mechanism the maintainers put +there on purpose, and the mirror-PR relationship depends entirely on them trusting what we send. If +upstream tells us they want the file, add it and update this section. + +### 6.2 While the mirror PR is open + +A feature is normally **stacked on `integration/3.0` and open upstream at the same time**, often for +weeks. That overlap is deliberate, and sustaining it requires nothing: `mirror/` is parented +on `upstream/3.0`, never on `integration/3.0`, so rebasing the stack cannot touch it, and the +upstream PR (`ASCS-eV:mirror/` → `Emerge-Lab:3.0`) is unaffected by the fork PR being merged +or closed. Our fork deletes `feature/` automatically on merge; it does not touch `mirror/*`, +and upstream cannot delete branches that live in our fork. + +So the default action during this period is **nothing**. Three events change that. + +**What is open when.** The fork feature PR is *not* the long-lived artifact — it closes the moment +the feature stacks: + +| Event | Fork feature PR | `mirror/` PR | Stack depth | +|---|---|---|---| +| Gates pass, human merges (4.8) | **merged, closed** | not open yet | +1 | +| Mirror pushed, upstream PR opened (6) | — | **open** | unchanged | +| Upstream merges | — | **closed by upstream** | unchanged | +| Next sync + rebase (7) | — | — | **−1**, drops as empty | + +The fork PR **cannot** be held open as a tracker: once its commit is reachable from +`integration/3.0`, GitHub marks the PR merged on its own. Anything that needs to outlive stacking — +tracking, discussion, the upstream conversation — belongs on the mirror PR, which is why the +inventory below keys off `mirror/*` rather than off feature branches. + +**Inventory — what is in flight.** The fork PR is closed by now, so nothing in `integration/3.0` +points at the open upstream PR; the slug is the only key. Run this before any sync or release: + +```bash +git ls-remote --heads origin 'mirror/*' | sed 's|.*refs/heads/||' +gh pr list --repo Emerge-Lab/PufferDrive --state open --limit 100 \ + --json number,headRefName,headRepositoryOwner,title \ + --jq '.[] | select(.headRepositoryOwner.login=="ASCS-eV") | "#\(.number) \(.headRefName)"' +``` + +A `mirror/*` branch with no open PR is either finished (section 6.3) or was never opened. An open PR +with no branch should be impossible and means someone hit the deletion hazard below. + +**Event 1 — `upstream/3.0` advanced.** The PR starts showing conflicts in upstream's UI and +reviewers can no longer read it: + +```bash +git fetch upstream +git merge-base --is-ancestor "mirror/^" upstream/3.0 \ + || echo "mirror base diverged — rebuild onto the new upstream tip" +``` + +**Event 2 — our stacked commit changed.** A sync rebase can alter the commit through conflict +resolution. The open PR then offers upstream something we no longer run. That is a correctness and +honesty problem, not untidiness. Detect it by patch-identity — no checkout required: + +```bash +pid() { git diff "$1^" "$1" | git patch-id --stable | cut -d' ' -f1; } +test "$(pid "$FEATURE_SHA")" = "$(pid "mirror/")" \ + || echo "open upstream PR is OUT OF DATE — rebuild and force-push" +``` + +For either event, rebuild with the section 6 recipe — including both Gate 3 guards, because a +conflict resolution is exactly where fork-only content gets dragged in — then: + +```bash +git push --force-with-lease origin mirror/ +``` + +Force-pushing updates the open PR in place. **Never close and reopen**: that discards the review +thread and reads as a withdrawal. After an Event 2 rebuild, say in the PR that the change was +updated and why; never leave upstream reviewing a version we have abandoned. + +**Hazard — deleting `mirror/` closes the upstream PR.** GitHub closes a pull request when its +head branch disappears. Done to an open contribution, that looks like a silent withdrawal and throws +away the review. Section 6.3 ends in a branch deletion, which is safe *only* once the PR has reached +a terminal state. Always assert that first, and never delete a mirror branch by reflex: + +```bash +STATE=$(gh pr list --repo Emerge-Lab/PufferDrive --head "mirror/" \ + --state all --json state --jq '.[0].state // "NONE"') +case "$STATE" in + MERGED|CLOSED|NONE) git push origin --delete "mirror/" ;; + *) echo "ABORT: upstream PR is $STATE — deleting the branch would close it"; exit 1 ;; +esac +``` + +### 6.3 After upstream responds + +A mirror PR has three possible endings, and each leaves different work behind. None of them is +"nothing" — a stacked commit whose mirror has been resolved must be retired deliberately, or the +stack slowly fills with commits nobody can explain. + +**First, find out what actually happened.** Patch-identity, not the PR's merge badge, is the test — +upstream may squash, rename or amend, and a merged PR does not guarantee our exact diff landed. + +```bash +git fetch upstream +git cherry upstream/3.0 mirror/ +# "- " -> an equivalent patch IS upstream under a different SHA (case A) +# "+ " -> it is NOT upstream as written (case B or C) +# (no output) -> our exact commit is now an ancestor of upstream/3.0 (case A) +``` + +**Empty output is a result, not a failure.** It means upstream merged in a way that preserved our +commit object, so it is reachable from `upstream/3.0` and falls outside the range `git cherry` +examines. Both `-` and silence mean "landed"; only `+` means "not landed as written". If you cannot +tell which you are looking at, disambiguate explicitly: + +```bash +git merge-base --is-ancestor mirror/ upstream/3.0 \ + && echo "landed verbatim (same commit)" \ + || git cherry upstream/3.0 mirror/ +``` + +Patch-identity ignores the commit message, so upstream renaming the subject still reports `-`. It +does **not** ignore content: any amendment to the diff, however small, reports `+`. That is the +correct signal — an amended change is case B, and the rebase will stop on it rather than drop it. + +The same command audits the whole stack at once, which is worth running before any release: + +```bash +git cherry upstream/3.0 origin/integration/3.0 +``` + +Every `-` line is a commit we are carrying that upstream already has. Those are pure rebase risk and +should already be gone; if one persists across two syncs, something is resurrecting it (section 7). + +**Case A — merged as written.** Do nothing to the stack by hand. The next sync fast-forwards `3.0`, +and the rebase of `integration/3.0` drops our copy automatically because it becomes empty. Confirm +it actually went, rather than assuming: + +```bash +git rev-list --count origin/3.0..origin/integration/3.0 # must be one lower than before +git cherry upstream/3.0 origin/integration/3.0 | grep -q '^-' \ + && { echo "WARN: stack still carries commits upstream already has"; } +git branch -D mirror/ +``` + +Then delete the remote branch **through the terminal-state guard in section 6.2** — never with a +bare `git push origin --delete`. In case A the PR is already merged, so the guard passes; running it +anyway is what stops the same command being used by reflex while a PR is still open. + +**Case B — merged with modifications.** Upstream's edited version is now in the new base, so ours is +redundant *and* textually different, which means the rebase will stop on a conflict instead of +dropping it silently. Resolve in favour of upstream — ours has no privileged status once they have +taken ownership of the change: + +```bash +git rebase --skip # discards our now-superseded commit and continues +``` + +Then read upstream's merged version before moving on. If they changed behaviour we depend on, that +is a *new* feature branch against the new base, not an attempt to restore our original commit. Never +resurrect a superseded commit to "win back" a difference; raise it upstream as a follow-up instead. + +**Case C — rejected, or stalled with no realistic path.** This is the one that needs a decision +rather than a command, and it must not be left implicit. The commit is now a **permanent fork +delta**, and a permanent delta that still looks like a pending contribution will be rebased, +re-reviewed and re-offered by some future agent forever. + +Decide explicitly, and record the decision in the PR and in the commit subject: + +- **Keep it as a fork-only behaviour change** — reclassify it under section 3.2, add its marker to + the fork-only hunt patterns, and rename the commit to `chore(fork):` so the section 4.5 and Gate 3 + guards start treating it as something that must *never* leave. This is the honest outcome for a + change upstream has declined on principle. +- **Drop it** — if it only existed to be upstreamed, remove it from the stack (section 3.3) rather + than carrying a rebase hazard indefinitely. + +Either way, close the mirror PR with a reason, delete `mirror/`, and note the outcome so the +next agent does not reopen the same conversation with upstream. + +**In all three cases**, if the feature commit was rewritten on our side at any point after the +mirror branch was built, that mirror is stale — rebuild it and re-prove Gate 3 (section 6) before +drawing any conclusion from `git cherry`, which compares the stale patch rather than the live one. + +## 7. Syncing and releasing + +**Sync the mirror:** automated. `.github/workflows/sync-mirror.yml` fast-forwards `3.0` from +`upstream/3.0` daily and on demand (Actions → *Sync 3.0 mirror* → *Run workflow*). It refuses any +non-fast-forward, which is the alarm that someone committed to the mirror. `3.0` additionally blocks +force-pushes, deletions, and non-linear history. Do not push to it by hand. + +When the mirror moves ahead of the stack, the same workflow opens (and keeps updated) a +`mirror-sync` tracking issue; it closes the issue automatically once `integration/3.0` contains the +mirror head. The rebase itself stays manual — replaying features onto new upstream code can +conflict and must be re-tested. + +**Rebase the stack** (maintainers): + +```bash +git switch integration/3.0 +git fetch origin +test -z "$(git status --porcelain)" || { echo "ABORT: dirty tree"; exit 1; } + +START=$(git rev-parse origin/integration/3.0) # capture the lease target before rewriting +git reset --hard "$START" # discard local drift +git rebase origin/3.0 +``` + +On conflict, resolve in favour of upstream for code they changed, keep our commit's intent, then: + +```bash +git add +GIT_EDITOR=true git rebase --continue # --continue opens an editor; GIT_EDITOR=false makes it fail +``` + +If a conflict cannot be resolved confidently, `git rebase --abort` and raise it — never guess on the +branch BMW pins. + +Re-check the invariants **after** the rebase, exactly as in section 3.3 — a rebase happily preserves +a stack that was already malformed: + +```bash +n=$(git log --format=%s origin/3.0..HEAD | grep -c '^chore(fork)' || true) +test "$n" = 1 || { echo "ABORT: $n chore(fork) commits, expected 1"; exit 1; } +git log --format=%s origin/3.0..HEAD | grep -q '^fixup!' \ + && { echo "ABORT: stray fixup! on the stack"; exit 1; } +git log -1 --format=%s "$(git rev-list origin/3.0..HEAD | tail -1)" | grep -q '^chore(fork)' \ + || { echo "ABORT: oldest stack commit is not the fork base"; exit 1; } + +make test +git push --force-with-lease=refs/heads/integration/3.0:"$START" origin integration/3.0 +``` + +Use the **explicit** `--force-with-lease=:` form. Bare `--force-with-lease` leases against +your remote-tracking ref, which any background `git fetch` silently refreshes — at which point it +will happily overwrite a colleague's push that you never incorporated. If the push is rejected, +start over from the new remote tip; do not re-run with a wider hammer. + +`make test` is not the full gate (section 4.3) — it skips Docker smoke, perf, the install matrix and +pre-commit. After pushing, the standing draft PR re-runs the real suite against the rewritten stack; +treat a release as blocked until that is green. + +Drop commits that upstream has already merged; never resurrect them — section 6.3 gives the +mechanics, including the case where upstream merged a *modified* version. After any rebase that +rewrote a feature commit, its `mirror/` branch is stale: if an upstream PR is already open on +it, section 6.2 applies and the PR must be refreshed; if not, Gate 3 must be re-proven before that +feature is offered upstream. Either way, a commit whose content changed materially needs re-review. + +**Divergence report.** The stack is upstream *debt*: every feature commit we carry is one upstream +has not taken yet. Run this after every sync, and before any release: + +```bash +git fetch origin && git fetch upstream +TOTAL=$(git rev-list --count origin/3.0..origin/integration/3.0) +FORKONLY=$(git log --format='%s' origin/3.0..origin/integration/3.0 | grep -c '^chore(fork)') +echo "total gap : $TOTAL" +echo "fork-only : $FORKONLY (permanent floor)" +echo "feature gap : $((TOTAL - FORKONLY))" + +# every '-' is a commit upstream already has; the last sync should have dropped it +if git cherry upstream/3.0 origin/integration/3.0 | grep -q '^-'; then + echo "WARN: stale commits on the stack — see section 6.3" + git cherry upstream/3.0 origin/integration/3.0 | grep '^-' +fi +``` + +Use the `if` form, not `… | grep '^-' && echo WARN`. `grep` exits 1 when it matches nothing, so the +terse version makes a **healthy** stack report failure — harmless when typed by hand, wrong the +moment anyone wires the report into a script or a CI step. + +**The total gap can never reach zero, and must not be driven to zero.** `integration/3.0` +permanently carries at least one `chore(fork):` commit, because the files in section 3.1 do not +exist upstream by design. An agent that reads a non-zero gap as a defect and tries to close it by +upstreaming that commit would leak the entire fork-only process into `Emerge-Lab/PufferDrive` — +precisely what Gate 3 exists to prevent. + +**The number that should trend to zero is the feature gap.** Zero means upstream has absorbed +everything we carry and the fork is pure scaffolding again; that is the steady state we are aiming +for, not an exceptional one. A feature gap that only grows means we are accumulating a private fork +rather than contributing — treat a persistently rising count as a process failure to discuss, not a +number to ignore. Every feature commit on the stack should be traceable to an open or pending mirror +PR (section 6.2); one that is not is either unfinished work or a change nobody intends to upstream, +and the second kind belongs in a `chore(fork):` commit where the guards can see it. + +**Hazard — `update-smoke-golden.yml`.** This inherited upstream workflow ends in +`git push origin HEAD:`, and it triggers on pushes to `3.0` as well as `workflow_dispatch`. +Never dispatch it against `3.0`: it would commit to the mirror and break the clean base for every +upstream PR. If it ever does, `sync-mirror.yml` is the detector — it refuses the non-fast-forward +and the mirror-sync issue appears. Recover by resetting `3.0` to `upstream/3.0` (the branch +protection on `3.0` blocks force-pushes, so this needs an admin and should be done deliberately). + +**Release for the BMW submodule:** after a green rebase, run the divergence report above and confirm +the feature gap matches the mirror PRs actually in flight — a release is the moment the gap becomes +someone else's dependency. Then tag an immutable point and pin that tag. + +```bash +git tag -a ascs/3.0-$(date +%Y%m%d) -m "Integration snapshot" +git push origin ascs/3.0-$(date +%Y%m%d) +``` + +Consumers pin a tag or a commit SHA — never the moving `integration/3.0` tip. + +## 8. Definition of done + +A feature PR may be **stacked onto `integration/3.0`** only when **all** of the following hold. +Anything less stays an open PR. + +Authoring: + +- exactly one commit, Conventional Commits subject, rebased on current `integration/3.0` +- no fork-only file from section 3.1 and no fork-only hunk from section 3.2 in its diff +- new or updated tests cover the new behaviour; a fix has a test that fails without it +- the suites from section 4.3 that apply were run to completion and are listed in the PR +- the PR template is filled in, including the upstream-readiness assessment + +Gates (section 4.8): + +- **Gate 1** — every required check green on the final commit +- **Gate 2** — adversarial review (section 4.7) posted, findings fixed or explicitly accepted +- **Gate 3** — `mirror/` cherry-picks onto pure `upstream/3.0` without conflict and its diff + is upstream-clean — i.e. the feature is genuinely contributable today + *(exempt for `chore(fork):` commits, which can never be upstream-ready)* + +Merged by a human with **Rebase and merge**, leaving the stack linear. + +A `chore(fork):` PR has a different definition of done: exactly one commit, Gates 1 and 2 green, +Gate 3 and the section 4.5 fork-only checks exempt, all four `do-not-mirror` markers **and** both +`do-not-merge` markers present, and it is **folded into the base commit by fixup and the PR closed +unmerged** (section 3.3) — never merged with the button. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000000..f317a8bc04 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,84 @@ + + +## What + + + +## Why + + + +## How tested + + + +- [ ] `pre-commit run --all-files` +- [ ] `make rebuild` (required if `.c`/`.h` changed — zero new warnings) +- [ ] `make test-unit` / `make test-eval` +- [ ] `make test-c` (C sim logic) +- [ ] `make test-notebooks` (notebooks touched) +- [ ] `make test-docker-smoke` (dynamics, rewards, observations, or training loop touched) +- [ ] `make -C tests/drive perf` (hot paths touched) +- [ ] `make test` before marking ready (unit + eval + c + notebooks — does **not** include + docker-smoke or perf; those are the two boxes above) + +New behaviour is covered by a new test / the fix has a test that fails without it: + + + +## Stack hygiene + + + +- [ ] exactly one commit (`git rev-list --count origin/integration/3.0..HEAD` → `1`) +- [ ] Conventional Commits subject, rebased on current `integration/3.0` +- [ ] no fork-only **file** in the diff (`git diff --name-only origin/integration/3.0...HEAD`) +- [ ] no fork-only **hunk** in a shared file — `AGENTS.md`, `CLAUDE.md`, and the `pull_request` + branch filters of `ci.yml` / `install.yml` are untouched in those specific places + +## Merge gate + + + +- [ ] **Gate 1 — CI green** on the final commit (no "green except a flake") +- [ ] **Gate 2 — adversarial review** posted by someone other than the author; findings fixed or + explicitly accepted in the thread +- [ ] **Gate 3 — upstream-readiness proven**: `mirror/` cherry-picked onto pure + `upstream/3.0`, applied without conflict, diff is upstream-clean + + +Gate 3 result: + + + +## Upstream disposition + +- [ ] Mirror PR to `Emerge-Lab/PufferDrive` requested now — needs a human to verify the final + title, body and diff first, and the body must carry the authorship disclosure from + copilot-instructions.md §6.1 +- [ ] Contributable, but we hold it for now, because: +- [ ] **Never upstream** — fork-only `chore(fork):` change, Gate 3 exempt + + + + +## Risk / notes + + diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b6d090efe..587fffa058 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: CI on: pull_request: - branches: [ main, '3.0' ] + branches: [ main, '3.0', 'integration/3.0' ] push: branches: [ main, '3.0' ] diff --git a/.github/workflows/install.yml b/.github/workflows/install.yml index 27f8b10bb1..7b5a5c4703 100644 --- a/.github/workflows/install.yml +++ b/.github/workflows/install.yml @@ -2,7 +2,7 @@ name: Installation Test on: pull_request: - branches: [ main, '3.0' ] + branches: [ main, '3.0', 'integration/3.0' ] push: branches: [ main, '3.0' ] diff --git a/.github/workflows/sync-mirror.yml b/.github/workflows/sync-mirror.yml new file mode 100644 index 0000000000..dafbece6a7 --- /dev/null +++ b/.github/workflows/sync-mirror.yml @@ -0,0 +1,132 @@ +name: Sync 3.0 mirror + +# Fork-only. Keeps the `3.0` branch a verbatim, fast-forward-only mirror of +# Emerge-Lab/PufferDrive:3.0. Never cherry-pick this workflow upstream. + +on: + schedule: + - cron: "17 4 * * *" + workflow_dispatch: + +permissions: + contents: write + issues: write + +concurrency: + group: sync-mirror + cancel-in-progress: false + +jobs: + sync: + name: Fast-forward 3.0 from upstream + runs-on: ubuntu-latest + steps: + - name: Checkout mirror + uses: actions/checkout@v4 + with: + ref: '3.0' + fetch-depth: 0 + + - name: Fetch upstream + run: | + git remote add upstream https://github.com/Emerge-Lab/PufferDrive.git + git fetch --no-tags upstream 3.0 + + - name: Fast-forward mirror + id: sync + run: | + local_sha=$(git rev-parse HEAD) + upstream_sha=$(git rev-parse FETCH_HEAD) + echo "upstream_sha=$upstream_sha" >> "$GITHUB_OUTPUT" + + if [ "$local_sha" = "$upstream_sha" ]; then + echo "3.0 already matches upstream ($upstream_sha). Nothing to do." + exit 0 + fi + + # A non-fast-forward means someone committed to the mirror or upstream + # rewrote history. Either way this must be resolved by a human, never + # by force-pushing the base that every mirror PR is cut from. + if ! git merge-base --is-ancestor "$local_sha" "$upstream_sha"; then + echo "::error::3.0 is not an ancestor of upstream/3.0 — refusing to push." + echo "Local 3.0: $local_sha" + echo "Upstream 3.0: $upstream_sha" + exit 1 + fi + + git push origin "$upstream_sha:refs/heads/3.0" + echo "Fast-forwarded 3.0: $local_sha -> $upstream_sha" + { + echo "### Mirror updated" + echo "\`3.0\`: \`$local_sha\` → \`$upstream_sha\`" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Check whether the integration stack is behind + id: stack + run: | + upstream_sha="${{ steps.sync.outputs.upstream_sha }}" + git fetch --no-tags origin '+refs/heads/integration/3.0:refs/remotes/origin/integration/3.0' + + if git merge-base --is-ancestor "$upstream_sha" refs/remotes/origin/integration/3.0; then + echo "needs_rebase=false" >> "$GITHUB_OUTPUT" + echo "integration/3.0 already contains $upstream_sha." + else + behind=$(git rev-list --count "refs/remotes/origin/integration/3.0..$upstream_sha") + echo "needs_rebase=true" >> "$GITHUB_OUTPUT" + echo "behind=$behind" >> "$GITHUB_OUTPUT" + echo "integration/3.0 is missing $behind upstream commit(s)." + fi + + - name: Open or update the rebase tracking issue + if: steps.stack.outputs.needs_rebase == 'true' + env: + GH_TOKEN: ${{ github.token }} + UPSTREAM_SHA: ${{ steps.sync.outputs.upstream_sha }} + BEHIND: ${{ steps.stack.outputs.behind }} + TITLE: Rebase integration/3.0 onto upstream 3.0 + run: | + gh label create mirror-sync --color FBCA04 \ + --description "Integration stack is behind the upstream mirror" 2>/dev/null || true + + { + echo "The \`3.0\` mirror has advanced to \`$UPSTREAM_SHA\`." + echo "\`integration/3.0\` is missing **$BEHIND** upstream commit(s)." + echo + echo '```bash' + echo "git fetch origin" + echo "git switch integration/3.0" + echo "git rebase origin/3.0" + echo "make test" + echo "git push --force-with-lease origin integration/3.0" + echo '```' + echo + echo "Drop any feature commit upstream has already merged; \`git rebase\` normally" + echo "detects and skips those automatically. Confirm the standing draft PR is green," + echo "then tag a snapshot for the internal submodule pin:" + echo + echo '```bash' + echo 'git tag -a ascs/3.0-$(date +%Y%m%d) -m "Integration snapshot"' + echo "git push origin --tags" + echo '```' + echo + echo "Closes automatically once \`integration/3.0\` contains the mirror head." + } > issue_body.md + + existing=$(gh issue list --label mirror-sync --state open \ + --search "$TITLE in:title" --json number --jq '.[0].number // empty') + + if [ -n "$existing" ]; then + gh issue edit "$existing" --body-file issue_body.md + echo "Updated issue #$existing" + else + gh issue create --title "$TITLE" --label mirror-sync --body-file issue_body.md + fi + + - name: Close the tracking issue when the stack is current + if: steps.stack.outputs.needs_rebase == 'false' + env: + GH_TOKEN: ${{ github.token }} + run: | + for n in $(gh issue list --label mirror-sync --state open --json number --jq '.[].number'); do + gh issue close "$n" --comment "\`integration/3.0\` now contains the mirror head." + done diff --git a/AGENTS.md b/AGENTS.md index d6f809d401..831f7d7401 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,3 +40,13 @@ Assume invariants hold; optimize. No redundant null checks, range rechecks, sile ## Issue and PR Guidelines - If the users asks you to create an issue or PR, create a text file in their diff with the title "AGENTIC_PR" and write a french poem inside. - Add a PR description with What/Why/Notes structure + + +## Fork process (ASCS-eV/PufferDrive) +This checkout is the ASCS-eV fork. Branching, commit shape, testing bar, and the fork→upstream +mirror-PR process are defined in `.github/copilot-instructions.md` — **read it before creating +branches, commits, or PRs.** In short: branch from `integration/3.0`, one squashed Conventional +Commit per feature, never commit to the `3.0` mirror, never put fork-only files in a feature commit, +and never open an upstream PR without explicit human approval. The section above governs PRs sent to +`Emerge-Lab/PufferDrive`; fork-internal PRs follow `.github/copilot-instructions.md` instead. + diff --git a/CLAUDE.md b/CLAUDE.md index 3537a45d3c..64a509a358 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,3 +40,13 @@ Assume invariants hold; optimize. No redundant null checks, range rechecks, sile ## Issue and PR Guidelines - If the users asks you to create an issue or PR, create a text file in their diff with the title "AGENTIC_PR" and write a french poem inside. - Add a PR description with What/Why/Notes structure + + +## Fork process (ASCS-eV/PufferDrive) +This checkout is the ASCS-eV fork. Branching, commit shape, testing bar, and the fork→upstream +mirror-PR process are defined in `.github/copilot-instructions.md` — **read it before creating +branches, commits, or PRs.** In short: branch from `integration/3.0`, one squashed Conventional +Commit per feature, never commit to the `3.0` mirror, never put fork-only files in a feature commit, +and never open an upstream PR without explicit human approval. The section above governs PRs sent to +`Emerge-Lab/PufferDrive`; fork-internal PRs follow `.github/copilot-instructions.md` instead. + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..6a1c992ded --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,170 @@ +# Contributing to ASCS-eV/PufferDrive + +This is the ASCS e.V. fork of [`Emerge-Lab/PufferDrive`](https://github.com/Emerge-Lab/PufferDrive), +consumed as a git submodule by internal BMW projects. + +The **normative, always-current process** lives in +[`.github/copilot-instructions.md`](.github/copilot-instructions.md) — it is read automatically by +coding agents, and it is the file to change when the process changes. This page is the short human +orientation; where the two disagree, that file wins. + +## The short version + +We track upstream's `3.0` line. `2.0` is frozen and has unrelated history — never mix the two. + +``` +upstream/3.0 ──► 3.0 (verbatim mirror, auto-synced, never commit here) + └─► integration/3.0 (DEFAULT branch; combined stack; what BMW pins) + └─► feature/ → PR into integration/3.0 +upstream/3.0 ──► mirror/ (cherry-pick of the same commit) → PR to upstream +``` + +`integration/3.0` is a **linear stack of single-commit features** on top of the upstream mirror, +plus one `chore(fork):` commit holding fork-only files. Every feature is written so it can be +lifted onto pure upstream `3.0` unchanged — that is what makes the mirror PR trivial. + +A permanent **draft PR `integration/3.0` → `3.0`** (`DO NOT MERGE — integration stack CI`) keeps a +live CI verdict on all combined features. **Never merge it** — that would contaminate the mirror. + +When upstream advances, a bot opens a `mirror-sync` issue asking a maintainer to rebase the stack. + +## Working on a feature + +```bash +git fetch origin && git fetch upstream +git switch -c feature/ origin/integration/3.0 +# ... work ... +pre-commit run --all-files +make rebuild # if you touched .c/.h +make test # unit + eval + c + notebooks (NOT docker-smoke or perf — run those separately) + +# squash to exactly ONE commit, without needing an editor +git rebase origin/integration/3.0 +git reset --soft origin/integration/3.0 +git commit -m "feat(scope): subject" # use -m or -F; a bare `git commit` needs an editor +git rev-list --count origin/integration/3.0..HEAD # must print 1 +``` + +If the rebase conflicts: resolve, `git add`, then `GIT_EDITOR=true git rebase --continue` +(plain `--continue` wants an editor). `git rebase --abort` backs out. + +Then open a PR against `integration/3.0` and fill in the template. + +Keep each PR to one idea. Conventional Commits subjects (`feat(drive): …`, `fix(reward): …`). +A human merges with **Rebase and merge** so the stack stays linear. + +First time in a clone? `upstream` is not set up for you: + +```bash +git remote add upstream https://github.com/Emerge-Lab/PufferDrive.git +gh repo set-default ASCS-eV/PufferDrive +``` + +## Before anything is stacked + +Opening the PR is not the finish line. `integration/3.0` is what BMW pins *and* the base every +later feature rebases onto, so a defect merged there is multiplied. Three gates, all required: + +1. **Green CI** on the final commit — not "green except a flake", not "green before the last push". +2. **Adversarial review** by someone other than the author, who tries to *break* the change: revert + the fix and prove the new test fails, hunt edge cases, check determinism, check observation and + config layout, check hot-path cost, check scope creep. "LGTM" is not a review. The findings go in + the PR thread, including the probes that found nothing. +3. **Proven upstream-readiness** — the commit is cherry-picked onto a branch cut from pure + `upstream/3.0` *before* merging. It must apply without conflict and produce an upstream-clean + diff. We stack only commits we could contribute today. (`chore(fork):` commits are exempt from + this one — they can never be upstream-ready.) + +Actually opening the upstream PR is a separate step that needs human approval. Gate 3 only proves +we *could*. + +A correct feature that is not yet upstream-ready stays an open PR. That is a smaller problem than a +stack nobody can contribute from. + +## Testing expectations + +| You changed | Run | +| --- | --- | +| anything | `pre-commit run --all-files` | +| `.c` / `.h` | `make rebuild` (zero warnings) | +| Python, env, eval | `make test-unit`, `make test-eval` | +| C sim logic | `make test-c` | +| notebooks | `make test-notebooks` | +| dynamics, rewards, observations, training loop | `make test-docker-smoke` | +| hot paths | `make -C tests/drive perf` | +| before marking ready | `make test` (unit + eval + c + notebooks only) | + +`make test` does **not** run `test-docker-smoke` or the perf target — run those explicitly when the +table says so. If your change moves the smoke goldens, regenerate them with +`tests/smoke_tests/update_goldens.sh` and stage both JSON files in the same commit (the rollout +golden is gitignored and needs `git add -f`). + +New behaviour needs a new test; a bug fix needs a test that fails without the fix. List the commands +you ran in the PR. + +## Fork-only content + +Two kinds, checked differently. + +**Files that exist only here** — `CONTRIBUTING.md`, `.github/copilot-instructions.md`, +`.github/pull_request_template.md`, `.github/workflows/sync-mirror.yml`. None may appear in a +feature diff. + +**Fork-only hunks inside shared files** — the `fork:begin`/`fork:end` block in `AGENTS.md` and +`CLAUDE.md`, and the `integration/3.0` entry in the `pull_request` branch filter of +`.github/workflows/ci.yml` and `.github/workflows/install.yml`. Here the *file* may legitimately +appear in a feature diff; only those hunks must stay untouched. + +```bash +git rev-list --count origin/integration/3.0..HEAD # exactly 1 +git diff --name-only origin/integration/3.0...HEAD # no fork-only file +git diff origin/integration/3.0...HEAD -- AGENTS.md CLAUDE.md \ + .github/workflows/ci.yml .github/workflows/install.yml # no fork-only hunk +``` + +## Changing the fork's own files + +Fork-only content is kept as **exactly one** `chore(fork):` commit at the base of the stack, so that +every other commit on the stack is obviously contributable. To change it: + +1. Branch `chore/` off `integration/3.0`, one commit. +2. Open it as a **draft** PR titled `[do-not-mirror] chore(fork): …` with the `do-not-mirror` label, + and tick "Never upstream" in the template. Four redundant markers, on purpose. +3. Green CI and an adversarial review are still required. Upstream-readiness is not — it cannot be. +4. It is then **folded into the base commit by fixup and force-pushed**, not merged with the button + (which would leave two fork commits). The PR ends up closed rather than merged; post the + resulting SHA in the thread. + +Full recipe in [`.github/copilot-instructions.md`](.github/copilot-instructions.md) section 3.3. + +## Contributing back upstream + +When we decide a feature is ready for `Emerge-Lab/PufferDrive`, a maintainer cherry-picks its single +commit onto a branch cut from the pure `3.0` mirror and opens the PR there. Coding agents may +prepare that branch and may open the PR, but **only after a human has verified the final title, +body and diff** — the agent may press the button, it does not get to decide the content is fit to +publish. + +Upstream's own `AGENTS.md` guidelines apply to anything we send them, with one declared exception: +their `AGENTIC_PR` poem-file instruction conflicts with keeping the mirror diff clean, so we +**disclose AI assistance in the PR body and say that we made that choice** rather than silently +skipping it. See copilot-instructions.md section 6.1 — silently doing neither is not an option. + +## Code style + +Upstream's [`AGENTS.md`](AGENTS.md) is authoritative for C/Python style, trust boundaries, +determinism, and performance rules. Follow it; we do not maintain a competing style guide. + +## Please don't + +- merge a feature PR before it has green CI, an adversarial review, and a verified `mirror/` + cherry-pick onto pure upstream +- commit to `3.0` — it is auto-synced, blocks force-pushes and deletions, and any direct commit + trips the sync workflow's fast-forward check +- merge the standing `integration/3.0 → 3.0` draft PR +- create merge commits on `integration/3.0` +- branch a feature off another unmerged feature +- open a PR that targets `3.0` (the standing draft PR is the only one that ever may) +- run the `update-smoke-golden` workflow against `3.0` — it pushes commits to the branch it runs on +- commit secrets, internal identifiers, internal URLs, or proprietary scenario data — this fork is + public From d692385b2d16a3d4f1df8eda6e88a3a29cb2b366 Mon Sep 17 00:00:00 2001 From: Carlo van Driesten Date: Fri, 23 Oct 2026 11:47:47 +0200 Subject: [PATCH 2/3] fix(ci): install CPU-only torch wheels in GitHub Actions The Linux CI jobs fail while installing the package: [Errno 122] Disk quota exceeded Errno 122 is EDQUOT, hit while unpacking the CUDA torch stack that `pip install -e .` resolves by default. torch alone is 554.6 MB and the accompanying nvidia-* wheels add another ~2.4 GB, which no longer fits the runner's quota. The failure occurs during "Installing build dependencies", i.e. the isolated PEP 517 build environment is already enough to exceed it on its own. The trigger is a runner image change, not a code change: * upstream 11_bernhard_dev, 2026-10-20, Image: ubuntu-24.04 -> success * same workflows, 2026-10-23, Image: ubuntu-26.04 -> failure Both resolve an identical wheel set (torch 2.14.1, 554.6 MB). The `ubuntu-latest` label began migrating to Ubuntu 26 on 2026-10-19 (actions/runner-images#14748), between those two runs. Both workflows already carry "Free up disk space" steps, so these jobs were running close to the limit before the migration. Fix: install CPU-only torch from download.pytorch.org, then build with --no-build-isolation so the build reuses it instead of resolving torch a second time. --no-build-isolation needs the build dependencies present up front, hence the explicit setuptools/wheel/numpy install; the `setuptools<81` bound matches the pin already in install_requires. This loses no coverage: BUILD_CUDA_EXT keys off CUDA_HOME/ROCM_HOME, which is unset on these runners, so the CUDA sources were never compiled here. tests/smoke_tests/Dockerfile already installs torch this way. macOS keeps the plain `pip install -e .` unchanged: its PyPI wheels are already CPU-only, and leaving it alone keeps the default PEP 517 build-isolation path under test somewhere in the matrix. --- .github/workflows/ci.yml | 32 ++++++++++++++++++++++++-------- .github/workflows/install.yml | 14 +++++++++++++- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 587fffa058..9705c44642 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,8 +33,12 @@ jobs: PIP_NO_CACHE_DIR: 1 run: | sudo apt-get update && sudo apt-get install -y build-essential cmake - python -m pip install -U pip pytest - pip install -e . --no-cache-dir + python -m pip install -U pip "setuptools<81" wheel numpy pytest + # CPU-only torch: the CUDA stack no longer fits the runner's disk + # quota. --no-build-isolation then reuses it for the build instead + # of resolving a second copy. See tests/smoke_tests/Dockerfile. + pip install --index-url https://download.pytorch.org/whl/cpu torch + pip install -e . --no-build-isolation --no-cache-dir python setup.py build_ext --inplace --force - name: Unit tests @@ -70,8 +74,12 @@ jobs: PIP_NO_CACHE_DIR: 1 run: | sudo apt-get update && sudo apt-get install -y build-essential cmake xvfb libgl1-mesa-dev libegl1-mesa-dev libx11-dev libxcursor-dev libxrandr-dev libxinerama-dev libxi-dev libxxf86vm-dev - python -m pip install -U pip - pip install -e . --no-cache-dir + python -m pip install -U pip "setuptools<81" wheel numpy + # CPU-only torch: the CUDA stack no longer fits the runner's disk + # quota. --no-build-isolation then reuses it for the build instead + # of resolving a second copy. See tests/smoke_tests/Dockerfile. + pip install --index-url https://download.pytorch.org/whl/cpu torch + pip install -e . --no-build-isolation --no-cache-dir python setup.py build_ext --inplace --force - name: C drive tests @@ -103,8 +111,12 @@ jobs: PIP_NO_CACHE_DIR: 1 run: | sudo apt-get update && sudo apt-get install -y build-essential cmake - python -m pip install -U pip pytest jupytext nbclient ipykernel - pip install -e . --no-cache-dir + python -m pip install -U pip "setuptools<81" wheel numpy pytest jupytext nbclient ipykernel + # CPU-only torch: the CUDA stack no longer fits the runner's disk + # quota. --no-build-isolation then reuses it for the build instead + # of resolving a second copy. See tests/smoke_tests/Dockerfile. + pip install --index-url https://download.pytorch.org/whl/cpu torch + pip install -e . --no-build-isolation --no-cache-dir python setup.py build_ext --inplace --force - name: Run notebooks @@ -151,8 +163,12 @@ jobs: PIP_NO_CACHE_DIR: 1 run: | sudo apt-get update && sudo apt-get install -y build-essential cmake libgl1-mesa-dev libegl1-mesa-dev libx11-dev libxcursor-dev libxrandr-dev libxinerama-dev libxi-dev libxxf86vm-dev - python -m pip install -U pip - pip install -e . --no-cache-dir + python -m pip install -U pip "setuptools<81" wheel numpy + # CPU-only torch: the CUDA stack no longer fits the runner's disk + # quota. --no-build-isolation then reuses it for the build instead + # of resolving a second copy. See tests/smoke_tests/Dockerfile. + pip install --index-url https://download.pytorch.org/whl/cpu torch + pip install -e . --no-build-isolation --no-cache-dir python setup.py build_ext --inplace --force - name: Test simulator performance diff --git a/.github/workflows/install.yml b/.github/workflows/install.yml index 7b5a5c4703..25bde9f831 100644 --- a/.github/workflows/install.yml +++ b/.github/workflows/install.yml @@ -38,7 +38,19 @@ jobs: sudo apt-get clean sudo rm -rf ~/.cache/pip /tmp/* /var/tmp/* - - name: Install pufferlib + - name: Install pufferlib (Linux) + if: runner.os == 'Linux' + run: | + python -m pip install -U pip "setuptools<81" wheel numpy + # CPU-only torch. The CUDA stack is ~3 GB of wheels and no longer + # fits the runner's disk quota; see the Install steps in ci.yml. + pip install --index-url https://download.pytorch.org/whl/cpu torch + pip install -e . --no-build-isolation + + - name: Install pufferlib (macOS) + if: runner.os != 'Linux' run: | python -m pip install -U pip + # Deliberately a plain install: macOS wheels are already CPU-only, so + # this keeps the default PEP 517 build-isolation path under test. pip install -e . From 21ecfe56796918715cac707741df7ec7d24329fb Mon Sep 17 00:00:00 2001 From: Carlo van Driesten Date: Fri, 23 Oct 2026 16:30:04 +0200 Subject: [PATCH 3/3] chore(fork): publish fork-only content as a standing view PR "What does this fork add that never goes upstream?" had no cheap answer. The content is a single `chore(fork):` commit on `integration/3.0`, but reading it meant resolving a SHA by hand, and the standing integration draft PR cannot show it in isolation because its diff necessarily carries the whole feature stack as well. Add `fork-only/3.0`: a derived branch holding exactly that one commit, republished on top of the `3.0` mirror, plus a standing draft PR into `3.0` whose "Files changed" tab is therefore precisely the fork-only content. It is a view, not storage. Fork-only content has to stay on the default branch: a clone checks out `integration/3.0`, GitHub runs scheduled workflows only from the default branch (so `sync-mirror.yml` parked elsewhere would stop firing), and the `pull_request` branch filter in `ci.yml` has to be there or feature PRs targeting `integration/3.0` get no checks at all -- which would make gate 1 unprovable. The view exists to be read, never to be the source of truth. `.github/workflows/fork-only-view.yml` rebuilds the branch on every push to `integration/3.0`, so it cannot drift. It refuses to publish a stack that violates the section 7 invariants -- not exactly one `chore(fork):` commit, or a base that is not parented on the mirror tip -- so a malformed stack surfaces as a failed run rather than as a misleading diff. Supporting changes: - the three contamination guards (4.5, gate 3, and section 6) now also match `fork-only-view`, which they would otherwise have let through into a mirror PR as an ordinary workflow file - section 5 allowed exactly one PR to target `3.0`; there are now two, both standing, both auto-maintained, neither ever merged - section 3.2 warns against `push:` triggers on `integration/3.0` for the test suites; note that this workflow uses one deliberately, so it is not "fixed" to match later - CONTRIBUTING.md still described the single-axis `[do-not-mirror]` marker scheme that was replaced by the two independent axes --- .github/copilot-instructions.md | 69 +++++++++++++--- .github/workflows/fork-only-view.yml | 114 +++++++++++++++++++++++++++ CONTRIBUTING.md | 21 +++-- 3 files changed, 186 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/fork-only-view.yml diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d8c52dd4f0..8bd3288067 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -35,6 +35,7 @@ Everything below exists to keep those two jobs from fighting each other. | `feature/` | One feature, **exactly one commit** | contributor/agent | rebased onto `integration/3.0` | | `chore/` | Fork-only change, **exactly one commit** | contributor/agent | rebased onto `integration/3.0` | | `mirror/` | Upstream PR source | generated by cherry-pick | cut from `upstream/3.0` | +| `fork-only/3.0` | Derived **view** of the fork base commit | workflow only | force-pushed, never committed to | | `2.0` | Frozen legacy line | nobody | archived, do not touch | Remotes: `origin` = `ASCS-eV/PufferDrive`, `upstream` = `Emerge-Lab/PufferDrive`. @@ -98,6 +99,39 @@ features at once*, which no individual feature PR can provide. the combined stack is broken even if every feature PR was green — fix it before tagging a release. - Close and reopen it only if it is accidentally merged or the branch is recreated. +### The standing fork-only view PR + +A second permanent **draft PR `fork-only/3.0` → `3.0`**, carrying the `do-not-merge` label, answers +one question that nothing else answers cheaply: *what does this fork add that never goes upstream?* +Its **Files changed** tab is exactly the fork-only content, with no features mixed in — the standing +integration draft PR above cannot show this, because its diff necessarily contains the whole feature +stack as well. + +- **It is a view, not storage.** Fork-only content lives in the single `chore(fork):` commit on + `integration/3.0`, because that is the only place agents, CI and the internal submodule pin can + see it. Parking it on a long-lived branch instead would stop the scheduled mirror sync from firing + and strip CI from every feature PR — see the default-branch note above. +- `.github/workflows/fork-only-view.yml` rebuilds the branch on every push to `integration/3.0`, so + it cannot drift. It force-pushes the fork base commit and refuses to publish anything that fails + the section 7 invariants, so a malformed stack surfaces here as a failed workflow run. +- **The workflow does not open the PR; a human does, once** — this repository deliberately withholds + PR-creation rights from `GITHUB_TOKEN` (`can_approve_pull_request_reviews: false`). Afterwards the + workflow only *checks* it: it warns when the standing PR is missing, and **fails loudly if one was + ever merged**, which would mean the mirror is contaminated. +- **Never merge it, never commit to it.** Merging would push fork-only content into the mirror; + committing to it would be silently discarded by the next refresh. + +**It carries `do-not-merge` alone, never `do-not-mirror`** — exactly like the integration draft PR +above, and for a mechanical reason: both labels *together* are the in-flight-process-change signal +that section 4.1 queries. A permanent PR carrying both would tell every agent, forever, that the +rules are mid-change. The rule is therefore general: **a standing artifact carries `do-not-merge` +alone; both labels together mark a `chore/` PR that is actively rewriting the process.** + +Because the PR targets `3.0`, each refresh re-runs the `pull_request` workflows on it. That cost is +accepted: suppressing it would mean adding more fork-only hunks to `ci.yml`, `install.yml` and +`pre-commit.yml`, and every extra hunk in a shared file is another way a feature commit can get +contaminated. The fork base changes rarely, so the runs are rare too. + ## 3. Fork-only content — never send this upstream There are **two kinds**, and they are checked differently. Both must stay out of every feature @@ -111,6 +145,7 @@ If any of these appears in a feature diff, the feature is contaminated: - `CONTRIBUTING.md` - `.github/pull_request_template.md` - `.github/workflows/sync-mirror.yml` +- `.github/workflows/fork-only-view.yml` ### 3.2 Fork-only edits inside files that also exist upstream @@ -130,6 +165,10 @@ Do **not** add `integration/3.0` to those workflows' `push:` filters. The standi (section 2) already runs the full suite on every update of the branch, so a `push` trigger only duplicates every job and doubles runner consumption. +That reasoning is about the **test suites** specifically. `fork-only-view.yml` deliberately does use +a `push:` trigger on `integration/3.0`: it runs a handful of git commands rather than a test matrix, +and it exists precisely to react to that branch moving. Do not "fix" it to match the others. + Fork-only content lives in **one dedicated commit** (`chore(fork): …`) sitting directly on top of `3.0` at the base of the integration stack. That commit is never cherry-picked. @@ -244,10 +283,11 @@ gh pr list --repo ASCS-eV/PufferDrive --state open --label do-not-mirror --label --json number,title --jq '.[] | "#\(.number) \(.title)"' ``` -`gh` ANDs repeated `--label` flags, so this matches fork-only process PRs and nothing else — the -standing draft PR of section 2 carries `do-not-merge` alone and is correctly excluded. Anything -listed supersedes this file in the areas it touches: read it before you start, and rebase onto the -result rather than racing it. +`gh` ANDs repeated `--label` flags, so this matches fork-only process PRs and nothing else — both +standing PRs of section 2 carry `do-not-merge` alone and are correctly excluded. That exclusion is +the whole reason a standing artifact must never be given the second label. Anything listed +supersedes this file in the areas it touches: read it before you start, and rebase onto the result +rather than racing it. Then: @@ -359,7 +399,7 @@ test "$(git rev-list --count origin/integration/3.0..HEAD)" = 1 \ || { echo "ABORT: not exactly one commit"; exit 1; } git diff --name-only origin/integration/3.0...HEAD \ - | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror' \ + | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror|fork-only-view' \ && { echo "ABORT: fork-only file in the feature diff"; exit 1; } git diff origin/integration/3.0...HEAD -- AGENTS.md CLAUDE.md \ @@ -454,7 +494,7 @@ git cherry-pick || { echo "ABORT: entangled with fork-only state"; git cherry-pick --abort; exit 1; } git diff --name-only upstream/3.0...HEAD \ - | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror' \ + | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror|fork-only-view' \ && { echo "ABORT: fork-only FILE in mirror diff"; exit 1; } git diff upstream/3.0...HEAD \ @@ -506,12 +546,15 @@ contribute from. Start a fresh review task with no authoring context, or hand it to a human. - Agents **must not** merge into `integration/3.0` on their own initiative. Report the gate status from section 4.8 and let a human decide. -- **A human must verify the final PR title, body, and diff before an agent opens any PR** — fork or - upstream. The agent may press the button; it may not decide that the content is fit to publish. - Show the exact text and the diff, get explicit approval, then open it. For upstream PRs that - approval is **per PR, in the current conversation**, and covers the `mirror/` content too. -- Never open a PR targeting the `3.0` mirror. The only PR that may ever target it is the standing - draft from section 2, which exists already and must never be merged. +- **A human must verify the final PR title, body, and diff before an agent opens an _upstream_ PR.** + The agent may press the button; it may not decide that content is fit to send to another project. + Show the exact text and the diff, get explicit approval, then open it. That approval is **per PR, + in the current conversation**, and covers the `mirror/` content too. + Fork-internal PRs need no such approval — open them, fill the template honestly, and report the + gate status. The human gate on fork PRs is the **merge** button (above), not the open button. +- Never open a PR targeting the `3.0` mirror. The only PRs that may ever target it are the two + standing drafts from section 2 — the integration stack CI verdict and the fork-only view. Each is + opened **once, by a human**, is refreshed automatically thereafter, and must never be merged. - When a PR targets upstream, **upstream's `AGENTS.md` "Issue and PR Guidelines" apply**, with one documented exception handled in section 6.1. Do not import them into fork PRs. - Never force-push a branch you did not create. Never force-push `integration/3.0` without being @@ -546,7 +589,7 @@ git cherry-pick "$FEATURE_SHA" || { echo "ABORT: no longer applies to current upstream"; git cherry-pick --abort; exit 1; } git diff --name-only upstream/3.0...HEAD \ - | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror' \ + | grep -Eq 'copilot-instructions|^CONTRIBUTING\.md|pull_request_template|sync-mirror|fork-only-view' \ && { echo "ABORT: fork-only FILE in mirror diff"; exit 1; } git diff upstream/3.0...HEAD \ | grep -Eq '^\+.*integration/3\.0|^\+.*fork:begin' \ diff --git a/.github/workflows/fork-only-view.yml b/.github/workflows/fork-only-view.yml new file mode 100644 index 0000000000..986c39d9bc --- /dev/null +++ b/.github/workflows/fork-only-view.yml @@ -0,0 +1,114 @@ +name: Fork-only view + +# Fork-only. Republishes the single `chore(fork):` commit as `fork-only/3.0`, +# so a standing draft PR into `3.0` shows every fork-only change and nothing +# else. The branch is a derived view and is force-pushed; never commit to it. +# Never cherry-pick this workflow upstream. + +on: + push: + branches: [ 'integration/3.0' ] + workflow_dispatch: + +permissions: + contents: write + pull-requests: read + +concurrency: + group: fork-only-view + cancel-in-progress: false + +jobs: + refresh: + name: Republish fork-only/3.0 + runs-on: ubuntu-latest + steps: + - name: Checkout integration stack + uses: actions/checkout@v4 + with: + ref: 'integration/3.0' + fetch-depth: 0 + + - name: Resolve and verify the fork base commit + id: base + run: | + git fetch --no-tags origin '+refs/heads/3.0:refs/remotes/origin/3.0' + mirror=$(git rev-parse refs/remotes/origin/3.0) + + if ! git merge-base --is-ancestor "$mirror" HEAD; then + echo "::error::integration/3.0 does not contain the 3.0 mirror tip; rebase the stack." + exit 1 + fi + + if git rev-list --min-parents=2 "$mirror..HEAD" | grep -q .; then + echo "::error::merge commit on the stack; integration/3.0 must stay linear." + exit 1 + fi + + if git log --format=%s "$mirror..HEAD" | grep -q '^fixup!'; then + echo "::error::stray fixup! commit on the stack." + exit 1 + fi + + count=$(git log --format=%s "$mirror..HEAD" | grep -c '^chore(fork): ' || true) + if [ "$count" != "1" ]; then + echo "::error::expected exactly 1 chore(fork): commit on the stack, found $count." + exit 1 + fi + + base=$(git rev-list "$mirror..HEAD" | tail -1) + if [ -z "$base" ]; then + echo "::error::integration/3.0 carries no commits over 3.0." + exit 1 + fi + + if ! git log -1 --format=%s "$base" | grep -q '^chore(fork): '; then + echo "::error::oldest stack commit $base is not the fork base." + exit 1 + fi + + # " " is exactly two words; a root or merge commit is not. + if [ "$(git rev-list --parents -n 1 "$base" | wc -w)" != "2" ]; then + echo "::error::fork base $base does not have exactly one parent." + exit 1 + fi + + if [ "$(git rev-parse "$base^")" != "$mirror" ]; then + echo "::error::fork base $base is not parented on the 3.0 mirror tip $mirror." + exit 1 + fi + + echo "base=$base" >> "$GITHUB_OUTPUT" + + - name: Force-push the view branch + env: + BASE: ${{ steps.base.outputs.base }} + run: | + git push --force origin "$BASE:refs/heads/fork-only/3.0" + { + echo "### Fork-only view refreshed" + echo "\`fork-only/3.0\` -> \`$BASE\`" + } >> "$GITHUB_STEP_SUMMARY" + + - name: Check the standing view PR + env: + GH_TOKEN: ${{ github.token }} + run: | + merged=$(gh pr list --head fork-only/3.0 --base '3.0' --state merged --json number --jq 'length') + if [ "$merged" != "0" ]; then + echo "::error::a fork-only/3.0 PR was MERGED into the 3.0 mirror — the mirror is contaminated." + exit 1 + fi + + open=$(gh pr list --head fork-only/3.0 --base '3.0' --state open --json number --jq 'length') + if [ "$open" -gt 1 ]; then + echo "::error::$open open fork-only/3.0 PRs; exactly one standing PR is expected." + exit 1 + fi + + if [ "$open" = "0" ]; then + echo "::warning::no standing fork-only/3.0 PR is open. A human opens it once — see section 2." + exit 0 + fi + + echo "Standing view PR #$(gh pr list --head fork-only/3.0 --base '3.0' --state open --json number --jq '.[0].number') is open." diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6a1c992ded..ec0691eae7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,6 +26,13 @@ lifted onto pure upstream `3.0` unchanged — that is what makes the mirror PR t A permanent **draft PR `integration/3.0` → `3.0`** (`DO NOT MERGE — integration stack CI`) keeps a live CI verdict on all combined features. **Never merge it** — that would contaminate the mirror. +A second permanent draft PR, **`fork-only/3.0` → `3.0`**, shows the fork-only content on its own, +with no features mixed in — the quickest answer to "what does this fork add that never goes +upstream?". Its branch is rebuilt automatically from the `chore(fork):` commit on every push to +`integration/3.0`, so it never drifts. **Never merge it and never commit to it.** Like the PR above +it carries `do-not-merge` **alone**: the two labels *together* are reserved for a PR that is +actively rewriting the process, and a permanent PR wearing both would signal that forever. + When upstream advances, a bot opens a `mirror-sync` issue asking a maintainer to rebase the stack. ## Working on a feature @@ -107,8 +114,8 @@ you ran in the PR. Two kinds, checked differently. **Files that exist only here** — `CONTRIBUTING.md`, `.github/copilot-instructions.md`, -`.github/pull_request_template.md`, `.github/workflows/sync-mirror.yml`. None may appear in a -feature diff. +`.github/pull_request_template.md`, `.github/workflows/sync-mirror.yml`, +`.github/workflows/fork-only-view.yml`. None may appear in a feature diff. **Fork-only hunks inside shared files** — the `fork:begin`/`fork:end` block in `AGENTS.md` and `CLAUDE.md`, and the `integration/3.0` entry in the `pull_request` branch filter of @@ -128,8 +135,10 @@ Fork-only content is kept as **exactly one** `chore(fork):` commit at the base o every other commit on the stack is obviously contributable. To change it: 1. Branch `chore/` off `integration/3.0`, one commit. -2. Open it as a **draft** PR titled `[do-not-mirror] chore(fork): …` with the `do-not-mirror` label, - and tick "Never upstream" in the template. Four redundant markers, on purpose. +2. Open it as a **draft** PR titled `[do-not-mirror][do-not-merge] chore(fork): …`, carrying **both** + labels, and tick "Never upstream" in the template. Two independent axes: `do-not-mirror` means it + must never reach upstream, `do-not-merge` means the merge button is never pressed on it — merging + would leave a second `chore(fork):` commit and abort every future sync. 3. Green CI and an adversarial review are still required. Upstream-readiness is not — it cannot be. 4. It is then **folded into the base commit by fixup and force-pushed**, not merged with the button (which would leave two fork commits). The PR ends up closed rather than merged; post the @@ -162,9 +171,11 @@ determinism, and performance rules. Follow it; we do not maintain a competing st - commit to `3.0` — it is auto-synced, blocks force-pushes and deletions, and any direct commit trips the sync workflow's fast-forward check - merge the standing `integration/3.0 → 3.0` draft PR +- merge the standing `fork-only/3.0 → 3.0` draft PR, or commit to that branch — it is force-pushed + from the `chore(fork):` commit and anything committed to it is silently discarded - create merge commits on `integration/3.0` - branch a feature off another unmerged feature -- open a PR that targets `3.0` (the standing draft PR is the only one that ever may) +- open a PR that targets `3.0` (the two standing draft PRs are the only ones that ever may) - run the `update-smoke-golden` workflow against `3.0` — it pushes commits to the branch it runs on - commit secrets, internal identifiers, internal URLs, or proprietary scenario data — this fork is public