From 57193939592b3ef901f0352a8a7caee86c9ea27f Mon Sep 17 00:00:00 2001 From: Claude Code Date: Tue, 13 Oct 2026 11:02:14 +0000 Subject: [PATCH] Accept a handoff note over a work commit that deleted a stale note A work commit that drops an earlier round's handoff note as a 'process artifact' alongside a real change touches the note path, so the #214 fix skips past it when deriving the expected last_work_commit and a valid note is rejected (downgrading handoff to incomplete). Skip a note-touching commit of this attempt only while it is still a note commit: a commit that deletes the note while carrying other files is work, so it ends the note run and validation expects it. Commits that leave the note in place (even with other files swept in, #34) or that only touch the note (including deleting just the note) stay in the run, and handoff_note_commit locating the newest note is unchanged. Co-Authored-By: Claude Code --- simple_coding_agent/attempt_workspace.py | 18 ++++- simple_coding_agent/git_workspace.py | 27 +++++++ simple_coding_agent/lifecycle.py | 40 +++++++---- tests/fakes.py | 12 ++++ tests/test_attempt_workspace_conformance.py | 78 +++++++++++++++++++++ tests/test_handoff_end_to_end.py | 40 +++++++++++ 6 files changed, 201 insertions(+), 14 deletions(-) diff --git a/simple_coding_agent/attempt_workspace.py b/simple_coding_agent/attempt_workspace.py index 3b59355..c509a9e 100644 --- a/simple_coding_agent/attempt_workspace.py +++ b/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. """ ... diff --git a/simple_coding_agent/git_workspace.py b/simple_coding_agent/git_workspace.py index 52b79fb..4f72fd1 100644 --- a/simple_coding_agent/git_workspace.py +++ b/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 -- `` (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: diff --git a/simple_coding_agent/lifecycle.py b/simple_coding_agent/lifecycle.py index 47a1669..3b68015 100644 --- a/simple_coding_agent/lifecycle.py +++ b/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: diff --git a/tests/fakes.py b/tests/fakes.py index 869023f..5193a6a 100644 --- a/tests/fakes.py +++ b/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: diff --git a/tests/test_attempt_workspace_conformance.py b/tests/test_attempt_workspace_conformance.py index efb807e..23317b4 100644 --- a/tests/test_attempt_workspace_conformance.py +++ b/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: diff --git a/tests/test_handoff_end_to_end.py b/tests/test_handoff_end_to_end.py index 7965d8a..3471537 100644 --- a/tests/test_handoff_end_to_end.py +++ b/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.