From 0064ec76960d04df2f165bc0a29332c4a7e2ee17 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Thu, 8 Oct 2026 02:16:51 +0000 Subject: [PATCH] Fixes from the rest of the live session - A near miss for abandon ("abort", "cancel", "drop it", ...) in an item's thread is asked about, and changes nothing. - The driver is told only the operator stops or closes an item, so it doesn't claim to have done either. - An issue assigned again after its item closed starts a new item that notes the one before (Gitea's timeline gives the assignment time); item_for_issue returns the latest item. - Abandoning an issue Gitea won't close because of open dependencies says so, naming them. - An agreed item isn't offered restate. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- docs/Architecture.md | 3 + nisse-dev/src/nisse_dev/gitea.py | 25 +++++++ nisse-dev/src/nisse_dev/loop/agent.py | 82 +++++++++++++++++++++-- nisse-dev/src/nisse_dev/loop/items.py | 15 +++-- nisse-dev/tests/test_gitea.py | 27 ++++++++ nisse-dev/tests/test_loop.py | 4 ++ nisse-dev/tests/test_loop_live_fixes.py | 88 ++++++++++++++++++++++++- nisse-dev/tests/test_loop_milestones.py | 4 ++ nisse-dev/tests/test_loop_properties.py | 8 ++- 9 files changed, 242 insertions(+), 14 deletions(-) diff --git a/docs/Architecture.md b/docs/Architecture.md index 29ca249..010e141 100644 --- a/docs/Architecture.md +++ b/docs/Architecture.md @@ -569,6 +569,9 @@ Work arrives as Gitea issues or conversation, and leaves as reviewed PRs. is. The agent acknowledges it ("queued myproject#14, 1 ahead"). Your issue is the item's record. Only issues filed or assigned by allowlisted operators count (L11). - **Unassigned issues are backlog,** never work, though the agent will discuss them. +- **An issue assigned again after its item closed is a new item,** which notes the + one before it; an issue whose item is still open, or whose assignment predates + the close, stays with that item. - **Open Gitea dependencies make an issue blocked** (L1). - **Work that starts in conversation gets a record** once it is work on a project, not at its first message: the transcript already keeps the conversation. The diff --git a/nisse-dev/src/nisse_dev/gitea.py b/nisse-dev/src/nisse_dev/gitea.py index dd22a8a..96a317e 100644 --- a/nisse-dev/src/nisse_dev/gitea.py +++ b/nisse-dev/src/nisse_dev/gitea.py @@ -261,6 +261,31 @@ class GiteaClient: who = (event.get("user") or {}).get("login") return who + def assigned_at(self, repo: str, index: int, assignee: str) -> str | None: + """When the issue was last assigned to ``assignee`` (ISO 8601), from + its timeline, oldest first and paged to the end; None if the + timeline doesn't say.""" + when = None + for page in range(1, MAX_PR_PAGES + 1): + events = ( + self._request( + "GET", + f"/repos/{repo}/issues/{index}/timeline", + params={"limit": PR_PAGE_SIZE, "page": page}, + ) + or [] + ) + for event in events: + if ( + event.get("type") == "assignees" + and not event.get("removed_assignee") + and (event.get("assignee") or {}).get("login") == assignee + ): + when = event.get("created_at") + if len(events) < PR_PAGE_SIZE: + break + return when + def repository(self, repo: str) -> dict[str, Any]: """The repository as this account sees it, permissions included.""" return dict(self._request("GET", f"/repos/{repo}")) diff --git a/nisse-dev/src/nisse_dev/loop/agent.py b/nisse-dev/src/nisse_dev/loop/agent.py index 9e56456..e4c55bb 100644 --- a/nisse-dev/src/nisse_dev/loop/agent.py +++ b/nisse-dev/src/nisse_dev/loop/agent.py @@ -22,6 +22,7 @@ import time import uuid from collections.abc import Callable, Iterable, Sequence from dataclasses import dataclass, replace +from datetime import UTC, datetime from nisse_dev.cancel import Cancel from nisse_dev.canonical import ( @@ -87,6 +88,19 @@ _NEXT = re.compile(rf"(?:do|work on) {_ISSUE} (?:next|first|now)") _NAMED = re.compile(r"(go|approve|abandon|stop) (?:([a-z0-9][a-z0-9-]*))?#(\d+)") # At top level these need an item; without one they are ambiguous (L5). _NEEDS_ITEM = frozenset({"go", "approve", "abandon"}) +# Near-misses for abandon: asked about, never taken for it or for a reply. +_NEAR_ABANDON = frozenset( + { + "abort", + "abort it", + "cancel", + "cancel it", + "drop it", + "quit", + "kill it", + "scrap it", + } +) BUG_GRACE_S = 600.0 GENERAL_IDLE_S = 86400.0 PROGRESS_EVERY_S = 60.0 # progress in prose at a readable rate (L5, #42) @@ -113,6 +127,9 @@ 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. At any point, asking is better than guessing. +Only the operator stops or closes an item (their stop and abandon commands): +never say you have stopped or closed anything yourself. + Before each group of tool calls, say in a sentence what you're doing and why: the operator sees it as your progress. @@ -504,7 +521,8 @@ class LoopAgent: self._oplog.emit("loop.intake_unavailable", cause=str(exc)) return for issue in issues: - if item_for_issue(self._store, issue.number, project.name) is not None: + before = item_for_issue(self._store, issue.number, project.name) + if before is not None and not self._assigned_again(project, issue, before): continue if _MARKER.search(issue.body or ""): continue # D1: a record of work started in conversation @@ -521,7 +539,7 @@ class LoopAgent: except GiteaError as exc: self._oplog.emit("loop.intake_unavailable", cause=str(exc)) continue # taken on the next round - self._open(project, issue, tuple(blocked_by)) + self._open(project, issue, tuple(blocked_by), follows=before) # -- PR review and merge (D4, D5) ------------------------------------------- @@ -689,8 +707,24 @@ class LoopAgent: return False return who in self._gitea_operators + def _assigned_again(self, project: Project, issue: Issue, before: WorkItem) -> bool: + """D1: an issue is taken once, and again only when it's assigned to + the agent anew after its last item closed (abandoned, say, then + given back): never on the same assignment twice.""" + if before.status == "open" or not before.closed_at: + return False + try: + when = self._gitea.assigned_at(project.repo, issue.number, self._account) + except GiteaError: + return False + return bool(when) and _instant(when) > _instant(before.closed_at) + def _open( - self, project: Project, issue: Issue, blocked_by: tuple[str, ...] + self, + project: Project, + issue: Issue, + blocked_by: tuple[str, ...], + follows: WorkItem | None = None, ) -> None: item_id = f"{project.name}-{issue.number}-{uuid.uuid4().hex[:6]}" cause = f"assigned {project.repo}#{issue.number}" @@ -708,6 +742,7 @@ class LoopAgent: issue=issue.number, blocked_by=blocked_by, labels=labels, + follows=follows.id if follows else "", ) self._threads[(project.name, issue.number)] = root self._store.open_work_item( @@ -729,6 +764,12 @@ class LoopAgent: if project.labels else "" ) + + ( + f"; assigned again after its earlier item {follows.id} closed " + f"({follows.outcome})" + if follows + else "" + ) ) ) text = f"{issue.title}\n\n{issue.body}".strip() @@ -830,6 +871,18 @@ class LoopAgent: elif not self._store.active_item(): self._transport.post("Nothing is running.", thread=msg.thread) return + if command in _NEAR_ABANDON: + # L5: a near-miss for a command is asked about, never read as a + # reply (the driver can't stop or close anything itself). + self._store.transcript(None, "operator").append( + UserMessage(text=text, source=f"transport:{msg.sender}") + ) + self._transport.post( + f"Did you mean `abandon`? That closes {self._label(item)} and its " + "issue; say abandon to confirm. (`stop` ends a run in progress.)", + thread=loop_of(item).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. @@ -1666,7 +1719,9 @@ class LoopAgent: loop = replace(loop, prepared=True) save(self._store, item.id, RUNNING, loop, cause="driver run") # D1: a `discuss` item talks before it restates. - hold = "discuss" in loop.labels and not loop.talked and not loop.started + hold = ( + "discuss" in loop.labels and not loop.talked + ) or loop.started # agreed: a restatement is behind it (D1) if loop.question: hold = True # nothing but answering (below) tools = [ @@ -2124,8 +2179,19 @@ class LoopAgent: self._gitea.post_comment(project.repo, loop.issue, f"{outcome}: {note}") self._gitea.close_issue(project.repo, loop.issue) except GiteaError as exc: + try: + blockers = self._gitea.open_dependencies(project.repo, loop.issue) + except GiteaError: + blockers = [] + why = ( + f"Gitea won't close an issue while it depends on open ones " + f"({', '.join(blockers)}): close it when they're done, or " + "remove the dependency" + if blockers + else str(exc) + ) self._transport.post( - f"closed here, but could not close #{loop.issue}: {exc}", + f"closed here, but could not close #{loop.issue}: {why}", thread=loop.thread, ) if outcome == "abandoned": @@ -2133,3 +2199,9 @@ class LoopAgent: def _fact(self, item: WorkItem, text: str) -> None: self._store.transcript(item.id, "orchestrator").append(Fact(text=text)) + + +def _instant(stamp: str) -> datetime: + """An ISO 8601 time from Gitea or the state store, comparable either way.""" + when = datetime.fromisoformat(stamp) + return when if when.tzinfo else when.replace(tzinfo=UTC) diff --git a/nisse-dev/src/nisse_dev/loop/items.py b/nisse-dev/src/nisse_dev/loop/items.py index bbbb3fa..7c40d4b 100644 --- a/nisse-dev/src/nisse_dev/loop/items.py +++ b/nisse-dev/src/nisse_dev/loop/items.py @@ -108,9 +108,12 @@ def item_for_thread( def item_for_issue( store: StateStore, issue: int, project: str | None = None ) -> WorkItem | None: - """Any item, open or closed, recorded by this issue (in ``project``, - when given): intake never takes an issue twice (D1).""" - for item in store.work_items(): - if loop_of(item).issue == issue and project in (None, item.scope): - return item - return None + """The latest item, open or closed, recorded by this issue (in + ``project``, when given): intake takes an issue once, and again only on a + fresh assignment after its item closed (D1).""" + found = [ + item + for item in store.work_items() + if loop_of(item).issue == issue and project in (None, item.scope) + ] + return found[-1] if found else None diff --git a/nisse-dev/tests/test_gitea.py b/nisse-dev/tests/test_gitea.py index 2804e9d..108d8ad 100644 --- a/nisse-dev/tests/test_gitea.py +++ b/nisse-dev/tests/test_gitea.py @@ -229,3 +229,30 @@ def test_closing_comments_first_then_closes_issue_and_pr() -> None: ("POST", "/api/v1/repos/o/r/issues/20/comments", {"body": "abandoned by don"}), ("PATCH", "/api/v1/repos/o/r/pulls/20", {"state": "closed"}), ] + + +def test_assigned_at_is_the_last_assignment_across_pages() -> None: + def assigned(login: str, at: str, removed: bool = False) -> dict: + return { + "type": "assignees", + "assignee": {"login": login}, + "removed_assignee": removed, + "created_at": at, + } + + filler = {"type": "comment", "created_at": "t0"} + pages = { + 1: [assigned("dev-agent", "t1")] + [filler] * (PR_PAGE_SIZE - 1), + 2: [ + assigned("dev-agent", "t2", removed=True), + assigned("someone", "t3"), + assigned("dev-agent", "t4"), + ], + } + + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.path == "/api/v1/repos/o/r/issues/6/timeline" + return httpx.Response(200, json=pages[int(request.url.params["page"])]) + + assert client(handler).assigned_at("o/r", 6, "dev-agent") == "t4" + assert client(handler).assigned_at("o/r", 6, "nobody") is None diff --git a/nisse-dev/tests/test_loop.py b/nisse-dev/tests/test_loop.py index ae490c6..8f45c89 100644 --- a/nisse-dev/tests/test_loop.py +++ b/nisse-dev/tests/test_loop.py @@ -49,6 +49,7 @@ class FakeGitea: self.comments: list[tuple[int, str]] = [] self.closed: list[int] = [] self.deps: dict[int, list[int]] = {} + self.reassigned: dict[int, str] = {} # issue -> when assigned again def create_issue(self, repo: str, title: str, body: str) -> int: n = max([i.number for i in self.issues] + [0]) + 1 @@ -67,6 +68,9 @@ class FakeGitea: def assigned_by(self, repo: str, index: int, assignee: str) -> str: return self.assigner + def assigned_at(self, repo: str, index: int, assignee: str) -> str | None: + return self.reassigned.get(index) + def post_comment(self, repo: str, index: int, body: str) -> None: self.comments.append((index, body)) diff --git a/nisse-dev/tests/test_loop_live_fixes.py b/nisse-dev/tests/test_loop_live_fixes.py index 435552c..19bce0b 100644 --- a/nisse-dev/tests/test_loop_live_fixes.py +++ b/nisse-dev/tests/test_loop_live_fixes.py @@ -1,14 +1,16 @@ """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. +item's thread taken as a reply, no way for the driver to answer a remark; +later, near-miss commands, the driver claiming a stop, a reassigned issue +ignored, an unexplained failed abandon and restating an agreed item. """ 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 import DON, 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 @@ -152,3 +154,85 @@ def test_a_review_comment_can_be_answered_without_a_fix( 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] + + +# --- the session's later findings ---------------------------------------------- + + +def test_a_near_miss_for_abandon_is_asked_about(tmp_path: Path) -> None: + model = ScriptedModel(RESTATED) # called once only: nothing reaches it + h = Harness(tmp_path, model, FakeGitea(issue())) + h.agent.intake() + h.say("abort") + assert h.posted()[-1].startswith("Did you mean `abandon`?") + assert (h.item().status, loop_of(h.item()).wait) == ("open", GO_AHEAD) + h.say("abandon") + assert h.item().status == "closed" + + +def test_the_driver_is_told_it_cant_stop_or_close(tmp_path: Path) -> None: + seen: list[str] = [] + + class Spy(ScriptedModel): + def complete(self, events, tools, model, system=None): # type: ignore[no-untyped-def] + seen.append(system or "") + return super().complete(events, tools, model, system) + + h = Harness(tmp_path, Spy(RESTATED), FakeGitea(issue())) + h.agent.intake() + assert "never say you have stopped or closed anything yourself" in seen[0] + + +def test_an_issue_assigned_again_after_its_item_closed_is_taken_again( + tmp_path: Path, +) -> None: + gitea = FakeGitea(issue()) + h = Harness(tmp_path, ScriptedModel(RESTATED, RESTATED), gitea) + h.agent.intake() + h.say("abandon") + [old] = h.store.work_items() + gitea.closed.remove(14) # the operator reopened it + h.agent.intake() + assert len(h.store.work_items()) == 1 # the same assignment: not again + gitea.reassigned[14] = "2999-01-01T00:00:00Z" # assigned anew, later + h.agent.intake() + items = h.store.work_items() + assert len(items) == 2 + new = next(i for i in items if i.id != old.id) + assert new.status == "open" and loop_of(new).follows == old.id + + +def test_abandoning_a_blocked_issue_says_why_it_stays_open(tmp_path: Path) -> None: + from nisse_dev.gitea import GiteaError + + class Strict(FakeGitea): + def close_issue(self, repo: str, index: int) -> None: + if self.open_dependencies(repo, index): + raise GiteaError("gitea PATCH returned 500: blocked") + super().close_issue(repo, index) + + gitea = Strict(issue(14), issue(15, title="other")) + gitea.deps[14] = [15] + h = Harness(tmp_path, ScriptedModel(RESTATED), gitea) + h.agent._running = True # hold the queue: only the abandon matters + h.agent.intake() + item = next(i for i in h.store.work_items() if loop_of(i).issue == 14) + h.transport.receive(DON, "abandon", thread=loop_of(item).thread) + said = " ".join(h.posted()) + assert "won't close an issue while it depends on open ones" in said + assert "don/sandbox#15" in said + + +def test_an_agreed_item_isnt_offered_restate(world: dict[str, Any]) -> None: + offered: list[set[str]] = [] + + class Spy(ScriptedModel): + def complete(self, events, tools, model, system=None): # type: ignore[no-untyped-def] + offered.append({t["function"]["name"] for t in tools}) + return super().complete(events, tools, model, system) + + model = Spy(RESTATE, [call("ask_human", question="anything else?")]) + agent, transport = loop_agent(world, model) + agent.intake() + say(transport, world, "go") + assert "restate" in offered[0] and "restate" not in offered[1] diff --git a/nisse-dev/tests/test_loop_milestones.py b/nisse-dev/tests/test_loop_milestones.py index 47a7238..196343d 100644 --- a/nisse-dev/tests/test_loop_milestones.py +++ b/nisse-dev/tests/test_loop_milestones.py @@ -61,6 +61,7 @@ class Forge: self.issues = [Issue(14, "Add count_words", "count words", "don", ())] self.deps: dict[int, list[int]] = {} # issue -> the issues it depends on self.assigned: list[int] = [] # issues the agent assigned to itself + self.reassigned: dict[int, str] = {} # issue -> when assigned again # intake def assigned_issues(self, repo: str, assignee: str) -> list[Issue]: @@ -84,6 +85,9 @@ class Forge: def assigned_by(self, repo: str, index: int, assignee: str) -> str: return "don" + def assigned_at(self, repo: str, index: int, assignee: str) -> str | None: + return self.reassigned.get(index) + # PRs def _pr(self, n: int) -> PullRequest: p = self.prs[n] diff --git a/nisse-dev/tests/test_loop_properties.py b/nisse-dev/tests/test_loop_properties.py index 4977431..5beea30 100644 --- a/nisse-dev/tests/test_loop_properties.py +++ b/nisse-dev/tests/test_loop_properties.py @@ -400,7 +400,13 @@ class Walk: where = f"seed {self.seed}, steps {self.steps}" store = self.world["store"] items = self.items() - issues = [(i.scope, loop_of(i).issue) for i in items if loop_of(i).issue] + # Open items: an issue assigned again after its item closed gets a new + # one (D1), which loop.qnt doesn't model. + issues = [ + (i.scope, loop_of(i).issue) + for i in items + if loop_of(i).issue and i.status == "open" + ] assert len(issues) == len(set(issues)), f"one_item_per_issue: {where}" for item in items: loop = loop_of(item)