Multi-milestone work and documents on the new loop (M2.4)

- Every milestone branch is cut from the latest default branch: the next
  milestone builds on the merged previous one, and a redo after a closed PR
  drops the rejected work. Uncommitted work comes along.
- While the item's PR is open, start_milestone is refused, with the PR's
  state read from Gitea at that moment, not the last watch round.
- finish_milestone(final): earlier PRs say "Part of #N", the last "Closes
  #N"; unsaid, it's final, as before.
- close_item with the item's PR still open waits for the merge (L2: a
  closed item leaves nothing open).
- The driver's instructions cover ARCHITECTURE.md and build plans under
  docs/plans/ (D2); the harness never parses a plan.
- Parking stashes uncommitted work and pushes it to <item>-parked, put back
  on resume, instead of committing it onto the item's branch. The walk
  found a half-done review fix parked onto a PR that had merged meanwhile,
  where it was lost. Architecture D4's parking line says so.

Tests: unit tests for fresh milestone branches, the merge wait (including
a merge not yet watched), redo after a closed PR, Part of/Closes, close
waiting on an open PR, and the parked-fix regression; the walk's new
milestones_start_fresh property (fork-from-current fails 9 seeds); an e2e
two-milestone feature.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE

nisse-dev/src/nisse_dev/git.py

@@ -210,8 +210,10 @@ class GitRepo:
         self._git("config", "user.name", name)
         self._git("config", "user.email", email)
 
-    def checkout_new_branch(self, name: str) -> None:
-        self._git("checkout", "-q", "-b", name)
+    def checkout_new_branch(self, name: str, start: str | None = None) -> None:
+        """A new branch, at ``start`` if given. Uncommitted changes come
+        along; git refuses if one would conflict with ``start``."""
+        self._git("checkout", "-q", "-b", name, *([start] if start else []))
 
     def checkout_existing_branch(self, name: str) -> None:
         self._git("checkout", "-q", name)
@@ -300,6 +302,46 @@ class GitRepo:
             "push", "-q", remote_url, f"HEAD:refs/heads/{branch}", credential=credential
         )
 
+    def push_commit(
+        self,
+        remote_url: str,
+        sha: str,
+        branch: str,
+        *,
+        credential: PushCredential | None = None,
+    ) -> None:
+        """Point the remote's ``branch`` at ``sha``, whatever it was."""
+        self._git(
+            "push",
+            "-q",
+            "-f",
+            remote_url,
+            f"{sha}:refs/heads/{branch}",
+            credential=credential,
+        )
+
+    def stash_all(self, message: str) -> str | None:
+        """Take every uncommitted change, untracked files included and build
+        artifacts not, off the work tree as a stash commit; returns its sha,
+        or None when there was nothing. The commit is kept by pushing it."""
+        if not self.has_changes():
+            return None
+        self._git("stash", "push", "-q", "-u", "-m", message)
+        sha = self._git("rev-parse", "stash@{0}")
+        self._git("stash", "drop", "-q")
+        return sha
+
+    def apply_stash(self, sha: str) -> None:
+        """Put a stash commit's changes back on the work tree, uncommitted."""
+        self._git("stash", "apply", "-q", sha)
+
+    def has_commit(self, sha: str) -> bool:
+        try:
+            self._git("cat-file", "-e", f"{sha}^{{commit}}")
+        except GitError:
+            return False
+        return True
+
     def discard_branch(self, branch: str, *, back_to: str) -> None:
         """Throw away a branch and everything on it.
 

nisse-dev/src/nisse_dev/loop/agent.py

@@ -25,6 +25,7 @@ from dataclasses import replace
 from nisse_dev.cancel import Cancel
 from nisse_dev.canonical import Fact, PlanSubmitted, UserMessage
 from nisse_dev.gates import command_form
+from nisse_dev.git import GitError
 from nisse_dev.gitea import GiteaClient, GiteaError, Issue
 from nisse_dev.loop.driver import Driver, RunOutcome
 from nisse_dev.loop.items import (
@@ -81,16 +82,28 @@ the project, and your first step. Wait for their go-ahead. If the request is
 unclear, ask_human instead of guessing: a restatement asks no questions, so
 the operator can agree to it as it stands.
 
-After the go-ahead, work in a milestone: call start_milestone before
-changing anything (never create or switch branches yourself), make the
-change, run the project's tests, then call finish_milestone with a summary.
-The harness commits, pushes and opens a PR, and you wait for review. Review
-comments arrive as messages: make the changes within the same milestone (don't
-start another), run the tests, and call finish_milestone again to push the fix. When the PR has merged, call
-close_item with what was done, what was verified, and what is left open. If
-the PR is closed without merging, ask the operator what next. When a review
-needs no change from you, or you are only waiting for the operator to merge
-(you can't merge it yourself), call wait_for_review.
+The project's documents are in its repository: read ARCHITECTURE.md and any
+build plans under docs/plans/ before you restate.
+
+After the go-ahead, work in milestones, one PR each. Call start_milestone
+before changing anything (never create or switch branches yourself; each
+milestone starts from the latest default branch), make the change, run the
+project's tests, then call finish_milestone with a summary, saying whether it
+is the final milestone. The harness commits, pushes and opens a PR, and you
+wait for review. Review comments arrive as messages: make the changes within
+the same milestone (don't start another), run the tests, and call
+finish_milestone again to push the fix. When a review needs no change from
+you, or you are only waiting for the operator to merge (you can't merge it
+yourself), call wait_for_review. The next milestone starts only after the
+previous PR has merged. When the last PR has merged, call close_item with
+what was done, what was verified, and what is left open. If a PR is closed
+without merging, ask the operator what next.
+
+Work longer than about one PR gets a build plan: a Markdown file under
+docs/plans/ giving, per milestone, the goal, the interfaces and behaviour, the
+edge cases, and the tests that are its acceptance criteria. Commit it with the
+first milestone, and mark each milestone done in it in that milestone's PR.
+Keep ARCHITECTURE.md current: a design change updates it in the same PR.
 
 {labels}Never label the issue yourself; you may suggest a label to the operator.
 
@@ -808,6 +821,20 @@ class LoopAgent:
                 loop,
                 str(args.get("summary", "")).strip(),
                 str(args.get("title", "")).strip(),
+                # Unsaid, it's final: the PR closes the issue, as before.
+                str(args.get("final", "yes")).strip().lower() != "no",
+            )
+        elif outcome.kind == "wait" and outcome.tool == "close_item" and loop.pr:
+            # Closing leaves nothing open (L2): an open PR is waited out first.
+            self._fact(
+                item,
+                f"close_item waits: PR #{loop.pr} is still open; the item closes "
+                "after it merges",
+            )
+            self._wait(
+                item,
+                replace(loop, wait=MERGE, note=f"PR #{loop.pr}"),
+                f"I'll close this once PR #{loop.pr} merges: {loop.pr_url}",
             )
         elif outcome.kind == "wait" and outcome.tool == "close_item":
             summary = str(args.get("summary", "")).strip()
@@ -828,6 +855,23 @@ class LoopAgent:
         item = self._store.work_item(item_id)
         assert item is not None
         loop = loop_of(item)
+        if loop.pr:
+            # D4: the next milestone waits for the merge, read from Gitea now,
+            # not from the last watch round.
+            try:
+                pr = self._gitea.pull_request(self._repo, loop.pr)
+            except GiteaError as exc:
+                return f"couldn't check PR #{loop.pr}: {exc}; try again", False
+            if pr.state == "open":
+                wait = (
+                    f"milestone {loop.milestone}'s PR #{loop.pr} hasn't merged yet; "
+                    "the next milestone starts after it merges. If you're only "
+                    "waiting for that, call wait_for_review."
+                )
+                return wait, False
+            how = "merged" if pr.merged else "was closed without merging"
+            self._fact(item, f"PR #{loop.pr} {how}")
+            loop = replace(loop, pr=0, pr_url="", branch="")
         if loop.branch:
             busy = f"milestone {loop.milestone} is still open; finish it first"
             return busy, False
@@ -835,7 +879,10 @@ class LoopAgent:
             return "milestones aren't available here", False
         number = loop.milestone + 1
         branch = f"{item.id}-milestone-{number}-{uuid.uuid4().hex[:4]}"
-        self._milestones.start(branch)
+        try:
+            self._milestones.start(branch)
+        except GitError as exc:
+            return f"couldn't start the milestone: {exc}", False
         name = str(args.get("name", "")).strip()
         self._fact(item, f"milestone {number} started ({name}) on branch {branch}")
         save(
@@ -869,7 +916,12 @@ class LoopAgent:
         )
 
     def _finish(
-        self, item: WorkItem, loop: Loop, summary: str, title: str = ""
+        self,
+        item: WorkItem,
+        loop: Loop,
+        summary: str,
+        title: str = "",
+        final: bool = True,
     ) -> None:
         """finish_milestone: publish the milestone, or push a review fix."""
         if self._milestones is None:
@@ -918,7 +970,12 @@ class LoopAgent:
             # D4: finishing without a start cuts the branch now.
             number = loop.milestone + 1
             branch = f"{item.id}-milestone-{number}-{uuid.uuid4().hex[:4]}"
-            self._milestones.start(branch)
+            try:
+                self._milestones.start(branch)
+            except GitError as exc:
+                note = f"I couldn't start milestone {number}: {exc}"
+                self._wait(item, replace(loop, wait=ANSWER, note=note), note)
+                return
             loop = replace(loop, milestone=number, branch=branch)
         published = self._milestones.publish(
             item_id=item.id,
@@ -926,6 +983,7 @@ class LoopAgent:
             title=title or f"{item.title} (milestone {loop.milestone})",
             summary=summary,
             issue=loop.issue,
+            final=final,
         )
         left = (
             f" (left out {len(published.left_out)} build artifact(s))"
@@ -984,6 +1042,11 @@ class LoopAgent:
                 )
         save(self._store, item.id, CLOSED, loop, cause=outcome)
         self._store.close_work_item(item.id, outcome=outcome, cause=note)
+        if self._milestones is not None:
+            try:
+                self._milestones.drop_parked(f"{item.id}-")
+            except GitError:
+                pass  # a leftover side branch; nothing depends on it
         if loop.issue:
             try:
                 self._gitea.post_comment(self._repo, loop.issue, f"{outcome}: {note}")

nisse-dev/src/nisse_dev/loop/milestones.py

@@ -91,38 +91,73 @@ class Milestones:
     # -- parking and resuming (L1, D4) -----------------------------------------
 
     def park(self, *, keep: tuple[str, ...]) -> str | None:
-        """Before another item uses the workspace: commit whatever the item
-        on it left, and push its branch, so nothing of a parked item lives
-        only on the build VM. Only a harness branch of an open item (one
-        starting with a ``keep`` prefix) is parked; returns its name."""
+        """Before another item uses the workspace: nothing of the item on it
+        may live only on the build VM. Its uncommitted work is stashed and
+        pushed to ``<item>-parked``, never committed onto its branch, which
+        may be a PR's that has merged meanwhile; resuming puts it back
+        uncommitted, for wherever the work goes next. Commits the harness
+        made but couldn't push are pushed. Only an open item's harness branch
+        (one starting with a ``keep`` prefix, ``<item>-``) is parked; returns
+        its name."""
         git = self._git
         branch = git.current_branch()
-        if not keep or not branch.startswith(keep):
+        prefix = next((k for k in keep if branch.startswith(k)), None)
+        if prefix is None:
             return None
-        git.commit_all("WIP: parked while another work item runs")
-        if git.remote_sha(self._remote, branch, credential=self._credential) != (
-            git.head_sha()
+        sha = git.stash_all(f"parked from {branch}")
+        if sha is not None:
+            git.push_commit(
+                self._remote, sha, f"{prefix}parked", credential=self._credential
+            )
+        remote = git.remote_sha(self._remote, branch, credential=self._credential)
+        if remote != git.head_sha() and (
+            remote is not None or git.commits_since(f"origin/{self._base}") > 0
         ):
             git.push(self._remote, branch, credential=self._credential)
         return branch
 
-    def resume(self, branch: str, *, keep: tuple[str, ...], mine: str = "") -> None:
-        """Back to a parked item: park whatever is on the workspace now, then
-        check out the item's branch. The local branch wins when it exists (it
-        can only be ahead: parking pushed it); otherwise it comes from origin.
-        A workspace already on one of the item's own branches (``mine``, a
-        prefix) is where it left off, and stays."""
+    def resume(self, branch: str, *, keep: tuple[str, ...], mine: str) -> None:
+        """Back to a parked item: park whatever is on the workspace now,
+        check out the item's branch, and put its parked work back. The local
+        branch wins when it exists (it can only be ahead: parking pushed it);
+        otherwise it comes from origin. A workspace already on one of the
+        item's own branches (``mine``, its ``<item>-`` prefix) stays."""
         git = self._git
         current = git.current_branch()
-        if current == branch or (mine and current.startswith(mine)):
+        if current != branch and not current.startswith(mine):
+            self.park(keep=keep)
+            if branch in git.local_branches():
+                git.checkout_existing_branch(branch)
+                git.reset_to(branch, branch)
+            else:
+                git.fetch(credential=self._credential)
+                pushed = git.remote_sha(
+                    self._remote, branch, credential=self._credential
+                )
+                # A branch never pushed held nothing beyond the default branch.
+                start = f"origin/{branch}" if pushed else f"origin/{self._base}"
+                git.reset_to(branch, start)
+        # Even when it stayed: a crash may have come between parking and the
+        # next item's start.
+        self.unpark(mine)
+
+    def unpark(self, mine: str) -> None:
+        parked = f"{mine}parked"
+        git = self._git
+        sha = git.remote_sha(self._remote, parked, credential=self._credential)
+        if sha is None:
             return
-        self.park(keep=keep)
-        if branch in git.local_branches():
-            git.checkout_existing_branch(branch)
-            git.reset_to(branch, branch)
-        else:
+        if not git.has_commit(sha):
             git.fetch(credential=self._credential)
-            git.reset_to(branch, f"origin/{branch}")
+        git.apply_stash(sha)
+        git.delete_remote_branch(self._remote, parked, credential=self._credential)
+
+    def drop_parked(self, mine: str) -> None:
+        """A closed item's parked work, if any, goes: closing set it aside."""
+        parked = f"{mine}parked"
+        git = self._git
+        if git.remote_sha(self._remote, parked, credential=self._credential):
+            git.delete_remote_branch(self._remote, parked, credential=self._credential)
 
     def _ahead_of_origin(self) -> bool:
         current = self._git.current_branch()
@@ -136,8 +171,12 @@ class Milestones:
     # -- milestones (D4) -------------------------------------------------------
 
     def start(self, branch: str) -> str:
-        """Cut the milestone's branch where the item is; returns its start."""
-        self._git.checkout_new_branch(branch)
+        """Cut the milestone's branch from the latest default branch (D4):
+        a milestone after a merge builds on it, and one redone after a closed
+        PR doesn't carry the rejected work. Uncommitted work comes along.
+        Returns where it starts."""
+        self._git.fetch(credential=self._credential)
+        self._git.checkout_new_branch(branch, f"origin/{self._base}")
         return self._git.head_sha()
 
     def publish(
@@ -148,6 +187,7 @@ class Milestones:
         title: str,
         summary: str,
         issue: int,
+        final: bool = True,
     ) -> Published:
         git = self._git
         try:
@@ -184,7 +224,7 @@ class Milestones:
                 head=branch,
                 base=self._base,
                 title=title,
-                body=_body(summary, issue, left_out),
+                body=_body(summary, issue, left_out, final),
             )
             self._store.commit_intent(intent, cause=f"opened PR #{pr.number}")
             return Published("opened", f"opened PR #{pr.number}", pr, left_out)
@@ -227,10 +267,13 @@ class Milestones:
             return Fixed("failed", f"pushing the fix failed: {exc}")
 
 
-def _body(summary: str, issue: int, left_out: tuple[str, ...]) -> str:
+def _body(
+    summary: str, issue: int, left_out: tuple[str, ...], final: bool = True
+) -> str:
     parts = [summary]
     if issue:
-        parts += ["", f"Closes #{issue}"]
+        # D1: the last PR closes the issue on merge; earlier ones are part of it.
+        parts += ["", f"Closes #{issue}" if final else f"Part of #{issue}"]
     if left_out:
         parts += ["", "## Left out", "", "Build artifacts, never part of the work:"]
         parts += [f"- `{path}`" for path in left_out]

nisse-dev/src/nisse_dev/loop/tools.py

@@ -56,6 +56,7 @@ FINISH_MILESTONE = _tool(
     "reviewed or merged.",
     summary="What the milestone did and how it was verified, for the PR.",
     title="A short title for the PR (ignored when pushing a review fix).",
+    final="yes if this milestone completes the issue, no if more milestones follow it.",
 )
 WAIT_FOR_REVIEW = _tool(
     "wait_for_review",