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)