agent: Valid handoff note still rejected when the work commit beneath it also deletes a handoff note

simple_coding_agent/attempt_workspace.py

@@ -96,8 +96,22 @@ def handoff_note_commits(
         position-independent, and limited to this attempt (earlier rounds'
         note commits are excluded). The newest entry is what
         :meth:`handoff_note_commit` returns. Used by handoff-note validation
-        to skip the unbroken run of this attempt's note commits beneath the
-        newest one when deriving the expected ``last_work_commit``.
+        together with :meth:`handoff_note_commit_carries_work` to skip the
+        unbroken run of this attempt's note commits beneath the newest one
+        when deriving the expected ``last_work_commit``.
+        """
+        ...
+
+    def handoff_note_commit_carries_work(self, revision: str, issue_number: int) -> bool:
+        """Whether a note-touching commit is really a work commit.
+
+        True only when ``revision`` deletes the issue's handoff note while
+        also changing other files -- a work commit that dropped a stale
+        note as a "process artifact" alongside a real change. Such a
+        commit ends the note run, so validation expects it as
+        ``last_work_commit``. A commit that leaves the note in place
+        (even with other files swept in) or that only touches the note
+        (including deleting just the note) is still part of the run.
         """
         ...
 

simple_coding_agent/git_workspace.py

@@ -446,6 +446,33 @@ def handoff_note_commit(
         )
         return commits[0] if commits else None
 
+    def handoff_note_commit_carries_work(self, revision: str, issue_number: int) -> bool:
+        """Whether a note-touching commit is really a work commit.
+
+        True only when ``revision`` deletes the issue's handoff note while
+        also changing other files. ``git log -- <note path>`` (which backs
+        :meth:`handoff_note_commits`) lists such a commit because it
+        touched the path, but it did so by removing a stale note, not by
+        writing one.
+        """
+
+        relpath = handoff_note_relpath(issue_number)
+        files = [
+            line.strip()
+            for line in self._git(
+                "diff-tree", "--no-commit-id", "--name-only", "-r", "--root", revision, "--"
+            ).splitlines()
+            if line.strip()
+        ]
+        touches_other_files = any(path != relpath for path in files)
+        if not touches_other_files:
+            return False
+        try:
+            self._git("cat-file", "-e", f"{revision}:{relpath}")
+        except GitWorkspaceError:
+            return True
+        return False
+
     def cleanup(
         self, *, base_branch: str, prepared: PreparedAttempt, retain_branch: bool = True
     ) -> None:

simple_coding_agent/lifecycle.py

@@ -4894,11 +4894,15 @@ def _this_attempt_note_commit_revisions(
     issue_number: int,
     attempt_id: str,
 ) -> frozenset[str]:
-    """Revisions of this attempt's note-touching commits for note validation.
-
-    An unreadable note history fails closed to just the newest note commit,
-    which is exactly the pre-fix strictness: validation then expects the
-    commit directly beneath the note.
+    """Revisions of this attempt's note-run commits for note validation.
+
+    The run is every note-touching commit of this attempt except work
+    commits that only touched the note path by deleting a stale note
+    alongside a real change: those end the run, so validation expects
+    them as ``last_work_commit`` instead of skipping past them. A
+    commit that cannot be inspected fails closed to work (it is not
+    skipped), which is exactly the pre-fix strictness: validation then
+    expects the commit directly beneath the note.
     """
 
     try:
@@ -4907,7 +4911,17 @@ def _this_attempt_note_commit_revisions(
         )
     except Exception:
         return frozenset()
-    return frozenset(commit.revision for commit in commits)
+    run: list[str] = []
+    for commit in commits:
+        try:
+            carries_work = workspace.handoff_note_commit_carries_work(
+                commit.revision, issue_number
+            )
+        except Exception:
+            continue
+        if not carries_work:
+            run.append(commit.revision)
+    return frozenset(run)
 
 
 def _validate_handoff_note(
@@ -4988,12 +5002,14 @@ def _validate_handoff_note(
     # when the note is the oldest attempt commit. When the model committed
     # the note more than once, the commits beneath the newest note commit
     # are earlier note commits of this same attempt, not work: skip the
-    # unbroken run of this attempt's note-touching commits below the note
-    # (whatever other files they also carry) and expect the first commit
-    # beneath that run. Note commits from earlier rounds of a continued
-    # branch are never in ``note_commit_revisions``, so a new round that
-    # names the previous round's note commit (HEAD at injection) still
-    # validates.
+    # unbroken run of this attempt's note-run commits below the note and
+    # expect the first commit beneath that run. A note-touching commit
+    # that deleted the note while carrying other files is work that
+    # dropped a stale note, not a note commit, so it is already excluded
+    # from ``note_commit_revisions`` and ends the run. Note commits from
+    # earlier rounds of a continued branch are never in
+    # ``note_commit_revisions``, so a new round that names the previous
+    # round's note commit (HEAD at injection) still validates.
     if note_commit_revisions is None:
         note_revisions = {note_commit.revision}
     else:

tests/fakes.py

@@ -97,6 +97,7 @@ class _RecordedCommit:
     body: str
     files: frozenset[str]
     prior_round: bool = False
+    existing: frozenset[str] = frozenset()
 
 
 class InMemoryWorkspace:
@@ -285,6 +286,14 @@ def handoff_note_commit(
         )
         return commits[0] if commits else None
 
+    def handoff_note_commit_carries_work(self, revision: str, issue_number: int) -> bool:
+        relpath = handoff_note_relpath(issue_number)
+        record = next((entry for entry in self._commits if entry.revision == revision), None)
+        if record is None:
+            return True
+        touches_other_files = any(name != relpath for name in record.files)
+        return touches_other_files and relpath not in record.existing
+
     def cleanup(
         self, *, base_branch: str, prepared: PreparedAttempt, retain_branch: bool
     ) -> None:
@@ -315,6 +324,9 @@ def _record_commit(
             body=body,
             files=files,
             prior_round=prior_round,
+            existing=frozenset(
+                name for name in files if (self.working_directory / name).exists()
+            ),
         )
         self._commits.append(record)
         if clear_dirty:

tests/test_attempt_workspace_conformance.py

@@ -433,6 +433,84 @@ def test_handoff_note_commits_is_empty_without_a_note(
     )
 
 
+def test_handoff_note_commit_carries_work_is_false_for_a_note_only_commit(
+    attempt: SimpleNamespace,
+) -> None:
+    note = attempt.workspace.commit_handoff_note(24, "# Handoff note: issue #24\n")
+
+    assert attempt.workspace.handoff_note_commit_carries_work(note.revision, 24) is False
+
+
+def test_handoff_note_commit_carries_work_is_false_for_a_note_commit_with_other_files(
+    attempt: SimpleNamespace,
+) -> None:
+    """A note commit with other files swept in is still a note commit (#34)."""
+
+    if attempt.kind == "git":
+        note_path = attempt.root / ".agent" / "handoff" / "24.md"
+        note_path.parent.mkdir(parents=True, exist_ok=True)
+        note_path.write_text("# Handoff note: issue #24\n")
+        (attempt.root / "work.txt").write_text("work")
+        git(attempt.root, "add", ".agent/handoff/24.md", "work.txt")
+        git(attempt.root, "commit", "-m", "Handoff note: issue #24")
+        note = attempt.workspace.handoff_note_commit(
+            attempt.prepared, 24, attempt_id=attempt.attempt_id
+        )
+        assert note is not None
+        revision = note.revision
+    else:
+        attempt.workspace.make_dirty(".agent/handoff/24.md", "# Handoff note: issue #24\n")
+        attempt.workspace.make_dirty("work.txt", "work")
+        revision = attempt.workspace.model_commit(
+            "Handoff note: issue #24",
+            files=(".agent/handoff/24.md", "work.txt"),
+        ).revision
+
+    assert attempt.workspace.handoff_note_commit_carries_work(revision, 24) is False
+
+
+def test_handoff_note_commit_carries_work_is_false_for_a_note_only_deletion(
+    attempt: SimpleNamespace,
+) -> None:
+    """Deleting just the note carries no work, so it stays in the run."""
+
+    model_commit(attempt, ".agent/handoff/24.md", "# Handoff note: issue #24\n", "Stale note")
+    if attempt.kind == "git":
+        git(attempt.root, "rm", ".agent/handoff/24.md")
+        git(attempt.root, "commit", "-m", "Remove agent handoff note")
+        deletion = attempt.workspace.commits_added(attempt.prepared)[0].revision
+    else:
+        (attempt.root / ".agent" / "handoff" / "24.md").unlink()
+        deletion = attempt.workspace.model_commit(
+            "Remove agent handoff note", files=(".agent/handoff/24.md",)
+        ).revision
+
+    assert attempt.workspace.handoff_note_commit_carries_work(deletion, 24) is False
+
+
+def test_handoff_note_commit_carries_work_is_true_for_work_that_deletes_the_note(
+    attempt: SimpleNamespace,
+) -> None:
+    """Work that drops a stale note alongside a real change ends the run (#216)."""
+
+    model_commit(attempt, ".agent/handoff/24.md", "# Handoff note: issue #24\n", "Stale note")
+    if attempt.kind == "git":
+        (attempt.root / "work.txt").write_text("work")
+        git(attempt.root, "rm", ".agent/handoff/24.md")
+        git(attempt.root, "add", "work.txt")
+        git(attempt.root, "commit", "-m", "Do work, drop the stale handoff note")
+        work = attempt.workspace.commits_added(attempt.prepared)[0].revision
+    else:
+        attempt.workspace.make_dirty("work.txt", "work")
+        (attempt.root / ".agent" / "handoff" / "24.md").unlink()
+        work = attempt.workspace.model_commit(
+            "Do work, drop the stale handoff note",
+            files=(".agent/handoff/24.md", "work.txt"),
+        ).revision
+
+    assert attempt.workspace.handoff_note_commit_carries_work(work, 24) is True
+
+
 def test_cleanup_on_a_dirty_tree_stays_on_the_attempt_branch(
     attempt: SimpleNamespace,
 ) -> None:

tests/test_handoff_end_to_end.py

@@ -711,6 +711,46 @@ def do_handoff(working_directory: Path) -> None:
     assert "Halfway done, with more detail." in comment.body
 
 
+def test_handoff_note_names_work_commit_that_deleted_a_stale_note(tmp_path: Path) -> None:
+    """The #216 shape: a work commit deletes a stale note alongside a code change.
+
+    The work commit touches the handoff note path (by dropping an earlier
+    round's note as a "process artifact"), so it shows up in this attempt's
+    note-touching commits -- but it deletes the note instead of writing it,
+    so it ends the note run: the note names it as ``last_work_commit`` and
+    the attempt still publishes as a ``handoff``.
+    """
+
+    remote, _ = repository_with_main(tmp_path)
+    github = FakeGitHub(issue(24, body="Rewrite the parser.", labels=frozenset({"ready-for-agent"})))
+    clone_dir = tmp_path / "clone"
+
+    def do_handoff(working_directory: Path) -> None:
+        # A stale note from an earlier round, still on the branch.
+        write_handoff_note(working_directory, 24, "Stale note from an earlier round.\n")
+        # This round's work: a code change that also drops the stale note.
+        (working_directory / "parser.py").write_text("half done")
+        git(working_directory, "rm", ".agent/handoff/24.md")
+        git(working_directory, "add", "parser.py")
+        git(working_directory, "commit", "-m", "Half-finish the parser, drop the stale handoff note")
+        work = git(working_directory, "rev-parse", "HEAD")
+        write_handoff_note(working_directory, 24, "Halfway done.\n", last_work_commit=work)
+        write_handoff_note(
+            working_directory, 24, "Halfway done, with more detail.\n", last_work_commit=work
+        )
+
+    executor = FakeModelExecutor(actions=[do_handoff], handoff=True)
+    lifecycle = build_lifecycle(remote, clone_dir, tmp_path / "data", github, executor)
+
+    result = lifecycle.run_once()
+
+    assert result.outcome is AttemptOutcome.HANDOFF
+    assert github.pull_requests == {}
+    assert github.added_labels == [("issue-24", "round-finished")]
+    [comment] = [c for c in github.comments if "Agent Attempt Result: handoff" in c.body]
+    assert "Halfway done, with more detail." in comment.body
+
+
 def test_handoff_note_with_work_swept_into_first_note_names_the_base(tmp_path: Path) -> None:
     """The #34 shape: the first note commit also carries another staged file.