[probe] exercise the feature path of the Fork gates check

.github/fork/PROCESS.md

@@ -12,7 +12,8 @@ Read the section you need, when you need it. The core tells you which one:
 | --- | --- |
 | merge or close one of the two standing draft PRs | [Standing PRs](#the-standing-draft-prs) |
 | change a fork-only file (`chore/<slug>`) | [Changing fork-only content](#changing-fork-only-content) |
-| review someone else's feature PR | [Adversarial review](#adversarial-review) |
+| review a Track A (sim-affecting) feature PR | [Adversarial review](#adversarial-review) |
+| regenerate the smoke goldens | [Smoke goldens](#smoke-goldens) |
 | open, refresh, or retire an upstream PR | [Mirror PR workflow](#mirror-pr-workflow-fork--upstream) |
 | rebase the stack onto new upstream, or cut a release | [Syncing and releasing](#syncing-and-releasing) |
 
@@ -61,10 +62,17 @@ stack as well.
   is a far worse trade than one extra line in two procedures that are already manual.
 - `.github/workflows/fork-only-view.yml` is therefore a **detector, not a publisher**, and needs no
   write access at all (`contents: read`). It re-derives the fork base, **fails if `fork-only/3.0` is
-  missing or stale** and prints the exact command to fix it, refuses to pass a stack that violates
-  the stack invariants in *Syncing and releasing*, and checks that the standing PR still has base `3.0`, draft status and
-  the `do-not-merge` label. It runs on pushes to `integration/3.0` **and to `fork-only/3.0`** (so
-  tampering with the view itself is caught), plus daily, plus on demand. Drift is loud, not silent.
+  missing or stale** and prints the exact command to fix it, and checks that the standing PR still
+  has base `3.0`, draft status and the `do-not-merge` label. It runs on pushes to `integration/3.0`
+  **and to `fork-only/3.0`** (so tampering with the view itself is caught), plus daily, plus on
+  demand. Drift is loud, not silent.
+- **A stack that is behind upstream is not view drift, and does not fail this job.** It is the
+  normal window between upstream moving and the next rebase, and it is owned by `sync-mirror.yml`,
+  which tracks it in an issue. `forkctl check-view` reports it with `::warning::` and then measures
+  the remaining invariants against the mirror the stack was actually built on. This matters: before
+  the split, one outstanding rebase turned the job red *and* returned early, so the view-drift and
+  standing-PR checks — the only reason the job exists — silently stopped running for as long as the
+  rebase was outstanding. An alarm that disables the detector it guards is worse than no alarm.
 - **The workflow does not open the PR either; 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
@@ -169,8 +177,15 @@ commit — do not fold it into the feature commit.
 
 ## 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.
+**This applies to Track A only** — a change touching `pufferlib/`, `config/`, `pufferl.py`,
+`models.py`, `setup.py` or the smoke goldens, as decided by `forkctl track`. Track B changes (docs,
+fork tooling, workflows, CI, build scripts, tests) get one human read against the three questions in
+core section 4.7, and none of this checklist. Spending a fresh-context agent and a written probe
+list on a Markdown edit is the cost the tiering exists to remove; skipping them on a reward change
+is the risk it exists to keep paying for.
+
+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
@@ -199,6 +214,43 @@ Post the outcome as a PR review listing what was probed and what was found — *
 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.
 
+**Do not spend a review comment on anything a tool owns.** Whitespace, import order, line length,
+formatting and wording preference belong to `pre-commit`; commit shape and fork-only contamination
+belong to `forkctl`. If one of those reaches review, the finding is "the tool did not catch this" —
+report it once and fix the tool. Hand-annotating instances costs the author a round trip, costs an
+agent a full context reload, and leaves the next change just as exposed.
+
+## Smoke goldens
+
+The golden train/rollout values are a *behavioural* fixture: they change only when observation
+layout, dynamics, rewards or RNG draw order change. If your change legitimately moves them,
+regenerate inside the pinned QEMU image so the numbers match CI, rather than on your own machine:
+
+```bash
+tests/smoke_tests/update_goldens.sh
+```
+
+Review the diff, then stage both files **in the same single commit as the change that caused them**.
+`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:
+
+```bash
+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`: our stack may already carry a feature that moved them. So if Gate 3 shows the
+smoke test failing on `mirror/<slug>`, regenerate *there* and carry the corrected values back —
+never assume our stack's numbers are upstream's.
+
+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, because Gate 3 would be a
+promise we could not keep.
+
+Touching the goldens makes a change Track A by construction (`forkctl track` lists
+`tests/smoke_tests/data/` as sim-affecting), so the full review above applies.
+
 ## Mirror PR workflow (fork → upstream)
 
 Gate 3 (section 4.8) already proved this commit cherry-picks cleanly. Do **not** assume the branch
@@ -434,10 +486,17 @@ drawing any conclusion from `git cherry`, which compares the stale patch rather
 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.
+When the mirror moves ahead of the stack, the same workflow opens (and keeps updated) a tracking
+issue titled `Rebase integration/3.0 onto upstream 3.0`, and closes it automatically once
+`integration/3.0` contains the mirror head. It is matched by **exact title, not by a label**: a
+label is a second object that must exist before the issue can be filed, and a masked
+`gh label create ... || true` once failed silently while every scheduled run went red on the
+`gh issue create` that depended on it.
+
+That issue is the **only** alarm for an outstanding rebase, and nothing fails while it is open.
+Being behind upstream is the normal window between upstream moving and the next rebase, not a
+defect, and a daily red cron for a known state trains everyone to ignore the board. The rebase
+itself stays manual — replaying features onto new upstream code can conflict and must be re-tested.
 
 **Rebase the stack** (maintainers):
 
@@ -521,7 +580,7 @@ and the second kind belongs in a `chore(fork):` commit where the guards can see
 `git push origin HEAD:<ref>`, 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
+and the rebase tracking 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

.github/fork/forkctl.py

@@ -39,6 +39,28 @@
 
 FORK_COMMIT_PREFIX = "chore(fork): "
 
+# Paths whose behaviour the submodule consumers depend on: the simulation, the
+# policy, the training loop and the config that drives them. A commit touching
+# any of them is Track A and pays the full review cost (section 4.7); anything
+# else is Track B. Defined here, not in prose, so the two cannot drift apart.
+# The list is deliberately coarse: over-classifying costs a review, while
+# under-classifying ships unreviewed sim behaviour.
+SIM_PATHS = (
+    "pufferl.py",
+    "models.py",
+    "setup.py",
+)
+
+SIM_PREFIXES = (
+    "pufferlib/",
+    "config/",
+    "tests/smoke_tests/data/",
+)
+
+# Documentation never changes shipped behaviour, so it stays Track B even under
+# a Track A prefix. Everything else under those prefixes is Track A.
+NON_SIM_SUFFIXES = (".md",)
+
 # Files that exist only in this fork; none may appear in a feature diff. This
 # tuple is the single source of truth. The docs point here instead of repeating
 # the list, so adding a fork-only file cannot leave a stale copy behind.
@@ -48,6 +70,7 @@
     ".github/pull_request_template.md",
     ".github/workflows/sync-mirror.yml",
     ".github/workflows/fork-only-view.yml",
+    ".github/workflows/fork-gates.yml",
 )
 
 # Anything under these prefixes is fork-only by construction, so a new fork-only
@@ -734,6 +757,37 @@ def current_branch() -> str:
     return git("rev-parse", "HEAD") if branch == "HEAD" else branch
 
 
+def is_sim_path(path: str) -> bool:
+    if path.endswith(NON_SIM_SUFFIXES):
+        return False
+    return path in SIM_PATHS or path.startswith(SIM_PREFIXES)
+
+
+def track_of(base: str, head: str = "HEAD") -> tuple[str, list[str]]:
+    """Section 4.7: which review track this diff falls into, and why.
+
+    Returns ("A", <the sim paths that forced it>) or ("B", []).
+    """
+    sim = sorted(path for path in diff_files(base, head) if is_sim_path(path))
+    return ("A" if sim else "B"), sim
+
+
+def cmd_track(args: argparse.Namespace) -> int:
+    track, sim = track_of(args.base, args.head)
+    if args.quiet:
+        print(track)
+        return 0
+    report = Report("track", args.format)
+    if track == "A":
+        report.info(f"Track A -- sim-affecting ({len(sim)} path(s)); full gates and adversarial review")
+        for path in sim:
+            report.info(f"  {path}")
+    else:
+        report.info("Track B -- no sim-affecting path; CI, forkctl and a human read are the gate")
+    print(track, file=report.stream)
+    return 0
+
+
 def cmd_fork_only(args: argparse.Namespace) -> int:
     for path in FORK_ONLY_PATHS:
         print(path)
@@ -763,15 +817,13 @@ def cmd_check_commit(args: argparse.Namespace) -> int:
         f"git rebase {args.base} && git reset --soft {args.base} && git commit -F msg",
     )
 
-    # One commit *ahead* is not the same as rebased onto the current base: a
-    # branch cut weeks ago is still one commit ahead of where it started.
-    if one_commit:
-        report.check(
-            git("rev-parse", "HEAD^") == git("rev-parse", args.base),
-            f"rebased directly onto {args.base}",
-            f"the commit is not parented on the current {args.base} tip",
-            f"git rebase {args.base}",
-        )
+    # Being behind the base is not a defect. "Rebase and merge" replays the
+    # commit itself and refuses when it cannot, so the merge button already
+    # enforces linearity; failing here only forces every open PR to rebase and
+    # re-run its gates each time anything else lands.
+    if one_commit and git("rev-parse", "HEAD^") != git("rev-parse", args.base):
+        behind = len(git_lines("rev-list", f"HEAD^..{args.base}"))
+        report.warn(f"{behind} commit(s) behind {args.base}; rebase before merging if the button objects")
 
     subject = git("log", "-1", "--format=%s") if commits else ""
     problems = subject_problems(subject)
@@ -814,8 +866,15 @@ def cmd_check_commit(args: argparse.Namespace) -> int:
 
     if args.chore:
         report.info("fork-only file and hunk checks: exempt (chore(fork) branch)")
+        report.info("review track: B (fork-only infrastructure is never sim-affecting)")
         return report.finish()
 
+    track, sim = track_of(args.base)
+    if track == "A":
+        report.info(f"review track: A -- sim-affecting, full gates and adversarial review ({len(sim)} path(s))")
+    else:
+        report.info("review track: B -- CI, forkctl and a human read are the gate (section 4.7)")
+
     offenders = fork_only_files_in(args.base)
     report.check(
         not offenders,
@@ -843,16 +902,32 @@ def cmd_check_commit(args: argparse.Namespace) -> int:
     return report.finish()
 
 
-def verify_stack(report: Report, mirror_ref: str, stack_ref: str) -> str | None:
-    """Sections 3.3 and 7: stack invariants. Returns the fork base, or None."""
+def verify_stack(report: Report, mirror_ref: str, stack_ref: str, behind_is_notice: bool = False) -> str | None:
+    """Sections 3.3 and 7: stack invariants. Returns the fork base, or None.
+
+    A stack that is behind the mirror is a *sync* state, not a malformed stack:
+    it is the normal window between upstream moving and the next rebase, and it
+    is owned and reported by `sync-mirror.yml`. Callers that police something
+    else pass `behind_is_notice` so that window does not mask their own checks.
+    """
     if not require_refs(report, mirror_ref, stack_ref):
         return None
 
     mirror = git("rev-parse", mirror_ref)
     stack = git("rev-parse", stack_ref)
 
     contains = git_rc("merge-base", "--is-ancestor", mirror, stack) == 0
-    if not report.check(
+    if not contains and behind_is_notice:
+        behind = len(git_lines("rev-list", f"{stack}..{mirror}"))
+        report.warn(
+            f"{stack_ref} is {behind} commit(s) behind {mirror_ref}; "
+            "the rebase is tracked by sync-mirror.yml, not by this check"
+        )
+        # Measure against the mirror the stack was actually built on, so the
+        # remaining invariants still describe this stack rather than failing
+        # for the one reason that is already reported elsewhere.
+        mirror = git("merge-base", mirror, stack)
+    elif not report.check(
         contains,
         f"{stack_ref} contains the {mirror_ref} tip",
         f"{stack_ref} does not contain the {mirror_ref} tip",
@@ -1025,13 +1100,16 @@ def resolve_feature_commit(report: Report, args: argparse.Namespace) -> str | No
                 "Gate 3 proves the commit under review; use --subject to rebuild a stacked commit",
             )
             return None
-        parented_on_tip = git_rc("rev-parse", "--verify", "--quiet", f"{sha}^") == 0 and git(
-            "rev-parse", f"{sha}^"
-        ) == git("rev-parse", args.stack)
-        if not parented_on_tip:
+        # The invariant is "one commit, and its parent is on the stack line" --
+        # not "parented on the tip". A branch that is merely behind still has
+        # exactly one commit to prove, and failing it here would drag every open
+        # PR through a rebase each time something else lands (4.4).
+        parent_exists = git_rc("rev-parse", "--verify", "--quiet", f"{sha}^") == 0
+        on_stack_line = parent_exists and git_rc("merge-base", "--is-ancestor", f"{sha}^", args.stack) == 0
+        if not on_stack_line:
             report.fail(
-                f"{head} is not a single commit on top of the {args.stack} tip",
-                "squash to one commit and rebase onto the current stack tip (section 4.4)",
+                f"{head} is not a single commit on top of {args.stack}",
+                "squash to one commit branched from the stack (section 4.4)",
             )
             return None
         return sha
@@ -1126,6 +1204,8 @@ def cmd_mirror(args: argparse.Namespace) -> int:
             candidate = git("rev-parse", "HEAD")
             if report.failed:
                 report.info(f"{branch} was left untouched; nothing was published")
+            elif args.no_publish:
+                report.info(f"checks only: {branch} was not moved")
             else:
                 if ref_exists(branch) and git("rev-parse", branch) != candidate:
                     report.info(f"{branch} moved from {git('rev-parse', branch)[:9]} to {candidate[:9]}")
@@ -1218,9 +1298,15 @@ def cmd_delete_mirror(args: argparse.Namespace) -> int:
 
 
 def cmd_check_view(args: argparse.Namespace) -> int:
-    """Section 2: fork-only/3.0 is a derived view, republished by hand."""
+    """Section 2: fork-only/3.0 is a derived view, republished by hand.
+
+    Whether the stack is current with upstream is deliberately not this check's
+    business: that is reported once, by sync-mirror.yml. Failing on it here
+    would skip every assertion below and silently disable the drift detector
+    for as long as a rebase is outstanding.
+    """
     report = Report("check-view", args.format)
-    base = verify_stack(report, args.mirror, args.stack)
+    base = verify_stack(report, args.mirror, args.stack, behind_is_notice=True)
     if base is None:
         return report.finish()
     report.ok(f"stack is sound; fork base is {base[:9]}")
@@ -1488,6 +1574,13 @@ def add_fmt(p: argparse.ArgumentParser) -> None:
     add_fmt(p)
     p.set_defaults(func=cmd_check_commit)
 
+    p = sub.add_parser("track", help="review track for a diff (section 4.7)")
+    p.add_argument("--base", default=STACK_REF)
+    p.add_argument("--head", default="HEAD")
+    p.add_argument("--quiet", action="store_true", help="print only A or B")
+    add_fmt(p)
+    p.set_defaults(func=cmd_track)
+
     p = sub.add_parser("check-stack", help="stack invariants (sections 3.3, 7)")
     p.add_argument("--mirror", default=MIRROR_REF)
     p.add_argument("--stack", default=STACK_REF)
@@ -1514,7 +1607,9 @@ def add_fmt(p: argparse.ArgumentParser) -> None:
     p.add_argument("--upstream", default=UPSTREAM_REF)
     p.add_argument("--mirror", default=MIRROR_REF)
     p.add_argument("--stack", default=STACK_REF)
-    p.add_argument("--stay", action="store_true", help="stay on the mirror branch")
+    publish = p.add_mutually_exclusive_group()
+    publish.add_argument("--stay", action="store_true", help="stay on the mirror branch")
+    publish.add_argument("--no-publish", action="store_true", help="prove the gate without moving mirror/<slug>")
     add_fmt(p)
     p.set_defaults(func=cmd_mirror)