diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..8bd3288067 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,973 @@ +# 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` | +| `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`. + +`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. + +### 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 +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` +- `.github/workflows/fork-only-view.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. + +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. + +### 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 — 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: + +```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|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 \ + .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|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' \ + && { 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 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 + 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|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' \ + && { 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..9705c44642 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' ] @@ -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/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/.github/workflows/install.yml b/.github/workflows/install.yml index 27f8b10bb1..25bde9f831 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' ] @@ -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 . 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..ec0691eae7 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,181 @@ +# 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. + +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 + +```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`, +`.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 +`.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][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 + 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 +- 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 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