Merge pull request #36 from 12vectors/task/44-archiving-a-card-reaches-git

44 — Archiving a card reaches git, and so does every other write the board makes to a task file

AGENTS.md

@@ -106,6 +106,16 @@ out of every column, never deleted, Status set to `Archived`. The toast says
 without an undo in the same breath. Cards in the working stages
 (in-progress, review) cannot be archived; finish or walk them back first.
 
+Archiving is a move, so it commits like one: under `BOARD_COMMIT_MOVES`
+the rename into `tasks/archive/` lands in a single `board: <n> → archived
+(<name>)` commit naming both paths, and ⌘Z commits its own way back. That
+is not bookkeeping for its own sake — an uncommitted deletion of a tracked
+file is exactly what `BOARD_SYNC` refuses to run over, so an archive that
+stopped at the disk would silently hold up every later move on that board,
+and the archived card would exist in one working tree only. The same law
+covers every write the board makes to a task file: the `**PR:**` line, a
+claim, and an agent's closing report all reach git the same way.
+
 ## Local commands
 
 Projects grow chores that belong to a specific checkout — applying a

manager/core/agents.py

@@ -22,7 +22,8 @@ import config
 import events
 import reports
 import state
-from taskfiles import actor_name, find_stage_of, move_task, read_task, set_assignee
+from taskfiles import (actor_name, append_to_task, find_stage_of, move_task,
+                       read_task, set_assignee)
 
 
 def _report_of(record: dict, text: str | None = None) -> str:
@@ -39,17 +40,17 @@ def _report_of(record: dict, text: str | None = None) -> str:
 
 
 def _file_report(record: dict, heading: str, report: str) -> None:
-    """The report travels with the task, like every review does."""
+    """The report travels with the task, like every review does — and, like
+    every other board-made write to a task file, it reaches git rather than
+    sitting modified in one working tree (task 44)."""
     stage = find_stage_of(record["task"])
     if not stage or not report:
         return
-    path = config.TASKS / stage / record["task"]
     stamp = time.strftime("%Y-%m-%d %H:%M")
-    try:
-        with path.open("a", encoding="utf-8") as fh:
-            fh.write(f"\n\n---\n\n## {heading} — {stamp} ({record.get('name') or 'agent'})\n\n{report}\n")
-    except OSError:
-        pass
+    name = record.get("name") or "agent"
+    append_to_task(record["task"], stage,
+                   f"\n\n---\n\n## {heading} — {stamp} ({name})\n\n{report}\n",
+                   f"{heading} filed")
 
 
 def _session_report(record: dict, report: str) -> None:

manager/core/taskfiles.py

@@ -1,8 +1,14 @@
-"""Reading and moving task files — the only module that touches tasks/.
+"""Reading, moving and writing task files — the only module that touches tasks/.
 
 The directory a task file sits in *is* its status (see ../AGENTS.md). Nothing
 here knows about agents or HTTP; it is the same folder kanban you could drive
 by hand with mv.
+
+Every write goes out through one of two doors — `_relocate` for anything
+that changes which directory a card sits in, `append_to_task`/`commit_edit`
+for anything written into it where it stands — and both commit under the
+`COMMIT_MOVES` gate. That is deliberate: committing is a property of
+writing to a task file, not something each caller has to remember.
 """
 
 from __future__ import annotations
@@ -105,9 +111,15 @@ def find_stage_of(filename: str) -> str | None:
 ARCHIVE_FROM = {"backlog", "to-do", "done"}
 
 
-def archive_task(filename: str, source: str) -> dict:
+def archive_task(filename: str, source: str, actor: str = "you") -> dict:
     """Archive: out of the flow but never deleted. tasks/archive/ is not a
-    stage — archived cards simply leave the board."""
+    stage — archived cards simply leave the board.
+
+    It is still a board-made write to a task file, so it goes out through
+    `_relocate` like every other one: attributed, and committed under the
+    same gate a move is (an uncommitted deletion of a tracked file is
+    precisely what stops sync publishing anything else).
+    """
     if source not in ARCHIVE_FROM:
         raise ValueError("archive takes cards from backlog, to-do or done only")
     if Path(filename).name != filename or not filename.endswith(".md"):
@@ -121,14 +133,13 @@ def archive_task(filename: str, source: str) -> dict:
     text = src.read_text(encoding="utf-8")
     if STATUS_RE.search(text):
         text = STATUS_RE.sub("**Status:** Archived", text, count=1)
-    dst.parent.mkdir(parents=True, exist_ok=True)
-    src.write_text(text, encoding="utf-8")
-    shutil.move(str(src), str(dst))
+    _relocate(filename, src, dst, text, "archived", actor)
     return {"file": filename, "from": source}
 
 
-def unarchive_task(filename: str, target: str) -> dict:
-    """⌘Z: bring the last archived card back where it came from."""
+def unarchive_task(filename: str, target: str, actor: str = "you") -> dict:
+    """⌘Z: bring the last archived card back where it came from — and
+    record the way back in git, exactly as the way out was."""
     if target not in ARCHIVE_FROM:
         raise ValueError("unknown stage to restore into")
     src = config.TASKS / "archive" / filename
@@ -140,8 +151,7 @@ def unarchive_task(filename: str, target: str) -> dict:
     text = src.read_text(encoding="utf-8")
     if STATUS_RE.search(text):
         text = STATUS_RE.sub(f"**Status:** {config.STAGE_LABELS[target]}", text, count=1)
-    src.write_text(text, encoding="utf-8")
-    shutil.move(str(src), str(dst))
+    _relocate(filename, src, dst, text, target, actor)
     return {"file": filename, "to": target}
 
 
@@ -225,9 +235,20 @@ def _commit(filename: str, message: str, spec: list[str], failure: str) -> bool:
     return False
 
 
+def _number(filename: str) -> str | None:
+    match = NUMBER_RE.match(filename)
+    return match.group(1) if match else None
+
+
 def _commit_move(filename: str, target: str, src: Path, dst: Path, who: str,
                  number: str | None) -> None:
-    """The move and the claim in one commit."""
+    """The move and the claim in one commit.
+
+    Both paths are named, so git records a rename rather than a delete and
+    an add — and a card git has never seen (a brand-new backlog file) names
+    only its destination, since a pathspec matching nothing in HEAD would
+    fail the commit outright.
+    """
     spec = [str(dst)]
     try:
         tracked = _git("ls-files", "--", str(src))
@@ -239,6 +260,27 @@ def _commit_move(filename: str, target: str, src: Path, dst: Path, who: str,
             spec, f"{filename} moved, but committing it failed")
 
 
+def _relocate(filename: str, src: Path, dst: Path, text: str, target: str,
+              actor: str, who: str | None = None) -> None:
+    """The one door out of a directory under tasks/: register who is doing
+    it, write the file, move it, commit it.
+
+    Every mover in this module goes through here, so committing is a
+    property of *writing to a task file* rather than something each caller
+    remembers — the way archiving forgot it. `target` is what the ticker
+    and the commit message call the destination (a stage slug, or
+    `archived`), and `who` is this checkout's git name when the caller has
+    already paid for it.
+    """
+    state.expect_move(filename, target, actor)
+    dst.parent.mkdir(parents=True, exist_ok=True)
+    src.write_text(text, encoding="utf-8")
+    shutil.move(str(src), str(dst))
+    if config.COMMIT_MOVES:
+        _commit_move(filename, target, src, dst,
+                     actor_name() if who is None else who, _number(filename))
+
+
 def commit_edit(filename: str, stage: str, what: str) -> bool:
     """Commit a board-made edit to a card in place — the `**PR:**` line and
     anything else the board writes into a file it does not move.
@@ -251,14 +293,32 @@ def commit_edit(filename: str, stage: str, what: str) -> bool:
     if not config.COMMIT_MOVES:
         return False
     path = config.TASKS / stage / filename
-    number = NUMBER_RE.match(filename)
     return _commit(filename,
-                   f"board: {number.group(1) if number else filename[:-3]} "
+                   f"board: {_number(filename) or filename[:-3]} "
                    f"{what} ({actor_name() or 'board'})",
                    [str(path)],
                    f"{filename}: {what} recorded, but committing it failed")
 
 
+def append_to_task(filename: str, stage: str, text: str, what: str) -> bool:
+    """Append to a card where it stands, and commit the write.
+
+    The other half of the same law: an agent's closing report is the
+    permanent record the project keeps on purpose, so it reaches git like
+    the `**PR:**` line does instead of sitting modified in one working
+    tree. Returns whether the text was written (the commit is the gate's
+    business, and is narrated if it fails).
+    """
+    path = config.TASKS / stage / filename
+    try:
+        with path.open("a", encoding="utf-8") as fh:
+            fh.write(text)
+    except OSError:
+        return False
+    commit_edit(filename, stage, what)
+    return True
+
+
 def move_task(filename: str, source: str, target: str, actor: str = "you") -> dict:
     """Move a task file between stage directories and fix its Status line.
 
@@ -293,11 +353,5 @@ def move_task(filename: str, source: str, target: str, actor: str = "you") -> di
         elif name and claims(source, target):
             text = _set_assignee(text, name)
 
-    state.expect_move(filename, target, actor)
-    dst.parent.mkdir(parents=True, exist_ok=True)
-    src.write_text(text, encoding="utf-8")
-    shutil.move(str(src), str(dst))
-    task = read_task(dst, target)
-    if config.COMMIT_MOVES:
-        _commit_move(filename, target, src, dst, name, task["number"])
-    return task
+    _relocate(filename, src, dst, text, target, actor, who=name)
+    return read_task(dst, target)

tests/test_archive_commits.py

@@ -0,0 +1,246 @@
+"""Archiving a card reaches git (task 44), and so does every other write the
+board makes to a task file.
+
+A move commits itself; an archive used to rename the file on disk and stop
+there, leaving an uncommitted deletion of a tracked file — precisely what
+sync refuses to run over. Appended agent reports had the same gap. These
+cases run against a throwaway git repo, so the commits are real ones.
+
+    python3 -m unittest discover -s tests -v
+"""
+
+from __future__ import annotations
+
+import shutil
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+REPO = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(REPO / "manager" / "core"))
+
+import agents  # noqa: E402
+import config  # noqa: E402
+import state  # noqa: E402
+import taskfiles  # noqa: E402
+import watch  # noqa: E402
+
+CARD = ("# 44 — A card that gets archived\n\n"
+        "**Status:** Backlog\n"
+        "**Priority:** High\n\n"
+        "Body text nobody should touch.\n")
+FILENAME = "44-a-card.md"
+MOVER = "Mover One"
+
+
+def git(cwd: Path, *args: str) -> subprocess.CompletedProcess:
+    return subprocess.run(["git", "-C", str(cwd), *args],
+                          capture_output=True, text=True)
+
+
+class ArchiveReachesGit(unittest.TestCase):
+    def setUp(self):
+        # resolve(): macOS tempdirs sit behind the /var → /private/var
+        # symlink and git reports the resolved path, so absolute pathspecs
+        # only match if we resolve too.
+        tmp = Path(tempfile.mkdtemp(prefix="bench-archive-")).resolve()
+        self.addCleanup(shutil.rmtree, tmp, True)
+        self.repo = tmp / "repo"
+        for slug in config.STAGE_DIRS:
+            (self.repo / "tasks" / slug).mkdir(parents=True)
+        subprocess.run(["git", "init", "-q", "-b", "main", str(self.repo)],
+                       check=True, capture_output=True)
+        git(self.repo, "config", "user.name", MOVER)
+        git(self.repo, "config", "user.email", "mover@example.com")
+
+        self.patch(TASKS=self.repo / "tasks", REPO=self.repo,
+                   SESSIONS_DIR=tmp / "sessions", COMMIT_MOVES=True)
+        state.BOARD_EVENTS.clear()
+        state.EXPECTED_MOVES.clear()
+
+        self.write("backlog", CARD)
+        git(self.repo, "add", "-A")
+        git(self.repo, "commit", "-q", "-m", "root")
+        self.baseline = self.commit_count()
+
+    def patch(self, **values) -> None:
+        for attr, value in values.items():
+            self.addCleanup(setattr, config, attr, getattr(config, attr))
+            setattr(config, attr, value)
+
+    # — the repo under test —
+
+    def path(self, stage: str) -> Path:
+        return self.repo / "tasks" / stage / FILENAME
+
+    def write(self, stage: str, text: str) -> None:
+        self.path(stage).write_text(text, encoding="utf-8")
+
+    def read(self, stage: str) -> str:
+        return self.path(stage).read_text(encoding="utf-8")
+
+    def commit_count(self) -> int:
+        return int(git(self.repo, "rev-list", "--count", "HEAD").stdout.strip())
+
+    def head_message(self) -> str:
+        return git(self.repo, "log", "-1", "--pretty=%s").stdout.strip()
+
+    def head_files(self) -> list[str]:
+        # --no-renames: the point is which paths the commit touched, not how
+        # git chooses to describe the pair.
+        out = git(self.repo, "show", "--name-only", "--no-renames",
+                  "--pretty=format:", "HEAD").stdout
+        return sorted(line for line in out.splitlines() if line.strip())
+
+    def porcelain(self) -> str:
+        return git(self.repo, "status", "--porcelain").stdout
+
+    def dirty(self) -> str:
+        """What sync._clean() looks at: tracked files only."""
+        return git(self.repo, "status", "--porcelain",
+                   "--untracked-files=no").stdout
+
+    # — archiving —
+
+    def test_archiving_commits_the_move_out_of_the_stage(self):
+        taskfiles.archive_task(FILENAME, "backlog")
+
+        self.assertEqual(self.commit_count(), self.baseline + 1)
+        self.assertEqual(self.head_message(), f"board: 44 → archived ({MOVER})")
+        self.assertEqual(self.head_files(),
+                         ["tasks/archive/" + FILENAME, "tasks/backlog/" + FILENAME],
+                         "both paths, so git records a rename not a delete and an add")
+        self.assertEqual(self.porcelain(), "",
+                         "nothing is left behind for a human to find later")
+        self.assertIn("**Status:** Archived", self.read("archive"))
+
+    def test_the_archive_no_longer_stalls_sync(self):
+        """The bug, stated the way it bit: `sync._clean()` reads
+        `git status --porcelain --untracked-files=no`, and an uncommitted
+        deletion of a tracked file makes it false for every later move."""
+        taskfiles.archive_task(FILENAME, "backlog")
+
+        self.assertEqual(self.dirty(), "")
+
+    def test_the_archive_commit_is_pushed_like_a_move(self):
+        """Event-driven push hangs off state.task_committed(), so routing
+        through the same helper publishes the archive without sync having
+        to know it happened."""
+        published: list[str] = []
+        state.COMMIT_HOOKS.append(published.append)
+        self.addCleanup(state.COMMIT_HOOKS.remove, published.append)
+
+        taskfiles.archive_task(FILENAME, "backlog")
+
+        self.assertEqual(published, [FILENAME])
+
+    def test_a_card_git_has_never_seen_is_archived_as_an_addition(self):
+        """A brand-new backlog file has no path in HEAD, so naming the
+        source in the pathspec would fail the commit outright."""
+        fresh = "45-brand-new.md"
+        (self.repo / "tasks" / "backlog" / fresh).write_text(
+            CARD.replace("44", "45"), encoding="utf-8")
+
+        taskfiles.archive_task(fresh, "backlog")
+
+        self.assertEqual(self.commit_count(), self.baseline + 1)
+        self.assertEqual(self.head_message(), f"board: 45 → archived ({MOVER})")
+        self.assertEqual(self.head_files(), ["tasks/archive/" + fresh])
+        self.assertEqual(self.porcelain(), "")
+
+    def test_the_archive_names_who_did_it_rather_than_the_disk(self):
+        taskfiles.archive_task(FILENAME, "backlog")
+
+        self.assertEqual(state.claim_expected(FILENAME, "archived"), "you",
+                         "the mover registers itself, as every other move does")
+
+    # — the way back —
+
+    def test_the_undo_commits_its_own_restore(self):
+        taskfiles.archive_task(FILENAME, "backlog")
+
+        taskfiles.unarchive_task(FILENAME, "backlog")
+
+        self.assertEqual(self.commit_count(), self.baseline + 2,
+                         "one commit out, one commit back")
+        self.assertEqual(self.head_message(), f"board: 44 → backlog ({MOVER})")
+        self.assertEqual(self.head_files(),
+                         ["tasks/archive/" + FILENAME, "tasks/backlog/" + FILENAME])
+        self.assertEqual(self.porcelain(), "")
+        self.assertEqual(self.read("backlog"), CARD,
+                         "the card comes back exactly as it went")
+
+    def test_the_undo_restores_the_status_of_the_stage_it_returns_to(self):
+        shutil.move(str(self.path("backlog")), str(self.path("to-do")))
+        self.write("to-do", CARD.replace("Backlog", "To Do"))
+        git(self.repo, "add", "-A")
+        git(self.repo, "commit", "-q", "-m", "hand move to to-do")
+        self.baseline = self.commit_count()
+        taskfiles.archive_task(FILENAME, "to-do")
+
+        taskfiles.unarchive_task(FILENAME, "to-do")
+
+        self.assertIn("**Status:** To Do", self.read("to-do"))
+        self.assertEqual(self.head_message(), f"board: 44 → to-do ({MOVER})")
+
+    def test_the_restored_card_is_attributed_to_the_person_on_the_ticker(self):
+        """The watcher sees the card reappear in a stage directory; without
+        an expectation registered it would call that appearance `disk`."""
+        taskfiles.archive_task(FILENAME, "backlog")
+        before = {slug: set() for slug in config.STAGE_DIRS}
+
+        taskfiles.unarchive_task(FILENAME, "backlog")
+        state.BOARD_EVENTS.clear()
+        watch.narrate(before, {slug: ({FILENAME} if slug == "backlog" else set())
+                               for slug in config.STAGE_DIRS})
+
+        events = [e for e in state.BOARD_EVENTS if e.get("file") == FILENAME]
+        self.assertEqual([e["actor"] for e in events], ["you"])
+
+    # — appended reports —
+
+    def test_an_appended_report_commits_too(self):
+        record = {"task": FILENAME, "name": "Wren", "log": "/dev/null"}
+
+        agents._file_report(record, "Work report", "It works.")
+
+        self.assertIn("## Work report", self.read("backlog"))
+        self.assertIn("It works.", self.read("backlog"))
+        self.assertEqual(self.commit_count(), self.baseline + 1)
+        self.assertEqual(self.head_message(), f"board: 44 Work report filed ({MOVER})")
+        self.assertEqual(self.head_files(), ["tasks/backlog/" + FILENAME])
+        self.assertEqual(self.porcelain(), "",
+                         "no run leaves a modified task file behind")
+
+    def test_an_empty_report_writes_nothing_and_commits_nothing(self):
+        agents._file_report({"task": FILENAME, "name": "Wren"}, "Work report", "")
+
+        self.assertEqual(self.read("backlog"), CARD)
+        self.assertEqual(self.commit_count(), self.baseline)
+
+    # — the gate —
+
+    def test_gate_off_archives_exactly_as_before(self):
+        self.patch(COMMIT_MOVES=False)
+
+        taskfiles.archive_task(FILENAME, "backlog")
+
+        self.assertEqual(self.commit_count(), self.baseline)
+        self.assertIn("**Status:** Archived", self.read("archive"))
+        self.assertEqual(self.porcelain(),
+                         " D tasks/backlog/44-a-card.md\n?? tasks/archive/\n",
+                         "the archive stays for a human to commit, index untouched")
+
+    def test_gate_off_files_a_report_without_committing_it(self):
+        self.patch(COMMIT_MOVES=False)
+
+        agents._file_report({"task": FILENAME, "name": "Wren"}, "Work report", "Done.")
+
+        self.assertIn("Done.", self.read("backlog"))
+        self.assertEqual(self.commit_count(), self.baseline)
+
+
+if __name__ == "__main__":
+    unittest.main()

tests/test_boards_sync.py

@@ -171,6 +171,22 @@ class TwoBoards(unittest.TestCase):
                          "a move that arrived over origin is not 'disk'")
         self.assertEqual((moves[0]["from"], moves[0]["to"]), ("backlog", "to-do"))
 
+    def test_an_archive_publishes_itself_and_empties_the_other_board(self):
+        """Task 44: an archive is a board-made move like any other, so it
+        commits, pushes and reaches the teammate — where the card is simply
+        gone from every column rather than lingering as one it never saw
+        leave."""
+        self.use(self.ada)
+        taskfiles.archive_task(FILENAME, "backlog")
+        self.assertEqual(sync.push_now(), "ok")
+
+        self.use(self.elena)
+        self.assertEqual(sync.pull_now(), "pulled")
+
+        self.assertIsNone(self.stage_of(self.elena), "out of every column")
+        self.assertTrue((self.elena / "tasks" / "archive" / FILENAME).is_file(),
+                        "never deleted: a fresh clone still has the card")
+
     def test_attribution_is_consumed_once_and_expires(self):
         self.move(self.ada, "backlog", "to-do")
         sync.push_now()