From 659915d309e114e923fec1caa362165d4fc27e53 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Thu, 8 Oct 2026 00:02:50 +0000 Subject: [PATCH] Fixes from the first live session of the new loop Found live on 2026-10-07 (bring-up on nisse-testing-operator/nisse-bringup, Opus as driver): - The driver's prose nudge went into the system prompt, so the retry ended on the model's own reply. Opus 5.5 refuses that ("does not support assistant message prefill. The conversation must end with a user message"). The nudge is now a harness fact in the conversation. The delegation harness's prose path did the same and is fixed likewise. - A helper's first request ended with its own delegation_started event, rendered as an assistant message: every delegation would have failed on Opus. The helper now sees it as the harness handing over its task. - model_contract checks that every request ends on the user's side, so scripted tests catch this from now on (each fix is mutation-checked). - `status` (and `details`) in an item's thread were taken as replies: they woke the driver and cancelled a bug's countdown. They now answer at once and change nothing. - The driver had no way to answer a remark without changing the item, so it answered in prose. Every run woken by a message (or a review comment) now offers `reply`, which answers where asked and returns to the very wait that was interrupted. Also: scripts/bringup-repo.sh makes the bring-up's test repository (make_repo/delete_repo, split out of fresh_repo), and made repositories get the agent's labels (bug, discuss, design). Tests: test_loop_live_fixes.py (one per finding); the walk sends `status` into threads (status_changes_nothing) and its scripted driver sometimes answers remarks with reply. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- nisse-dev/src/nisse_dev/delegation.py | 10 ++ nisse-dev/src/nisse_dev/loop/agent.py | 82 +++++++++---- nisse-dev/src/nisse_dev/loop/driver.py | 7 +- nisse-dev/src/nisse_dev/views.py | 8 ++ nisse-dev/tests/model_contract.py | 7 ++ nisse-dev/tests/test_loop_live_fixes.py | 154 ++++++++++++++++++++++++ nisse-dev/tests/test_loop_properties.py | 22 +++- scripts/bringup-repo.sh | 42 +++++++ tests/integration/e2e_support.py | 51 ++++++-- 9 files changed, 341 insertions(+), 42 deletions(-) create mode 100644 nisse-dev/tests/test_loop_live_fixes.py create mode 100755 scripts/bringup-repo.sh diff --git a/nisse-dev/src/nisse_dev/delegation.py b/nisse-dev/src/nisse_dev/delegation.py index 4c908e1..9825493 100644 --- a/nisse-dev/src/nisse_dev/delegation.py +++ b/nisse-dev/src/nisse_dev/delegation.py @@ -33,6 +33,7 @@ from nisse_dev.canonical import ( AssistantText, DelegationCompleted, DelegationStarted, + Fact, PlanSubmitted, StoredEvent, ToolCall, @@ -414,6 +415,15 @@ class Delegation: # and its tree state is unknown. Treat it as a failure rather # than inventing a handback on its behalf. if any(isinstance(e, AssistantText) for e in produced): + # Said something but did nothing: told so, as the + # harness, so the next request ends on the user's side + # (models refuse assistant prefill). + transcript.as_actor("orchestrator").append( + Fact( + text="Your last reply was prose. Use your tools to " + "do the work, then call hand_back." + ) + ) continue return DelegationResult( "failed", diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index 18ad3aa..9e56456 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/nisse-dev/src/nisse_dev/loop/agent.py @@ -116,6 +116,10 @@ asking is better than guessing. Before each group of tool calls, say in a sentence what you're doing and why: the operator sees it as your progress. +When the operator says something that needs an answer but no change (a +remark, a status question), answer with reply: the item goes back to what it +was waiting for. Every reply of yours is a tool call, never bare prose. + The project's documents are in its repository: read ARCHITECTURE.md and any build plans under docs/plans/ before you restate. @@ -587,6 +591,8 @@ class LoopAgent: loop, position = replace(loop, pr=0, pr_url="", branch=""), READY elif new and item.position == WAITING: position = READY + # A comment can be answered with a reply, back to this wait. + loop = replace(loop, back_to=loop.wait, back_note=loop.note) if position != item.position or loop != loop_of(item): if position == READY: loop = replace(loop, wait="", note="") @@ -824,6 +830,19 @@ class LoopAgent: elif not self._store.active_item(): self._transport.post("Nothing is running.", thread=msg.thread) return + if command in ("status", "details"): + # About the item, not a reply to it: nothing changes, a + # countdown runs on (L5). Recorded at the agent's level. + self._store.transcript(None, "operator").append( + UserMessage(text=text, source=f"transport:{msg.sender}") + ) + said = ( + self._details(item) + if command == "details" + else self._status_line(item, [i.id for i in self._queue()]) + ) + self._transport.post(said, thread=loop_of(item).thread) + return self._store.transcript(item.id, "operator").append( UserMessage(text=text, source=f"transport:{msg.sender}") ) @@ -844,8 +863,6 @@ class LoopAgent: self._close(item, loop, "abandoned", f"abandoned by {msg.sender}") elif command == "done" and loop.new_project: self._provisioned(item, loop) - elif command == "details": - self._transport.post(self._details(item), thread=loop.thread) elif ( item.position == WAITING and item.scope != GENERAL @@ -903,12 +920,13 @@ class LoopAgent: ) self._go(item, loop, f"{msg.sender} (in plain language)") elif item.position == WAITING: - # An answer, or changes to a restatement: the driver reads it. + # An answer, changes to a restatement, or a remark: the driver + # reads it, and may reply and go back to this wait (L5). save( self._store, item.id, READY, - replace(loop, wait="", note=""), + replace(loop, wait="", note="", back_to=loop.wait, back_note=loop.note), cause="the operator answered", ) self._schedule() @@ -1272,28 +1290,28 @@ class LoopAgent: f"{self._repos()}." ) queue = [i.id for i in self._queue()] - lines = [] - for item in items: - loop = loop_of(item) - what = item.position - if item.position == WAITING and loop.wait == CHAT: - what = "talking; your turn" - elif item.position == WAITING: - what = f"waiting for {loop.wait} ({loop.note or f'rev-{loop.rev}'})" - if loop.grace_until: - left = max(0, int(loop.grace_until - self._clock()) // 60) - what += f", going ahead in {left} min unless you reply" - elif item.position == BLOCKED: - what = f"blocked by {', '.join(loop.blocked_by)}" - elif item.position == READY: - what = f"ready, {queue.index(item.id) + 1} of {len(queue)} in line" - name = ( - f'conversation "{item.title}"' - if item.scope == GENERAL - else f"{self._label(item)} {item.title}" - ) - lines.append(f"{name}: {what}") - return "\n".join(lines) + return "\n".join(self._status_line(item, queue) for item in items) + + def _status_line(self, item: WorkItem, queue: list[str]) -> str: + loop = loop_of(item) + what = item.position + if item.position == WAITING and loop.wait == CHAT: + what = "talking; your turn" + elif item.position == WAITING: + what = f"waiting for {loop.wait} ({loop.note or f'rev-{loop.rev}'})" + if loop.grace_until: + left = max(0, int(loop.grace_until - self._clock()) // 60) + what += f", going ahead in {left} min unless you reply" + elif item.position == BLOCKED: + what = f"blocked by {', '.join(loop.blocked_by)}" + elif item.position == READY: + what = f"ready, {queue.index(item.id) + 1} of {len(queue)} in line" + name = ( + f'conversation "{item.title}"' + if item.scope == GENERAL + else f"{self._label(item)} {item.title}" + ) + return f"{name}: {what}" # -- running (L1, L2) ------------------------------------------------------ @@ -1663,12 +1681,15 @@ class LoopAgent: CLOSE_ITEM, *([WAIT_FOR_REVIEW] if loop.pr else []), *([DELEGATE, ASK_EXPERT] if self._helpers else []), + *([REPLY] if loop.back_to else []), *EXECUTOR_TOOLS, ] ), ] if loop.question: tools = [REPLY, *_LOOK] + elif loop.back_to and not loop.started: + tools = [*tools, REPLY] # before the go-ahead, a remark answered roles = self._roles_of(item) workspace = project.workspace system = SYSTEM.format( @@ -1771,6 +1792,13 @@ class LoopAgent: replace(loop, rev=rev, wait=GO_AHEAD, note="", grace_until=grace), None, ) + elif outcome.kind == "wait" and outcome.tool == "reply" and loop.back_to: + # A project item's reply: answered where asked (the thread or the + # PR), then back to the very wait the message interrupted (L5). + self._answer_where_asked(item, loop, str(args.get("text", "")).strip()) + self._wait( + item, replace(loop, wait=loop.back_to, note=loop.back_note), None + ) elif outcome.kind == "wait" and outcome.tool == "reply": text = str(args.get("text", "")).strip() self._transport.post(text, thread=loop.thread) @@ -2068,6 +2096,8 @@ class LoopAgent: def _wait(self, item: WorkItem, loop: Loop, say: str | None) -> None: if say: self._transport.post(say, thread=loop.thread) + # Waiting again: whatever woke the item is settled. + loop = replace(loop, question=False, back_to="", back_note="") save(self._store, item.id, WAITING, loop, cause=f"waiting for {loop.wait}") def _close(self, item: WorkItem, loop: Loop, outcome: str, note: str) -> None: diff --git a/nisse-dev/src/nisse_dev/loop/driver.py b/nisse-dev/src/nisse_dev/loop/driver.py index 6d3d3f5..de39edc 100644 --- a/nisse-dev/src/nisse_dev/loop/driver.py +++ b/nisse-dev/src/nisse_dev/loop/driver.py @@ -12,7 +12,7 @@ from dataclasses import dataclass from typing import Any, Literal from nisse_dev.cancel import Cancel, skip_calls -from nisse_dev.canonical import AssistantText, ToolCall, ToolResult +from nisse_dev.canonical import AssistantText, Fact, ToolCall, ToolResult from nisse_dev.llm import LLMClient, LLMError from nisse_dev.loop.tools import WAITING, names from nisse_dev.state import Transcript @@ -100,7 +100,10 @@ class Driver: if nudged: return RunOutcome("failed", note="kept replying in prose") nudged = True - system = f"{system}\n\n{_PROSE_NUDGE}" + # In the conversation, as the harness: a request must end on + # the user's side (models refuse assistant prefill), and the + # nudge is then in the log as what it was. + transcript.as_actor("orchestrator").append(Fact(text=_PROSE_NUDGE)) continue for index, call in enumerate(calls): if self._cancel.requested: diff --git a/nisse-dev/src/nisse_dev/views.py b/nisse-dev/src/nisse_dev/views.py index bf13941..f677302 100644 --- a/nisse-dev/src/nisse_dev/views.py +++ b/nisse-dev/src/nisse_dev/views.py @@ -22,6 +22,7 @@ from dataclasses import replace from nisse_dev.canonical import ( AssistantText, DelegationCompleted, + DelegationStarted, Fact, GateOpened, PlanSubmitted, @@ -105,6 +106,13 @@ def project(events: list[StoredEvent], viewer: str) -> list[StoredEvent]: if viewer == "helper": out.append(_said(stored, "approved plan", event.plan)) continue + if isinstance(event, DelegationStarted): + # The harness handing a task over: to the helper given it, a + # message from the harness, never its own turn (a request must + # end on the user's side; models refuse assistant prefill). + if actor == viewer: + out.append(_said(stored, "orchestrator", f"your task: {event.brief}")) + continue if actor == viewer: out.append(stored) elif actor == "operator" and isinstance(event, UserMessage): diff --git a/nisse-dev/tests/model_contract.py b/nisse-dev/tests/model_contract.py index 55d97ec..4c87a7d 100644 --- a/nisse-dev/tests/model_contract.py +++ b/nisse-dev/tests/model_contract.py @@ -20,6 +20,13 @@ def assert_renderable(events: list[Any]) -> None: def _anthropic(messages: list[dict[str, Any]]) -> None: + # A request ends on the user's side: current models refuse assistant + # prefill ("The conversation must end with a user message"), found live + # on 2026-10-07 when the driver's prose nudge left its own reply last. + assert not messages or messages[-1]["role"] == "user", ( + f"anthropic messages.{len(messages) - 1}: the conversation ends with an " + "assistant message (prefill), which the API refuses" + ) for index, message in enumerate(messages): if message["role"] != "assistant": continue diff --git a/nisse-dev/tests/test_loop_live_fixes.py b/nisse-dev/tests/test_loop_live_fixes.py new file mode 100644 index 0000000..435552c --- /dev/null +++ b/nisse-dev/tests/test_loop_live_fixes.py @@ -0,0 +1,154 @@ +"""What the first live session of the new loop found (2026-10-07), each as a +test: a request ending on the assistant's side (refused by models without +prefill: the driver's prose nudge, a helper's first request), `status` in an +item's thread taken as a reply, and no way for the driver to answer a remark. +""" + +from pathlib import Path +from typing import Any + +from model_contract import assert_renderable +from test_loop import RESTATED, FakeGitea, Harness, ScriptedModel, call, issue +from test_loop_help import ROLES, hand_back +from test_loop_milestones import RESTATE, Forge, loop_agent, say +from test_loop_milestones import item as the_item +from test_loop_milestones import world as world # noqa: PLC0414 -- the fixture + +from nisse_dev.canonical import AssistantText, Fact +from nisse_dev.delegation import Caps +from nisse_dev.gitea import Comment +from nisse_dev.loop.help import Helpers +from nisse_dev.loop.items import GO_AHEAD, MERGE, WAITING, loop_of + + +class Clock: + def __init__(self) -> None: + self.now = 1000.0 + + def __call__(self) -> float: + return self.now + + +def test_the_prose_nudge_keeps_requests_ending_on_the_user_side( + tmp_path: Path, +) -> None: + # ScriptedModel checks every request as the API would, now including + # "the conversation must end with a user message". + h = Harness( + tmp_path, + ScriptedModel([AssistantText(text="Let me think.", model="m")], RESTATED), + FakeGitea(issue()), + ) + h.agent.intake() + assert loop_of(h.item()).wait == GO_AHEAD # the nudge worked + facts = [ + e.event.text + for e in h.store.transcript(h.item().id, "orchestrator").read() + if isinstance(e.event, Fact) + ] + assert any("Your last reply was prose" in f for f in facts) + + +class ContractHelper: + """A helper that checks its requests as the API would.""" + + def __init__(self, *turns: list[Any]) -> None: + self.turns = list(turns) + + def complete(self, events, tools, model, system=None) -> list[Any]: + assert_renderable(events) + return self.turns.pop(0) + + +def test_a_helpers_requests_end_on_the_user_side(world: dict[str, Any]) -> None: + helper = ContractHelper( + [AssistantText(text="Looking around first.", model="h")], # prose + [call("write_file", path="helped.py", content="H = 1\n")], + [hand_back()], + ) + model = ScriptedModel( + RESTATE, + [ + call("start_milestone", name="words"), + call("delegate", task="write helped.py"), + call("finish_milestone", summary="with help", final="yes"), + ], + ) + helpers = Helpers( + client=helper, # type: ignore[arg-type] + roles=ROLES, + client_for=lambda model: helper, # type: ignore[arg-type,return-value] + caps=Caps(turns=5), + ) + agent, transport = loop_agent(world, model, helpers=helpers) + agent.intake() + say(transport, world, "go") + forge: Forge = world["forge"] + [pr] = forge.prs + assert forge.files(pr) == ["helped.py"] + + +def test_status_in_a_thread_is_not_a_reply(tmp_path: Path) -> None: + clock = Clock() + model = ScriptedModel(RESTATED) # called once only: status runs nothing + h = Harness( + tmp_path, + model, + FakeGitea(issue(labels=("bug",))), + clock=clock, + bug_grace_s=600, + ) + h.agent.intake() + grace = loop_of(h.item()).grace_until + h.say("status") + assert h.posted()[-1].startswith("#14 Add count_words: waiting for go-ahead") + loop = loop_of(h.item()) + assert (h.item().position, loop.wait, loop.grace_until) == ( + WAITING, + GO_AHEAD, + grace, + ) + h.say("details") + assert loop_of(h.item()).grace_until == grace + + +def test_a_remark_gets_a_reply_and_the_restatement_stays_up(tmp_path: Path) -> None: + h = Harness( + tmp_path, + ScriptedModel( + RESTATED, + [call("reply", text="Noted: I'll keep it to text.py.")], + ), + FakeGitea(issue()), + ) + h.agent.intake() + h.say("just a thought: keep it small") + loop = loop_of(h.item()) + assert (loop.wait, loop.rev) == (GO_AHEAD, 1) + assert h.posted()[-1] == "Noted: I'll keep it to text.py." + + +def test_a_review_comment_can_be_answered_without_a_fix( + world: dict[str, Any], +) -> None: + forge: Forge = world["forge"] + model = ScriptedModel( + RESTATE, + [ + call("start_milestone", name="words"), + call("write_file", path="w.py", content="W = 1\n"), + call("finish_milestone", summary="words", final="yes"), + ], + [call("reply", text="It's a constant on purpose, for the test.")], + ) + agent, transport = loop_agent(world, model) + agent.intake() + say(transport, world, "go") + [pr] = forge.prs + forge.comments[pr] = [Comment(7, "don", "why a constant", "t")] + agent.watch() + agent._schedule() + loop = loop_of(the_item(world)) + assert (loop.wait, loop.pr) == (MERGE, pr) + assert forge.posted[-1][0] == pr # answered on the PR, where asked + assert "It's a constant on purpose" in forge.posted[-1][1] diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index 1debd61..4977431 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/nisse-dev/tests/test_loop_properties.py @@ -83,8 +83,15 @@ class PolicyModel: return [call("ask_human", question=f"question {self.n}?")] if roll < 0.12: return [ToolCall(call_id=f"p{self.n}", tool="nonsense", arguments={})] - if "reply" in offered and "propose_work" not in offered: + if "reply" in offered and not offered & { + "propose_work", + "restate", + "ask_human", + "start_milestone", + }: return [call("reply", text=f"because {self.n}")] # a question run + if "reply" in offered and "propose_work" not in offered and roll < 0.2: + return [call("reply", text=f"noted {self.n}")] # a remark answered if "reply" in offered: # a conversation: talk, or propose work if self.rng.random() < 0.1: return [ @@ -258,6 +265,7 @@ class Walk: if item and loop: weights.update({"go": 3, "answer": 2, "abandon": 1, "go-stale": 1}) weights.update({"plain-yes": 2, "wait": 2, "question": 2}) + weights.update({"status-here": 1}) if loop.new_project: weights.update({"done": 2}) if item.scope != "general": # conversations aren't named by number @@ -273,6 +281,18 @@ class Walk: elif action == "done" and loop: # The operator says a new project is ready; no entry was added. self.transport.receive(DON, "done", thread=loop.thread) + elif action == "status-here" and loop: + # L5: a command about the item, not a reply: nothing changes, + # not even a countdown. + before = (item.position, loop.wait, loop.rev, loop.grace_until) + self.transport.receive(DON, "status", thread=loop.thread) + after_item = self.world["store"].work_item(item.id) + after = loop_of(after_item) + now = (after_item.position, after.wait, after.rev, after.grace_until) + assert now == before, ( + f"status_changes_nothing: {before} -> {now}: " + f"seed {self.seed}, steps {self.steps}" + ) elif action == "question" and loop: # L5: a question never changes the item it's asked about. before = (item.position, loop.wait, loop.rev, loop.pr, loop.branch) diff --git a/scripts/bringup-repo.sh b/scripts/bringup-repo.sh new file mode 100755 index 0000000..df58556 --- /dev/null +++ b/scripts/bringup-repo.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# The bring-up's test repository (Development Plan, live runs): +# nisse-testing-operator/nisse-bringup, made as the end-to-end tests make +# theirs (tests/integration/e2e_support.py, make_repo), but kept between runs. +# Seeded with the test package, the bring-up agent's account a collaborator, +# main protected, and a deploy key of its own at $E2E/ssh/nisse-bringup.key. +# +# scripts/bringup-repo.sh create make it +# scripts/bringup-repo.sh reset delete it and make it again +# scripts/bringup-repo.sh delete delete it +# +# Needs NISSE_OPERATOR_TOKEN (.env here) and the bring-up agent's token +# (NISSE_GITEA_TOKEN in $E2E/.env), which names the agent's account. +set -euo pipefail +cd "$(dirname "$0")/.." +E2E=${NISSE_E2E:-/root/nisse-e2e} +case "${1:-}" in create|reset|delete) ;; *) sed -n '2,15p' "$0" | sed 's/^# \{0,1\}//'; exit 2 ;; esac +set -a; [ -f .env ] && . ./.env; set +a +exec uv run python - "$1" "$E2E" <<'PY' +import sys +from pathlib import Path + +sys.path.insert(0, "tests/integration") +from e2e_support import OWNER, delete_repo, make_repo, operator_gitea # noqa: E402 + +from nisse_dev.envfile import load_env_file # noqa: E402 +from nisse_dev.gitea import GiteaClient # noqa: E402 + +action, e2e = sys.argv[1], Path(sys.argv[2]) +url = "https://gitea.dongar.net" +operator = operator_gitea(url) +name = "nisse-bringup" +if action in ("reset", "delete"): + delete_repo(operator, f"{OWNER}/{name}") + print(f"deleted {OWNER}/{name}") +if action in ("create", "reset"): + agent_env: dict[str, str] = {} + load_env_file(e2e / ".env", agent_env) + agent = GiteaClient(url, agent_env["NISSE_GITEA_TOKEN"]).whoami() + repo = make_repo(operator, name, e2e / "ssh", agent=agent) + print(f"made {repo.name}: {agent} a collaborator; push key {repo.push_key}") +PY diff --git a/tests/integration/e2e_support.py b/tests/integration/e2e_support.py index e63950f..8b649fc 100644 --- a/tests/integration/e2e_support.py +++ b/tests/integration/e2e_support.py @@ -61,16 +61,14 @@ class FreshRepo: push_key: Path # this repository's deploy key, private half -@contextlib.contextmanager -def fresh_repo( - operator: GiteaClient, run: str, keys: Path, *, agent: str, seed: bool = True -) -> Iterator[FreshRepo]: - """A repository of its own for one test run, as the operator would make a - project's (D4, D8): seeded with a small Python package, the ``agent`` - account a collaborator with write access, the default branch protected (pushes only by the - owner, an approval to merge), and a deploy key of its own, written under - ``keys``. Deleted afterwards, unless NISSE_E2E_KEEP_REPOS is set.""" - name = f"nisse-e2e-{run}" +def make_repo( + operator: GiteaClient, name: str, keys: Path, *, agent: str, seed: bool = True +) -> FreshRepo: + """A repository as the operator would make a project's (D4, D8): seeded + with a small Python package (or only a README), the ``agent`` account a + collaborator with write access, the default branch protected (pushes only + by the owner, an approval to merge), and a deploy key of its own, written + under ``keys``. A failure part-way deletes what was made.""" full = f"{OWNER}/{name}" operator._request( "POST", @@ -147,8 +145,35 @@ def fresh_repo( "required_approvals": 1, }, ) - yield FreshRepo(full, key) + # The labels the agent reads (D1), as a project would have them. + for label, color in ( + ("bug", "#ee0701"), + ("discuss", "#0e8a16"), + ("design", "#5319e7"), + ): + operator._request( + "POST", f"/repos/{full}/labels", json={"name": label, "color": color} + ) + except BaseException: + delete_repo(operator, full) + raise + return FreshRepo(full, key) + + +def delete_repo(operator: GiteaClient, full: str) -> None: + with contextlib.suppress(GiteaError): + operator._request("DELETE", f"/repos/{full}") + + +@contextlib.contextmanager +def fresh_repo( + operator: GiteaClient, run: str, keys: Path, *, agent: str, seed: bool = True +) -> Iterator[FreshRepo]: + """A repository of its own for one test run (make_repo), deleted + afterwards unless NISSE_E2E_KEEP_REPOS is set.""" + repo = make_repo(operator, f"nisse-e2e-{run}", keys, agent=agent, seed=seed) + try: + yield repo finally: if not os.environ.get("NISSE_E2E_KEEP_REPOS"): - with contextlib.suppress(GiteaError): - operator._request("DELETE", f"/repos/{full}") + delete_repo(operator, repo.name)