PROP-3: durable-at-propose via a bounded push to prop/* refs only

docs/proposals/PROP-3-durable-at-propose-via-a-bounded-push-to-prop-refs-only.md

@@ -0,0 +1,82 @@
+# PROP-3 — Durable-at-propose via a bounded push to prop refs only
+
+_proposed 2026-08-15T20:05:22.575750Z  |  risk class: **infrastructure**  |  status: **IMPLEMENTED — awaiting human merge**_
+
+## Observation
+
+PROP-2 made the proposal durable by committing it at propose time, but only on the machine it ran from. With the protect-main ruleset in force a direct push to main is rejected, so a proposal committed on main can never reach origin: the record survives a branch switch and a branch deletion, which were the observed PROP-1 failures, but it does not survive the loss of this checkout and is invisible to any other clone. A proposal that is written and never implemented therefore still has no off-machine trace, which is the gap PROP-2 set out to close and only half closed. The remaining risk is asymmetric - the analysis is exactly the artifact that is cheap to produce and expensive to reconstruct, because it carries the dated reasoning and the evidence paths that were true at the time.
+
+### Evidence
+
+- `docs/proposals`
+- `docs/decisions.md`
+
+## Proposed change
+
+propose seeds a branch for the proposal and pushes it, so the record is durable on the remote the moment the analysis is made. The push target is bounded IN CODE: a guard validates the ref name and raises UnsafePushTarget for anything that is not a prop ref, and it raises BEFORE any git subprocess is constructed, so a bad target cannot reach the network even transiently. No pull request is opened at propose time - a proposal is not a request to merge anything, and opening one would put unreviewed analysis into the review queue. implement opens the pull request once every gate has passed, which is the first moment a request to merge is meaningful. The human merge gate is unchanged and implement still never merges.
+
+## Affected files
+
+- `src/quantlab/improve/propose.py`
+- `src/quantlab/improve/implement.py`
+- `tests/test_improve_pipeline.py`
+
+## Risk class
+
+**infrastructure**
+
+## Test plan
+
+A recording runner is injected that captures every git invocation and executes none. The guard test asserts that a non-prop ref raises UnsafePushTarget AND that the recorder captured exactly zero invocations, so before any git call is an observable property rather than an ordering claim. Parametrised over main, origin/main, an empty string, a bare prefix, and a traversal attempt. Further tests assert propose pushes only its own ref, that no pull request is opened at propose time, that implement opens one when gates pass and does not when they fail, and that the existing proofs that implement never touches main stay green. Full pytest, ruff, mypy, and CI must be green before human review.
+
+## Firewall
+
+```
+FIREWALL PASS — no forbidden path or change class touched.
+```
+
+## Merge gate
+
+`implement` stops after pushing the branch. **Merge is human-only:** Daniel merges via pull request after Quant Lead review. No automated path to `main` exists in this pipeline.
+
+---
+
+<!-- IMPLEMENTATION REPORT ANCHOR -->
+
+## Implementation report
+
+_implemented 2026-08-15T20:15:27.865319Z  |  branch `prop/3`  |  status: **GATES PASSED**_
+
+### Diff stat
+
+```
+src/quantlab/improve/implement.py |  63 ++++++++++++++++-
+ src/quantlab/improve/propose.py   | 145 ++++++++++++++++++++++++++++++++++++--
+ tests/test_improve_pipeline.py    | 132 ++++++++++++++++++++++++++++++++--
+ 3 files changed, 328 insertions(+), 12 deletions(-)
+```
+
+### Firewall re-check (against the actual diff)
+
+```
+FIREWALL PASS — no forbidden path or change class touched.
+```
+
+### Gates
+
+| gate | result | detail |
+|---|---|---|
+| `ruff` | PASS | All checks passed! |
+| `mypy` | PASS | Success: no issues found in 71 source files |
+| `pytest` | PASS | 691 passed, 1 warning in 84.84s (0:01:24) |
+| `frontend` | SKIP | no frontend/ path in the diff |
+| `verify-dist` | SKIP | site not touched |
+
+### Branch
+
+- branch: `prop/3`
+- commit and push: performed immediately after this report was written into the proposal, since the report is part of what gets committed. The resulting SHA and push result are in the run output, and the commit itself is the one carrying this file.
+
+### Merge gate — STOPPED HERE
+
+This pipeline does not merge. The change sits on `prop/3` and `main` is untouched. **Daniel merges via pull request after Quant Lead review.** There is no automated path to `main` in `quantlab implement` — verified by test, not by convention.

src/quantlab/improve/implement.py

@@ -94,6 +94,8 @@ class ImplementResult:
     commit_sha: str = ""
     pushed: bool = False
     push_detail: str = ""
+    pr_url: str = ""
+    pr_detail: str = ""
     aborted: str = ""
     # False while the report is being written INTO the proposal, true once the run has
     # finished. The report has to be written before the commit — it is part of what gets
@@ -142,6 +144,7 @@ def render(self) -> str:
             if self.commit_sha:
                 lines.append(f"- commit: `{self.commit_sha}`")
             lines.append(f"- pushed: **{self.pushed}** — {self.push_detail or 'n/a'}")
+            lines.append(f"- pull request: {self.pr_detail or 'n/a'}")
         else:
             lines.append(
                 "- commit and push: performed immediately after this report was written "
@@ -220,7 +223,15 @@ def abort(reason: str) -> ImplementResult:
     # -- 1. branch ---------------------------------------------------------
     # `checkout -B` is deliberate: re-running `implement` for the same proposal resets
     # the branch rather than failing or stacking a second attempt on the first.
-    made = _git(run, repo, "checkout", "-B", branch)
+    # JOIN the branch if it exists rather than resetting it. `propose` now seeds
+    # `prop/{n}` with the proposal commit and pushes it, so the old `checkout -B` would
+    # have thrown that commit away and then failed to push as a non-fast-forward — which
+    # is exactly what happened by hand during the PROP-1 rebuild.
+    exists = _git(run, repo, "rev-parse", "--verify", f"refs/heads/{branch}").returncode == 0
+    made = (
+        _git(run, repo, "checkout", branch) if exists
+        else _git(run, repo, "checkout", "-b", branch)
+    )
     if made.returncode != 0:
         return abort(f"could not create branch {branch}: {made.stderr.strip()}")
 
@@ -299,12 +310,62 @@ def abort(reason: str) -> ImplementResult:
     else:
         result.push_detail = "push suppressed (--no-push)"
 
+    # -- 9. open the PR, but ONLY once every gate passed --------------------
+    #
+    # This is the first moment "please merge this" is a meaningful thing to say. `propose`
+    # deliberately does not open one: a proposal is an observation, not a request, and
+    # most should be readable without entering a review queue. Opening a PR is also the
+    # closest this pipeline comes to the merge gate, so it is conditioned on the gates
+    # rather than on the push — a red branch stays a branch, visible but not queued.
+    if result.pushed and result.gates_ok:
+        result.pr_url, result.pr_detail = _open_pull_request(run, repo, branch, number)
+    elif result.pushed:
+        result.pr_detail = "no PR opened — gates did not all pass; branch pushed as evidence"
+    else:
+        result.pr_detail = "no PR opened — nothing was pushed"
+
     # Only the console rendering may claim the commit and push, and only now that both
     # have actually happened. There is deliberately nothing after this point.
     result.finalised = True
     return result
 
 
+def _open_pull_request(
+    run: Runner, repo: Path, branch: str, number: int
+) -> tuple[str, str]:
+    """Open a PR from ``branch`` into the protected trunk. Never merges it.
+
+    `gh pr create` and nothing else. There is no `gh pr merge` here, no `--auto`, and no
+    admin override — the source-level test that forbids merge verbs covers those spellings
+    too, so a future "just enable auto-merge" would fail the suite rather than ship.
+    """
+    # The same bounded-namespace guard `propose` pushes through. A PR is opened from a
+    # branch, so the branch had better be one of ours.
+    propose_mod.assert_prop_ref(branch)
+
+    existing = run(["gh", "pr", "list", "--head", branch, "--json", "url"], repo)
+    if existing.returncode == 0 and existing.stdout.strip() not in ("", "[]"):
+        return "", f"PR already open for {branch}; left as is"
+
+    created = run([
+        "gh", "pr", "create",
+        "--base", PROTECTED_BRANCH,
+        "--head", branch,
+        "--title", f"PROP-{number}: implemented, gates green",
+        "--body",
+        f"Implemented by `quantlab implement` on `{branch}`. The proposal and its full "
+        f"implementation report are in `docs/proposals/PROP-{number}-*.md` on this branch.\n\n"
+        f"Every gate passed before this PR was opened. **Merge is human-only:** Daniel "
+        f"merges after Quant Lead review, and the required status check must be green. "
+        f"`quantlab implement` has no merge path.",
+    ], repo)
+    if created.returncode != 0:
+        detail = (created.stderr or created.stdout).strip().splitlines()
+        return "", f"PR not opened: {detail[-1] if detail else 'unknown error'}"
+    url = created.stdout.strip().splitlines()[-1] if created.stdout.strip() else ""
+    return url, f"opened {url}" if url else "opened"
+
+
 def _run_gates(run: Runner, repo: Path, changed: Sequence[str]) -> list[Gate]:
     """ruff, mypy, pytest, the frontend suite, and verify-dist when the site is touched."""
     gates: list[Gate] = []

src/quantlab/improve/propose.py

@@ -19,6 +19,7 @@
 
 import re
 import subprocess
+from collections.abc import Callable
 from dataclasses import dataclass, field
 from datetime import UTC, datetime
 from pathlib import Path
@@ -28,10 +29,60 @@
 
 PROPOSALS_DIR = PROJECT_ROOT / "docs" / "proposals"
 
-# The two states a proposal file can be in. `propose` writes AWAITING and commits it
-# where it was run; `implement` flips it to IMPLEMENTED on the prop branch only, so the
-# trunk keeps saying AWAITING until a human merges. The status therefore answers "has
-# this been done?" honestly from whichever branch you are reading.
+# THE ONLY NAMESPACE THIS COMMAND MAY EVER PUSH.
+# `propose` publishes the analysis so it survives the loss of this checkout. That means it
+# needs the network, and a command that can push is a command that can push to the wrong
+# place. The blast radius is therefore bounded in code rather than by care: `assert_prop_ref`
+# is the single chokepoint every push goes through, and it raises BEFORE any subprocess is
+# constructed, so a bad target never reaches the network even transiently.
+PROP_REF_PREFIX = "prop/"
+
+# Characters that turn a ref into something other than a plain branch name — refspec
+# separators, globs, and the reflog/ancestry operators. None of them belong in a name this
+# command generates, so their presence means the value did not come from where we think.
+_REF_FORBIDDEN = frozenset(':+~^?*[]\\ \t\n')
+
+
+class UnsafePushTarget(RuntimeError):
+    """A push was attempted at something outside the `prop/*` namespace."""
+
+
+def assert_prop_ref(ref: str) -> str:
+    """Validate a push target. Raises :class:`UnsafePushTarget` before any git runs.
+
+    Fails closed on everything that is not obviously a `prop/<something>` branch name.
+    `main` and `origin/main` are the targets that matter, but the traversal case
+    (`prop/../main`) is why a prefix check alone is not enough: it satisfies
+    `startswith("prop/")` and still names the trunk.
+    """
+    if not isinstance(ref, str) or not ref:
+        raise UnsafePushTarget(f"refusing to push: empty or non-string ref {ref!r}")
+    if not ref.startswith(PROP_REF_PREFIX):
+        raise UnsafePushTarget(
+            f"refusing to push {ref!r}: `propose` may only push refs under "
+            f"{PROP_REF_PREFIX!r}. This is enforced in code, not by convention — there is "
+            f"no flag that widens it."
+        )
+    remainder = ref[len(PROP_REF_PREFIX):]
+    if not remainder:
+        raise UnsafePushTarget(f"refusing to push bare prefix {ref!r}")
+    if ".." in ref or ref.endswith("/") or "//" in ref:
+        raise UnsafePushTarget(
+            f"refusing to push {ref!r}: path traversal or empty segment. A prefix check "
+            f"alone would accept 'prop/../main', which names the trunk."
+        )
+    bad = sorted(set(ref) & _REF_FORBIDDEN)
+    if bad:
+        raise UnsafePushTarget(
+            f"refusing to push {ref!r}: illegal character(s) {''.join(bad)!r} in a ref name"
+        )
+    return ref
+
+
+# The two states a proposal file can be in. `propose` writes AWAITING; `implement` flips
+# it to IMPLEMENTED on the prop branch only, so the trunk keeps saying AWAITING until a
+# human merges. The status therefore answers "has this been done?" honestly from
+# whichever branch you are reading.
 STATUS_AWAITING = "AWAITING IMPLEMENTATION"
 STATUS_IMPLEMENTED = "IMPLEMENTED — awaiting human merge"
 
@@ -209,6 +260,82 @@ def git(*args: str) -> subprocess.CompletedProcess[str]:
     return f"committed {sha} on {branch}"
 
 
+def push_prop_ref(
+    ref: str,
+    *,
+    root: Path | None = None,
+    runner: Callable[..., subprocess.CompletedProcess[str]] | None = None,
+) -> str:
+    """Push ``ref`` to origin. THE GUARD RUNS FIRST, before any subprocess exists.
+
+    That ordering is the point, and it is asserted as an observable rather than claimed:
+    the test injects a recording runner, calls this with ``main``, and requires both that
+    :class:`UnsafePushTarget` is raised and that the recorder captured **zero** git
+    invocations. A guard that ran after the argv was assembled would still be a guard, but
+    it would not be one you could prove had never reached the network.
+    """
+    assert_prop_ref(ref)  # <- first statement. Nothing above it may touch git.
+
+    repo = root if root is not None else PROJECT_ROOT
+    run = runner if runner is not None else _subprocess_runner
+    pushed = run(["git", "push", "--set-upstream", "origin", ref], repo)
+    if pushed.returncode != 0:
+        detail = (pushed.stderr or pushed.stdout).strip().splitlines()
+        return f"NOT PUSHED ({detail[-1] if detail else 'unknown error'})"
+    return f"pushed origin/{ref}"
+
+
+def _subprocess_runner(
+    cmd: list[str], cwd: Path
+) -> subprocess.CompletedProcess[str]:
+    return subprocess.run(
+        list(cmd), cwd=str(cwd), capture_output=True, text=True, shell=False,
+    )
+
+
+def publish_proposal(
+    path: Path,
+    number: int,
+    *,
+    root: Path | None = None,
+    runner: Callable[..., subprocess.CompletedProcess[str]] | None = None,
+) -> str:
+    """Seed ``prop/{number}`` with the proposal and push it, then return to where we were.
+
+    NO PULL REQUEST IS OPENED HERE. A proposal is not a request to merge anything — it is
+    an observation with evidence, and most of them should be readable without ever
+    entering a review queue. `implement` opens the PR once the gates pass, which is the
+    first moment "please merge this" is a meaningful thing to say.
+
+    Returning to the original branch matters: `propose` is an analysis command and must
+    not leave the operator somewhere they did not ask to be.
+    """
+    repo = root if root is not None else PROJECT_ROOT
+    run = runner if runner is not None else _subprocess_runner
+    ref = f"{PROP_REF_PREFIX}{number}"
+    assert_prop_ref(ref)
+
+    # Commit WHERE WE ARE first — PROP-2's local durability, unchanged. The proposal has
+    # to stay in the working tree: an earlier draft of this function checked out `prop/n`
+    # to commit there and then returned, which made the file vanish from the operator's
+    # tree and left `implement` unable to find it. That is the PROP-1 failure in a new
+    # costume, and it is why the branch is created by POINTER here rather than by
+    # checkout — no HEAD movement, nothing to restore, nothing to lose if it fails.
+    committed = commit_proposal(path, number, root=repo)
+    if committed.startswith("NOT COMMITTED"):
+        return committed
+
+    # `prop/n` is just a name for the commit we already made. Force is safe: it names a
+    # ref this command owns, and the guard has already refused anything outside `prop/*`.
+    pointed = run(["git", "branch", "--force", ref, "HEAD"], repo)
+    if pointed.returncode != 0:
+        detail = (pointed.stderr or pointed.stdout).strip().splitlines()
+        return f"{committed}; NOT PUBLISHED ({detail[-1] if detail else 'branch failed'})"
+
+    pushed = push_prop_ref(ref, root=repo, runner=run)
+    return f"{committed}; {pushed}"
+
+
 def write_proposal(
     proposal: Proposal,
     *,
@@ -239,7 +366,7 @@ def write_proposal(
     out = directory / proposal.filename
     out.write_text(render(proposal, generated_at=generated_at) + "\n", encoding="utf-8")
     proposal.commit_status = (
-        commit_proposal(out, proposal.number, root=root) if commit
+        publish_proposal(out, proposal.number, root=root) if commit
         else "not committed (--no-commit)"
     )
     return out
@@ -274,4 +401,12 @@ def find_proposal(number: int, proposals_dir: Path | None = None) -> Path:
     "render",
     "write_proposal",
     "find_proposal",
+    "PROP_REF_PREFIX",
+    "UnsafePushTarget",
+    "assert_prop_ref",
+    "push_prop_ref",
+    "publish_proposal",
+    "commit_proposal",
+    "STATUS_AWAITING",
+    "STATUS_IMPLEMENTED",
 ]