From 88c7d2e8c9397c6627c0bcb5c6e81055ddaf2564 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Wed, 7 Oct 2026 15:26:17 +0000 Subject: [PATCH] A design milestone holds documents only, enforced Found by the first local e2e run: qwen3.8-27b put a new project's code in the same PR as its ARCHITECTURE.md, with no build plan. D1 says a `design` item agrees its design before any code, and that rested on wording only. - Until a PR of a `design` item has merged, finish_milestone is sent back if the milestone changes anything but ARCHITECTURE.md and docs/, naming the paths to take out. The merge is recorded (design_merged) and code may follow. - The label's instruction and the new project's brief say so plainly: documents first, in a PR of their own; code after it has merged. Tests: a design milestone with code is sent back and only the documents are published, then code after the merge; the walk's model follows the rule. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- nisse-dev/src/nisse_dev/git.py | 7 ++++ nisse-dev/src/nisse_dev/loop/agent.py | 45 +++++++++++++++++++--- nisse-dev/src/nisse_dev/loop/items.py | 1 + nisse-dev/src/nisse_dev/loop/milestones.py | 4 ++ nisse-dev/tests/test_loop_multi.py | 42 ++++++++++++++++++++ nisse-dev/tests/test_loop_properties.py | 9 ++++- 6 files changed, 101 insertions(+), 7 deletions(-) diff --git a/nisse-dev/src/nisse_dev/git.py b/nisse-dev/src/nisse_dev/git.py index 8808435..ee1225f 100644 --- a/nisse-dev/src/nisse_dev/git.py +++ b/nisse-dev/src/nisse_dev/git.py @@ -255,6 +255,13 @@ class GitRepo: self._git("commit", "-q", "-m", message) return self.head_sha() + def changed_paths(self, since: str) -> list[str]: + """Every path the work changes relative to ``since``, committed or + not, new files included; build artifacts aren't work (#80).""" + tracked = self._git("diff", "--name-only", since).splitlines() + new = self._git("ls-files", "--others", "--exclude-standard").splitlines() + return sorted({p for p in [*tracked, *new] if p}) + def left_out(self) -> list[str]: """Changes not in the last commit: build artifacts the harness left out, to be reported rather than silently dropped (D4).""" diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index 39a422d..dd314cf 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/nisse-dev/src/nisse_dev/loop/agent.py @@ -150,9 +150,11 @@ LABELS = { "discuss": "This issue is labelled discuss: talk it through with the " "operator (ask_human) before restating, and restate only once you agree " "on what is wanted.", - "design": "This issue is labelled design: agree the design with the " - "operator before any code. Your restatement puts the design to them; " - "code waits for their go-ahead on it.", + "design": "This issue is labelled design: the design comes before any " + "code. Your first milestone is documents only: ARCHITECTURE.md and a " + "build plan under docs/plans/, in a PR of their own (if you restate, put " + "the design to the operator). Code starts in a later milestone, after that " + "PR has merged; the harness refuses a design milestone with anything else.", } @@ -519,6 +521,9 @@ class LoopAgent: if pr.merged: self._fact(item, f"PR #{loop.pr} merged") loop, position = replace(loop, pr=0, pr_url="", branch=""), READY + if "design" in loop.labels and not loop.design_merged: + self._fact(item, "the design has merged: code may follow") + loop = replace(loop, design_merged=True) elif pr.state != "open": self._fact(item, f"PR #{loop.pr} was closed without merging") loop, position = replace(loop, pr=0, pr_url="", branch=""), READY @@ -1047,7 +1052,8 @@ class LoopAgent: project.repo, title, f"{brief}\n\nFrom a conversation with the operator, agreed there " - f"as rev-{loop.rev}. First: ARCHITECTURE.md and the build plan." + f"as rev-{loop.rev}. First: ARCHITECTURE.md and the build plan, " + "in a PR of documents only; code after it has merged." f"\n\n", ) self._gitea.assign_issue(project.repo, number, self._account) @@ -1211,6 +1217,32 @@ class LoopAgent: ) self._last_project = here + def _design_problem(self, item_id: str) -> str | None: + """D1, D2: a `design` item agrees its design before any code. Until + a PR of it has merged, a milestone holds documents only.""" + item = self._store.work_item(item_id) + if item is None: + return None + loop = loop_of(item) + milestones = self._at(item).milestones + if "design" not in loop.labels or loop.design_merged or milestones is None: + return None + try: + changed = milestones.changed() + except GitError: + return None # publishing reports what's wrong with the workspace + code = [ + p for p in changed if p != "ARCHITECTURE.md" and not p.startswith("docs/") + ] + if not code: + return None + return ( + "not published: this milestone is the design, so it holds documents " + "only (ARCHITECTURE.md, docs/); code starts in a later milestone, " + "after this one's PR has merged. Take these out of it (they can " + "wait in the plan): " + ", ".join(code) + ) + def _converse_run(self, item: WorkItem) -> None: """A conversation's turn (L1, L5): it talks, and may propose work on a project. No workspace, nothing to run.""" @@ -1329,7 +1361,10 @@ class LoopAgent: tools=tools, workspace=workspace, handlers={"start_milestone": lambda args: self._start(item.id, args)}, - checks={"restate": _restatement_problem}, + checks={ + "restate": _restatement_problem, + "finish_milestone": lambda args: self._design_problem(item.id), + }, ) finally: self._running = False diff --git a/nisse-dev/src/nisse_dev/loop/items.py b/nisse-dev/src/nisse_dev/loop/items.py index 6de75a9..e366ff3 100644 --- a/nisse-dev/src/nisse_dev/loop/items.py +++ b/nisse-dev/src/nisse_dev/loop/items.py @@ -55,6 +55,7 @@ class Loop: proposal_title: str = "" proposal_kind: str = "" # "work" on a project, or a new "project" new_project: str = "" # a new project being provisioned (D1) + design_merged: bool = False # a `design` item's design PR has merged (D1, D2) # Where the operator last spoke (D5: answer where asked): "" for the # Matrix thread, or the PR it came from, with the inline location and a # little of what they said, for quoting. diff --git a/nisse-dev/src/nisse_dev/loop/milestones.py b/nisse-dev/src/nisse_dev/loop/milestones.py index c88629e..125731e 100644 --- a/nisse-dev/src/nisse_dev/loop/milestones.py +++ b/nisse-dev/src/nisse_dev/loop/milestones.py @@ -161,6 +161,10 @@ class Milestones: if git.remote_sha(self._remote, parked, credential=self._credential): git.delete_remote_branch(self._remote, parked, credential=self._credential) + def changed(self) -> list[str]: + """What the current milestone changes, from the default branch.""" + return self._git.changed_paths(f"origin/{self._base}") + # -- a new project's checks (D1, D4) ----------------------------------------- def verify_access(self, account: str) -> list[str]: diff --git a/nisse-dev/tests/test_loop_multi.py b/nisse-dev/tests/test_loop_multi.py index b412d4e..69cf7f8 100644 --- a/nisse-dev/tests/test_loop_multi.py +++ b/nisse-dev/tests/test_loop_multi.py @@ -19,6 +19,7 @@ from test_loop_milestones import ( from test_loop_milestones import world as world # noqa: PLC0414 -- the fixture from nisse_dev.canonical import ToolCall, ToolResult +from nisse_dev.gitea import Issue from nisse_dev.loop.items import MERGE, WAITING, loop_of @@ -174,3 +175,44 @@ def test_an_unsaid_final_closes_the_issue(world: dict[str, Any]) -> None: say(transport, world, "go") [pr] = forge.prs assert "Closes #14" in forge.prs[pr]["body"] + + +def test_a_design_milestone_holds_documents_only(world: dict[str, Any]) -> None: + """D1, D2: a `design` item agrees its design before any code; the + harness refuses a design milestone with code in it.""" + forge: Forge = world["forge"] + forge.issues[0] = Issue(14, "Add count_words", "count words", "don", ("design",)) + model = ScriptedModel( + RESTATE, + [ + call("start_milestone", name="design"), + call("write_file", path="ARCHITECTURE.md", content="# arch\n"), + call("write_file", path="docs/plans/words.md", content="# plan\n"), + call("write_file", path="words.py", content="W = 1\n"), # too soon + call("finish_milestone", summary="design", final="no"), + ], + [ + call("run_command", command="rm words.py"), + call("finish_milestone", summary="design", final="no"), + ], + milestone("words.py", final="yes"), # after the design merged + ) + agent, transport = loop_agent(world, model) + agent.intake() + say(transport, world, "go") + [design] = forge.prs + assert sorted(forge.files(design)) == ["ARCHITECTURE.md", "docs/plans/words.md"] + refused = [ + e.event.result + for e in world["store"].transcript(item(world).id, "driver").read() + if isinstance(e.event, ToolResult) + and "this milestone is the design" in e.event.result + ] + assert refused and "words.py" in refused[0] + + forge.merge(design) + agent.watch() + agent._schedule() + code = max(forge.prs) + assert code != design and forge.files(code) == ["words.py"] + assert loop_of(item(world)).design_merged diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index d246269..9b92437 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/nisse-dev/tests/test_loop_properties.py @@ -63,6 +63,11 @@ class PolicyModel: self.n += 1 offered = {t["function"]["name"] for t in tools} facts = [getattr(e.event, "text", "") for e in events] + # A `design` item writes documents until its design has merged. + design = "labelled design" in (system or "") and not any( + "the design has merged" in t for t in facts + ) + path = f"docs/d{self.n}.md" if design else f"f{self.n}.py" roll = self.rng.random() if roll < 0.08: return [call("ask_human", question=f"question {self.n}?")] @@ -98,7 +103,7 @@ class PolicyModel: # Half done, then a question: work a switch must park. return [ call("start_milestone", name=f"m{self.n}"), - call("write_file", path=f"f{self.n}.py", content=f"x = {self.n}\n"), + call("write_file", path=path, content=f"x = {self.n}\n"), call("ask_human", question=f"halfway {self.n}?"), ] merged = any("merged" in t for t in facts[-6:]) @@ -106,7 +111,7 @@ class PolicyModel: return [call("close_item", summary=f"done {self.n}")] return [ call("start_milestone", name=f"m{self.n}"), - call("write_file", path=f"f{self.n}.py", content=f"x = {self.n}\n"), + call("write_file", path=path, content=f"x = {self.n}\n"), call( "finish_milestone", summary=f"work {self.n}",