Merge pull request #50 from 12vectors/task/59-finishing-a-phase-finishes-its-cards

59 — Finishing a phase finishes its cards, and clears up after them
This commit is contained in:
Ronald Ashri
2026-08-02 12:31:20 +02:00
committed by GitHub
8 changed files with 825 additions and 48 deletions
+55 -5
View File
@@ -530,8 +530,10 @@ changes), **just move the card** (branch, PR and worktree stay), or
branch into main, push (which marks the PR merged) and delete the remote
branch, remove the worktree and local branch, then move the card. Every
step narrates in the ticker; a merge conflict aborts cleanly and the card
stays put. Cards without work move silently, and hand-moves on disk are
never intercepted — the board only asks when you act through it.
stays put. On a phase card the same action also finishes the cards the
phase merged — see "Finishing a phase finishes its cards". Cards without
work move silently, and hand-moves on disk are never intercepted — the
board only asks when you act through it.
That work takes as long as it takes, so **the card wears it** rather than
sitting there looking idle while its branch is disassembled: from the
@@ -975,6 +977,51 @@ stage — and, while a phase is in flight, the runner's own reading of each
(merged in, working, checking, stopped here) — so the card answers "where
is this up to" without a hunt across five columns.
### Finishing a phase finishes its cards
A member stops at `review/` on purpose: `done/` has always meant *merged
into `main`*, and merged into a phase branch is not that. **Merge & clean
up** on the phase card is the moment it becomes that. The phase's PR goes
into `main`, so every card the phase carried is in `main` too — and since
a phase reaching `done/` releases its members back onto the Board, the
alternative is handing back three or five cards you have already judged,
in the column whose note is "your move".
So the merge sweeps. After it has actually succeeded — never before, and
never if it conflicts — every member the phase merged moves to `done/`,
and each one's workspace is cleared exactly as completing an ordinary card
clears its own: the worktree removed, the local branch deleted, the branch
on the remote deleted. That half is not cosmetic. A stale worktree is a
trap laid for whoever reopens the card, since a work launch refuses a card
whose worktree already exists, and the accumulation is per member per
phase.
Four rules keep it honest:
- **Only what the phase merged.** A member is swept when its card settled
into `review/` or `done/` *and* its branch is contained in the phase
branch — the same pair the runner reads a member as `merged` by, and a
member with no branch at all was finished before the phase reached it.
One that halted, was held or was walked back is neither moved nor
cleared: there is work in its worktree and its branch, and it is the
reason a person will look at this phase afterwards.
- **Nothing uncommitted is thrown away.** A member's worktree comes out
without `--force`, so one with uncommitted changes in it is reported in
the ticker and kept, with its branch. Removing a worktree with work in
it is the one unrecoverable step in the ending.
- **One ending, told once.** The cards move together in a single commit
naming all of them (`board: 47, 52 → done with phase 53`), and the
ticker gets one line — the phase, how many cards went with it, and how
many it never merged and left where they are — instead of five moves
scrolling past. In team mode that commit publishes like any other.
- **Only merging sweeps.** "Just move the card" leaves the work alone,
including the members'. Archiving the phase card, or dropping a number
from its `## Cards`, releases the members to the Board in whatever stage
they are genuinely in (that is card 56's rule, and it is unchanged) —
neither one says anything is done, because neither one puts anything in
`main`. The members' own PRs are closed by their own merges into the
phase branch and are not touched here.
### A phase's members leave the Board view
The Board is the work you are personally holding, in five columns that
@@ -990,9 +1037,12 @@ Membership is the only thing that hides a card, so *removing* membership
is the un-hiding, with no sweep and no second rule: archive the phase
card, drop a number from its `## Cards` list, or let the phase reach
`done/` — it holds nothing once it is over — and its former members are
back in the columns they are genuinely in. A membership that did not
resolve hides nothing either: a card wearing `phase drift` stays on the
board, because an authoring mistake must not make work vanish.
back in the columns they are genuinely in. Which, when the phase got
there by being merged, is `done/`: the merge moved them (see "Finishing a
phase finishes its cards"). The other two endings move nothing, so the
cards they release are wherever they actually were. A membership that did
not resolve hides nothing either: a card wearing `phase drift` stays on
the board, because an authoring mistake must not make work vanish.
That leaves the phase card standing for all of it, so it carries the
summary it owes: a `⟶ 1 of 2 merged` chip in the footer row that opens
+14 -2
View File
@@ -2002,6 +2002,11 @@ function closeSheet() { $('#sheetwrap').classList.remove('open'); $('#sheetwrap'
function completeSheet(task, from) {
const d = S.state.drive;
const driving = d && d.task === task.file && ['starting', 'up'].includes(d.status);
// A phase card stands for cards the Board does not draw, so the sheet
// has to say what merging it does to them: the ones it merged go to
// done/ with it and their workspaces go too. Counted the way the lane
// counts them, from the runner's reading and the phase's own log.
const swept = task.isPhase ? mergedIn(task).size : 0;
const wrap = $('#sheetwrap');
wrap.innerHTML =
`<div class="sheet">` +
@@ -2010,13 +2015,15 @@ function completeSheet(task, from) {
`<p>This task has ${task.pr ? 'a PR and ' : ''}a branch with work on it. What should happen?</p>` +
`<div class="sbtns">` +
`<button id="sh-keep">Keep it where it is<small>Nothing moves, nothing changes.</small></button>` +
`<button id="sh-move">Just move the card<small>The branch${task.pr ? ', PR' : ''} and worktree stay as they are.</small></button>` +
`<button id="sh-move">Just move the card<small>The branch${task.pr ? ', PR' : ''} and worktree stay as they are` +
`${task.isPhase ? ", and so does every card in the phase" : ''}.</small></button>` +
// team mode merges on origin: the board never makes a merge commit of
// its own, so every replica's main keeps fast-forwarding
`<button id="sh-ship" class="shipit">Merge &amp; clean up<small>${driving ? 'Park the drive, then m' : 'M'}erge ` +
((S.state.sync || {}).enabled
? `the PR on GitHub, remove the worktree and branches, move the card — local main fast-forwards on the next sync beat.`
: `the branch into main, push${task.pr ? ' (marks the PR merged)' : ''}, remove the worktree and branches, move the card.`) +
(swept ? ` The ${swept} card${swept === 1 ? '' : 's'} this phase merged go to done/ with it, and their worktrees and branches go too.` : '') +
`</small></button>` +
`</div></div>`;
wrap.classList.add('open');
@@ -2032,7 +2039,12 @@ function completeSheet(task, from) {
});
const data = await res.json();
if (!res.ok) { toast(data.error || 'completion failed — the card stays', true); }
else toast(data.merged ? `${task.file} merged and cleaned up` : `${task.file} moved to done`);
else {
const went = (data.swept || []).length;
toast(data.merged ? `${task.file} merged and cleaned up` +
(went ? ` — ${went} card${went === 1 ? '' : 's'} went to done/ with it` : '')
: `${task.file} moved to done`);
}
await loadState();
});
}
+135 -2
View File
@@ -28,7 +28,7 @@ import drive as drive_mod
import reports
import state
from taskfiles import (STATUS_RE, collect, commit_edit, find_stage_of,
move_task, read_task)
member_entry, move_task, move_together, read_task)
PR_STATE: dict[str, dict] = {} # filename -> {verdict, detail, url, ts}
_OPENING: set[str] = set() # filenames with a PR-open in flight
@@ -53,6 +53,13 @@ def _branch_exists(branch: str) -> bool:
return _run(["git", "rev-parse", "--verify", "--quiet", branch]).returncode == 0
def _contains(branch: str, tip: str) -> bool:
"""Is `branch` already in `tip`? The same question phases.py asks of a
member a merge leaves no record but this one, and it is the only
thing that can say a phase's ending speaks for a particular card."""
return _run(["git", "merge-base", "--is-ancestor", branch, tip]).returncode == 0
def branch_of(filename: str) -> str:
"""The branch a card's work lives on. Ordinary cards get `task/<stem>`;
a phase runs on `phase/<stem>`, its own integration branch (phases.py),
@@ -493,6 +500,11 @@ def _complete(filename: str, stage: str) -> dict:
"""The steps themselves, run under the claim complete_task holds."""
stem = filename[:-3]
branch = branch_of(filename) # a phase's own branch, or task/<stem>
task = _woven(filename, stage)
# Read before anything is destroyed and acted on only after the merge
# lands: the phase branch is deleted in the middle of this, and it is
# the only thing that can say which members it carried.
brought = _brought(task, branch) if task.get("isPhase") else []
# 1. the app must not keep running code that is about to be merged away
d = drive_mod.DRIVE
@@ -504,6 +516,7 @@ def _complete(filename: str, stage: str) -> dict:
time.sleep(0.5)
merged = False
swept: list[dict] = []
if _branch_exists(branch):
if config.SYNC:
_merge_on_origin(filename, stage, branch)
@@ -522,9 +535,129 @@ def _complete(filename: str, stage: str) -> dict:
"kind": "agent", "actor": "board", "file": filename,
"summary": f"cleaned up: worktree and local branch for {stem} removed"})
# the merge succeeded, so the members' work is in main too
swept = _sweep(filename, task, brought)
move_task(filename, stage, "done", actor="you")
state.broadcast({"type": "board"})
return {"merged": merged}
return {"merged": merged, "swept": [card["file"] for card in swept]}
def _brought(task: dict, phase_branch: str) -> list[dict]:
"""The members a phase actually merged — the only cards its ending
speaks for.
Two things have to be true, and they are the same pair phases.py reads
a member as `merged` by: the card settled into `review/` or `done/`,
and its branch is contained in the phase branch or there was never a
branch to bring, which is a member finished before the phase reached
it. Containment alone is not enough, for the reason it is not enough
there either: a run that exits without committing leaves an empty
branch that is trivially contained.
A member that halted, was held or was walked back satisfies neither.
Its card stays where it is and so do its worktree and its branch:
there is work in them, and it is the reason a person will look at this
phase afterwards. Removing a worktree with work in it is the one
unrecoverable thing here.
"""
brought = []
for member in task.get("members") or []:
if member.get("stage") not in ("review", "done"):
continue
branch = f"task/{member['file'][:-3]}"
if _branch_exists(branch) and not _contains(branch, phase_branch):
continue
brought.append(member)
return brought
def _clear_member(member: dict) -> str:
"""One merged member's workspace, cleared exactly as completing an
ordinary card clears its own: the worktree, the local branch, the
branch on the remote. Returns what to say when something was kept.
The worktree comes out without `--force`, which is the whole
difference from the card's own cleanup above. A member's work is in
`main` by the time this runs, so nothing in there is *needed* but
that is a judgement about committed files, and an uncommitted change
is exactly what it does not cover. A dirty worktree is reported and
kept, with its branch, rather than forced: this is the one
unrecoverable step in the ending, and it is asked before it is taken
rather than left to git to refuse.
"""
stem = member["file"][:-3]
branch = f"task/{stem}"
worktree = config.WORKTREES / stem
try:
if worktree.exists():
dirty = _run(["git", "-C", str(worktree), "status",
"--porcelain"]).stdout.split("\n")
dirty = [line for line in dirty if line.strip()]
if dirty:
return (f"{stem}: worktree kept — {len(dirty)} uncommitted "
f"change{'' if len(dirty) == 1 else 's'} in it")
removed = _run(["git", "worktree", "remove", str(worktree)])
if removed.returncode != 0:
detail = (removed.stderr.strip() or removed.stdout.strip()
or "git would not remove it")
return f"{stem}: worktree kept — {detail.splitlines()[-1][:120]}"
if _branch_exists(branch):
# -D under sync for the same reason the card's own branch takes
# it: main here has not caught up with the merge origin made.
_run(["git", "branch", "-D" if config.SYNC else "-d", branch])
rname = remote()
if rname:
_run(["git", "push", rname, "--delete", branch], timeout=60)
except (subprocess.SubprocessError, OSError) as exc:
return f"{stem}: workspace kept — {str(exc)[:120]}"
PR_STATE.pop(member["file"], None)
return ""
def _sweep(filename: str, task: dict, brought: list[dict]) -> list[dict]:
"""Finish the cards the phase finished.
A member stops at `review/` on purpose `done/` has always meant
merged into `main`, and merged into a phase branch is not that. This
is the moment it becomes that: the phase's PR is in `main`, so every
card it carried is too, and leaving them in the column whose note is
"your move" would hand back a pile of work already judged.
One ending, so it is told as one: the cards move together in a single
commit (`taskfiles.move_together`) and the ticker gets one line naming
the phase and how many went with it, rather than five moves scrolling
past. What was *not* swept is named too a phase that left cards
behind is a phase somebody still has to look at.
"""
members = task.get("members") or []
if not members: # a phase card listing nobody
return []
kept = [note for note in (_clear_member(member) for member in brought) if note]
moved = move_together([(member["file"], member["stage"]) for member in brought
if member["stage"] != "done"],
"done", f"with phase {task['number'] or filename[:-3]}")
left = len(members) - len(brought)
name = (member_entry(task["number"], task["title"]) if task["number"]
else task["title"])
summary = f"phase {name} finished — "
summary += (f"{_cards(len(brought))} merged into it went to done/, worktrees "
f"and branches cleaned up" if brought
else "it had merged nothing, so no card went with it")
if left:
summary += (f" · {_cards(left)} it never merged "
+ ("stays where it is" if left == 1 else "stay where they are"))
state.record_board_event({"kind": "phase", "actor": "board",
"file": filename, "summary": summary})
for note in kept:
state.record_board_event({
"kind": "agent", "actor": "board", "file": filename,
"summary": f"{note} — nothing uncommitted is thrown away"})
return moved
def _cards(count: int) -> str:
return f"{count} card{'' if count == 1 else 's'}"
def _merge_locally(filename: str, branch: str) -> None:
+1 -1
View File
@@ -709,7 +709,7 @@ def _member_name(member: dict) -> str:
way the card that holds it does."""
if not member.get("number"):
return member["file"]
return taskfiles._member_entry(member["number"], member["title"])
return taskfiles.member_entry(member["number"], member["title"])
def _joined(parts: list[str]) -> str:
+24 -11
View File
@@ -21,7 +21,7 @@ SESSIONS: dict[str, dict] = {} # session_id -> meta
EVENTS: dict[str, list[dict]] = {} # session_id -> slim events
BOARD_EVENTS: list[dict] = [] # moves + agent lifecycle
AGENTS: dict[str, dict] = {} # agent_id -> launch record
EXPECTED_MOVES: dict[tuple[str, str], tuple[str, float]] = {} # (file, to) -> (actor, ts)
EXPECTED_MOVES: dict[tuple[str, str], tuple[str, float, bool]] = {} # (file, to) -> (actor, ts, quiet)
COMMIT_HOOKS: list = [] # run after a board-made task commit
COMPLETING: dict[str, dict] = {} # filename -> {started, step}: merge & clean up in flight
@@ -173,17 +173,30 @@ def task_committed(filename: str) -> None:
pass
def expect_move(filename: str, target: str, actor: str) -> None:
"""Tell the watcher who is about to move a file so it can attribute it."""
def expect_move(filename: str, target: str, actor: str, quiet: bool = False) -> None:
"""Tell the watcher who is about to move a file so it can attribute it.
`quiet` adds the other half of that: the mover has already narrated
this one, so the watcher renders it and says nothing. A phase ending
moves every card it merged at once and reports it as the one thing it
is; five identical move lines scrolling behind that would be the same
fact told badly. It suppresses the ticker line only the move is
still a move, and everything the board hangs off one still happens.
"""
with LOCK:
EXPECTED_MOVES[(filename, target)] = (actor, time.time())
EXPECTED_MOVES[(filename, target)] = (actor, time.time(), quiet)
def claim_move(filename: str, target: str) -> tuple[str, bool]:
"""Who moved this, and whether they already said so."""
with LOCK:
expected = EXPECTED_MOVES.pop((filename, target), None)
# forget stale expectations while we're here
cutoff = time.time() - 30
for key in [k for k, (_, ts, _) in EXPECTED_MOVES.items() if ts < cutoff]:
EXPECTED_MOVES.pop(key, None)
return (expected[0], expected[2]) if expected else ("disk", False)
def claim_expected(filename: str, target: str) -> str:
with LOCK:
actor_ts = EXPECTED_MOVES.pop((filename, target), None)
# forget stale expectations while we're here
cutoff = time.time() - 30
for key in [k for k, (_, ts) in EXPECTED_MOVES.items() if ts < cutoff]:
EXPECTED_MOVES.pop(key, None)
return actor_ts[0] if actor_ts else "disk"
return claim_move(filename, target)[0]
+70 -15
View File
@@ -375,14 +375,13 @@ def _number(filename: str) -> str | None:
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.
def _move_spec(src: Path, dst: Path) -> list[str]:
"""The paths one move's commit names.
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.
Both ends, 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:
@@ -391,12 +390,19 @@ def _commit_move(filename: str, target: str, src: Path, dst: Path, who: str,
spec.insert(0, str(src)) # git knew the old path: record its removal
except (subprocess.SubprocessError, OSError):
pass
return spec
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."""
_commit(filename, f"board: {number or filename[:-3]}{target} ({who or 'board'})",
spec, f"{filename} moved, but committing it failed")
_move_spec(src, dst), 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:
actor: str, who: str | None = None, commit: bool = True,
quiet: bool = False) -> None:
"""The one door out of a directory under tasks/: register who is doing
it, write the file, move it, commit it.
@@ -406,12 +412,16 @@ def _relocate(filename: str, src: Path, dst: Path, text: str, target: str,
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.
`commit` and `quiet` are what a batch of moves turns off: several cards
that moved as one thing are committed as one thing and narrated as one
thing (`move_together`), and both would be wrong done per file.
"""
state.expect_move(filename, target, actor)
state.expect_move(filename, target, actor, quiet=quiet)
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:
if config.COMMIT_MOVES and commit:
_commit_move(filename, target, src, dst,
actor_name() if who is None else who, _number(filename))
@@ -492,7 +502,7 @@ PHASE_JOIN_FROM = {"backlog", "to-do"} # a card joins a phase before it starts
PHASE_HOST = "to-do" # and only a phase still waiting accepts it
def _member_entry(number: str, title: str) -> str:
def member_entry(number: str, title: str) -> str:
"""`33 — The landing page` — the way a person writes it.
The section is authored by hand and read by hand, so a line the board
@@ -569,7 +579,7 @@ def add_to_phase(filename: str, stage: str, phase_file: str) -> dict:
else f"phase {_phase_name(holder)} already")
raise ValueError(f"{where} lists {card['number']}")
entry = _member_entry(card["number"], card["title"])
entry = member_entry(card["number"], card["title"])
if not append_to_section(phase_file, PHASE_HOST, "Cards", f"- {entry}",
f"gained {number}"):
raise ValueError(f"{phase_file} could not be written")
@@ -578,12 +588,16 @@ def add_to_phase(filename: str, stage: str, phase_file: str) -> dict:
"entry": entry, "line": f"- {entry}"}
def move_task(filename: str, source: str, target: str, actor: str = "you") -> dict:
def move_task(filename: str, source: str, target: str, actor: str = "you",
commit: bool = True, quiet: bool = False) -> dict:
"""Move a task file between stage directories and fix its Status line.
With `BOARD_COMMIT_MOVES` on, the move also claims the card (an
**Assignee:** line, this checkout's git name) or releases it when the
card is walked back to backlog, and commits the whole change.
`commit` and `quiet` belong to `move_together` below a lone move
commits itself and narrates itself, as it always has.
"""
if source not in config.STAGE_DIRS or target not in config.STAGE_DIRS:
raise ValueError("unknown stage")
@@ -612,5 +626,46 @@ def move_task(filename: str, source: str, target: str, actor: str = "you") -> di
elif name and claims(source, target):
text = _set_assignee(text, name)
_relocate(filename, src, dst, text, target, actor, who=name)
_relocate(filename, src, dst, text, target, actor, who=name,
commit=commit, quiet=quiet)
return read_task(dst, target)
def move_together(moves: list[tuple[str, str]], target: str, note: str,
actor: str = "you") -> list[dict]:
"""Move several cards into one stage as one thing.
A phase's ending is one event that happens to move five files, so the
record it leaves should be one too: one commit naming every card that
moved, rather than five `board: NN done` lines in a row that a
reader of `git log` has to reassemble into the thing that happened.
The message still says what moved the numbers are in it and it
carries the `board: ` prefix, so in team mode it reaches the other
boards on the same push every other move rides.
The moves are quiet for the same reason (see `state.expect_move`): the
caller narrates the ending, and the ticker does not also scroll the
parts. Each takes `moves` as `(filename, source stage)`.
A card that cannot be moved gone, or already in `target` is
skipped rather than failing the rest. This runs after work that has
already landed in `main`, and finishing four cards beats abandoning
the sweep over the fifth.
"""
moved, spec = [], []
for filename, source in moves:
src = config.TASKS / source / filename
dst = config.TASKS / target / filename
try:
moved.append(move_task(filename, source, target, actor=actor,
commit=False, quiet=True))
except (ValueError, OSError):
continue
spec += _move_spec(src, dst)
if moved and config.COMMIT_MOVES:
numbers = ", ".join(task["number"] or task["file"][:-3] for task in moved)
_commit(moved[0]["file"],
f"board: {numbers}{target}" + (f" {note}" if note else "")
+ f" ({actor_name() or 'board'})",
spec, f"{len(moved)} cards moved, but committing them failed")
return moved
+20 -12
View File
@@ -30,20 +30,25 @@ def _board_sig() -> dict[str, set[str]]:
return sig
def _actor(filename: str, stage: str) -> tuple[str, bool]:
"""Who did this, and whether it happened somewhere else.
def _actor(filename: str, stage: str) -> tuple[str, bool, bool]:
"""Who did this, whether it happened somewhere else, and whether it has
already been narrated.
A move this board made is claimed from the expectations; one a pull
brought carries its commit author's name and is *remote* — this board
is only rendering it; a plain mv on this disk is nobody in particular,
but it is still this board's own disk, so it acts.
*Quiet* is the mover saying it has told this story already: a phase
ending sweeps its whole list into done/ and reports one ending. The
line is skipped; nothing else about the move is.
"""
actor = state.claim_expected(filename, stage)
actor, quiet = state.claim_move(filename, stage)
if actor == "disk":
who = sync.arrived_actor(filename)
if who:
return who, True
return actor, False
return who, True, False
return actor, False, quiet
def narrate(prev: dict[str, set[str]], cur: dict[str, set[str]]) -> None:
@@ -52,12 +57,13 @@ def narrate(prev: dict[str, set[str]], cur: dict[str, set[str]]) -> None:
cur_loc = {f: s for s, files in cur.items() for f in files}
for f, stage in sorted(cur_loc.items()):
if f in prev_loc and prev_loc[f] != stage:
actor, remote = _actor(f, stage)
state.record_board_event({
"kind": "move", "file": f, "from": prev_loc[f], "to": stage,
"actor": actor, "remote": remote,
"summary": f"{f} moved {prev_loc[f]}{stage} ({actor})",
})
actor, remote, quiet = _actor(f, stage)
if not quiet:
state.record_board_event({
"kind": "move", "file": f, "from": prev_loc[f], "to": stage,
"actor": actor, "remote": remote,
"summary": f"{f} moved {prev_loc[f]}{stage} ({actor})",
})
# a failed run is worn by the card in the stage it died in —
# wherever the card goes next, it arrives without the alarm
agents.forget_failure(f)
@@ -67,7 +73,9 @@ def narrate(prev: dict[str, set[str]], cur: dict[str, set[str]]) -> None:
# per replica
github.open_pr_async(f)
elif f not in prev_loc:
actor, remote = _actor(f, stage)
actor, remote, quiet = _actor(f, stage)
if quiet:
continue
state.record_board_event({
"kind": "new", "file": f, "to": stage, "actor": actor,
"remote": remote,
+506
View File
@@ -0,0 +1,506 @@
"""Finishing a phase finishes its cards, and clears up after them (task 59).
A member stops at `review/` on purpose, and **merge & clean up** on the
phase card is the moment that stops being right: the phase's branch is in
`main`, so every card it carried is too. These run the real thing a real
phase run over a real git repo, then a real merge because what the card
is about is which branches ended up inside which, and mocking git would be
mocking the subject.
python3 -m unittest discover -s tests -v
"""
from __future__ import annotations
import shutil
import subprocess
import sys
import unittest
from pathlib import Path
REPO = Path(__file__).resolve().parents[1]
sys.path.insert(0, str(REPO / "manager" / "core"))
import config # noqa: E402
import github # noqa: E402
import state # noqa: E402
import taskfiles # noqa: E402
import watch # noqa: E402
from tests.test_phase_runs import (DIES, ONE, PHASE, PHASE_BRANCH, TWO, # noqa: E402
PhaseCase, card, git)
THREE = "33-the-landing-page.md"
THREE_LISTED = ("- 31 — Stand up site/\n- 32 — Serve it\n"
"- 33 — The landing page\n")
class PhaseEnding(PhaseCase):
"""The whole run, then the ending: two members merged into the phase
branch, the phase card in review/, and a person dragging it to done/."""
def run_phase(self) -> None:
self.start() # cut the branch, run 31
self.advance() # merge 31, run 32
self.advance() # merge 32, open the phase PR, card → review/
self.assertEqual(self.stage_of(PHASE), "review", "the run never finished")
def complete(self, stage: str = "review") -> dict:
return github.complete_task(PHASE, stage)
def worktrees(self) -> list[str]:
return [line.split(" ", 1)[1] for line in
git(self.repo, "worktree", "list", "--porcelain").stdout.splitlines()
if line.startswith("worktree ")]
def branches(self) -> list[str]:
return git(self.repo, "for-each-ref", "--format=%(refname:short)",
"refs/heads/").stdout.split()
def ending(self) -> str:
"""The one line the ticker gets for the ending."""
lines = [s for s in self.summaries() if s.startswith("phase ")
and "finished" in s]
self.assertEqual(len(lines), 1, f"one ending, not {len(lines)}: {lines}")
return lines[0]
class AFinishedPhase(PhaseEnding):
"""Merge & clean up on the phase card, single-player and offline."""
# — the cards —
def test_the_phase_card_and_every_merged_member_are_done(self):
self.run_phase()
self.complete()
self.assertEqual(self.stage_of(PHASE), "done")
self.assertEqual(self.stage_of(ONE), "done")
self.assertEqual(self.stage_of(TWO), "done")
def test_the_swept_cards_say_done_in_their_own_headers(self):
"""A move is the file *and* the Status line — a swept card that
still says Review would wear `status drift` the moment it landed."""
self.run_phase()
self.complete()
self.assertIn("**Status:** Done", self.text(ONE))
self.assertIn("**Status:** Done", self.text(TWO))
def test_the_completion_reports_what_went_with_it(self):
self.run_phase()
result = self.complete()
self.assertTrue(result["merged"])
self.assertEqual(sorted(result["swept"]), sorted([ONE, TWO]),
"the browser is told which cards went, for its toast")
# — the workspaces —
def test_no_merged_member_leaves_a_worktree_or_a_branch(self):
self.run_phase()
for stem in (PHASE[:-3], ONE[:-3], TWO[:-3]):
self.assertIn(str(config.WORKTREES / stem), self.worktrees(),
"the run should have left one to clean up")
self.complete()
self.assertEqual(self.worktrees(), [str(self.repo)],
"git worktree list names only what was there before")
self.assertEqual(self.branches(), ["main"],
"git branch names only what was there before")
self.assertFalse((config.WORKTREES / ONE[:-3]).exists())
self.assertFalse((config.WORKTREES / TWO[:-3]).exists())
def test_a_members_pr_state_is_dropped_with_its_branch(self):
self.run_phase()
github.PR_STATE[ONE] = {"verdict": "green", "ci": "pass", "url": "u"}
self.complete()
self.assertNotIn(ONE, github.PR_STATE)
# — one ending, said once —
def test_the_ticker_reports_one_ending_naming_the_phase_and_the_count(self):
self.run_phase()
state.BOARD_EVENTS.clear()
self.complete()
line = self.ending()
self.assertIn("40 — Ship the site", line)
self.assertIn("2 cards", line)
self.assertIn("done/", line)
def test_the_member_moves_do_not_also_scroll_past(self):
"""The watcher narrates every move it finds on disk. A sweep is one
thing that happens to move several files, so the moves it makes are
claimed as already told and the phase card's own move is not."""
self.run_phase()
before = watch._board_sig()
state.BOARD_EVENTS.clear()
self.complete()
watch.narrate(before, watch._board_sig())
moves = [e for e in state.BOARD_EVENTS if e.get("kind") == "move"]
self.assertEqual([e["file"] for e in moves], [PHASE],
"only the card the person dragged narrates its move")
self.ending()
def test_a_card_moved_normally_still_narrates(self):
"""The suppression is scoped to the sweep: an ordinary move made
through the board is as loud as it ever was."""
before = watch._board_sig()
taskfiles.move_task(ONE, "backlog", "to-do")
watch.narrate(before, watch._board_sig())
self.assertTrue(any(e.get("kind") == "move" and e["file"] == ONE
for e in state.BOARD_EVENTS))
# — only what the phase merged —
def test_a_member_the_phase_never_merged_is_left_exactly_as_it_is(self):
"""Walked back out of review/ before the phase could bring it: its
card, its worktree and its branch are none of the ending's
business, and there is work in all three."""
self.write_cards(listed=THREE_LISTED)
self.write(THREE, card("33 — The landing page"))
self.start()
self.advance() # merge 31, run 32
self.advance() # merge 32, run 33
self.assertEqual(self.stage_of(THREE), "review")
taskfiles.move_task(THREE, "review", "in-progress")
state.BOARD_EVENTS.clear()
self.complete("in-progress")
self.assertEqual(self.stage_of(THREE), "in-progress", "the card stays")
self.assertTrue(self.branch_exists(f"task/{THREE[:-3]}"))
self.assertIn(str(config.WORKTREES / THREE[:-3]), self.worktrees())
self.assertEqual(self.stage_of(ONE), "done")
self.assertEqual(self.stage_of(TWO), "done")
self.assertIn("1 card it never merged stays where it is", self.ending())
def test_a_member_whose_run_died_is_not_swept(self):
self.write_cards(listed=THREE_LISTED)
self.write(THREE, card("33 — The landing page"))
self.start()
self.advance()
self.adapter_is(DIES)
self.advance() # 33 dies; the phase halts on it
self.advance()
self.assertEqual(self.stage_of(THREE), "in-progress")
self.complete("in-progress")
self.assertEqual(self.stage_of(THREE), "in-progress")
self.assertEqual(self.stage_of(ONE), "done")
def test_only_what_reached_the_branch_goes_however_far_the_run_got(self):
"""Stopped mid-run: 31 is merged, 32 has reached review/ but the
phase has not brought it, and 33 has not been started. Exactly one
of those three is the ending's business."""
self.write_cards(listed=THREE_LISTED)
self.write(THREE, card("33 — The landing page"))
self.start()
self.advance() # merge 31, run 32 — and stop there
self.complete("in-progress")
self.assertEqual(self.stage_of(ONE), "done", "merged, so finished")
self.assertEqual(self.stage_of(TWO), "review",
"in review/ but never merged into the phase branch")
self.assertTrue(self.branch_exists(f"task/{TWO[:-3]}"))
self.assertEqual(self.stage_of(THREE), "backlog", "never started")
# — abort together —
def test_a_merge_conflict_moves_nothing_at_all(self):
self.run_phase()
for where, text in ((self.repo, "main's own line\n"),
(config.WORKTREES / PHASE[:-3], "the phase's line\n")):
(where / "contested.txt").write_text(text, encoding="utf-8")
git(where, "add", "-A")
git(where, "commit", "-q", "-m", "contested")
with self.assertRaises(ValueError) as caught:
self.complete()
self.assertIn("merge conflict", str(caught.exception))
self.assertEqual(self.stage_of(PHASE), "review", "the phase card stays")
self.assertEqual(self.stage_of(ONE), "review", "and so does every member")
self.assertEqual(self.stage_of(TWO), "review")
self.assertTrue(self.branch_exists(f"task/{ONE[:-3]}"))
self.assertIn(str(config.WORKTREES / ONE[:-3]), self.worktrees())
def test_the_sweep_waits_for_the_merge_rather_than_racing_it(self):
"""Ordering, not luck: the cards move after the merge has landed,
so a merge that fails half way finds nothing already swept."""
self.run_phase()
seen = {}
real = github._merge_locally
self.addCleanup(setattr, github, "_merge_locally", real)
def watched(filename, branch):
seen["before"] = (self.stage_of(ONE), self.stage_of(TWO))
real(filename, branch)
github._merge_locally = watched
self.complete()
self.assertEqual(seen["before"], ("review", "review"))
# — the other endings do not sweep —
def test_just_moving_the_card_moves_no_member(self):
self.run_phase()
taskfiles.move_task(PHASE, "review", "done")
self.assertEqual(self.stage_of(PHASE), "done")
self.assertEqual(self.stage_of(ONE), "review")
self.assertEqual(self.stage_of(TWO), "review")
self.assertTrue(self.branch_exists(f"task/{ONE[:-3]}"))
def test_archiving_the_phase_card_marks_nothing_done(self):
"""Archiving releases the members to the board in whatever stage
they are genuinely in which is not done/, because nothing was
merged into main."""
self.run_phase()
taskfiles.move_task(PHASE, "review", "to-do")
taskfiles.archive_task(PHASE, "to-do")
self.assertTrue((self.tasks / "archive" / PHASE).is_file())
self.assertEqual(self.stage_of(ONE), "review")
self.assertEqual(self.stage_of(TWO), "review")
self.assertTrue(self.branch_exists(f"task/{TWO[:-3]}"))
self.assertIn(str(config.WORKTREES / TWO[:-3]), self.worktrees())
# — the edges —
def test_a_member_already_moved_to_done_by_hand_is_no_trouble(self):
self.run_phase()
taskfiles.move_task(ONE, "review", "done")
result = self.complete()
self.assertEqual(self.stage_of(ONE), "done")
self.assertEqual(self.stage_of(TWO), "done")
self.assertEqual(result["swept"], [TWO],
"a card already there is not moved twice")
self.assertFalse(self.branch_exists(f"task/{ONE[:-3]}"),
"it was still cleaned up: its work is in main too")
def test_a_dirty_worktree_is_reported_and_kept_not_forced(self):
self.run_phase()
(config.WORKTREES / TWO[:-3] / "notes.txt").write_text(
"something a person was in the middle of\n", encoding="utf-8")
state.BOARD_EVENTS.clear()
self.complete()
self.assertTrue((config.WORKTREES / TWO[:-3] / "notes.txt").is_file(),
"uncommitted work is never thrown away")
self.assertTrue(self.branch_exists(f"task/{TWO[:-3]}"),
"and its branch stays with it — it is still checked out")
self.assertIn(f"{TWO[:-3]}: worktree kept — 1 uncommitted change in it "
f"— nothing uncommitted is thrown away", self.summaries(),
"the ticker says which one and why")
self.assertEqual(self.stage_of(TWO), "done",
"the card is still finished: its work is in main")
self.assertFalse((config.WORKTREES / ONE[:-3]).exists(),
"the clean one still went")
def test_a_phase_that_merged_nothing_says_so_and_moves_nobody(self):
"""The card is still the person's to drag to done/ — what it must
not do is claim an ending for work that never landed."""
self.write_cards(listed=THREE_LISTED)
self.write(THREE, card("33 — The landing page"))
self.start() # 31 runs, but nothing is merged yet
state.BOARD_EVENTS.clear()
result = self.complete("in-progress")
self.assertEqual(result["swept"], [])
self.assertEqual(self.stage_of(ONE), "review")
self.assertIn("it had merged nothing, so no card went with it",
self.ending())
self.assertIn("3 cards it never merged stay where they are", self.ending())
def test_a_card_with_no_members_completes_as_it_always_did(self):
"""An ordinary card never reaches any of this."""
self.write(ONE, card("31 — Stand up site/", status="Review"), "review")
(self.tasks / "backlog" / ONE).unlink()
git(self.repo, "worktree", "add", "-q", "-b", f"task/{ONE[:-3]}",
str(config.WORKTREES / ONE[:-3]))
(config.WORKTREES / ONE[:-3] / "work.txt").write_text("x\n", encoding="utf-8")
git(config.WORKTREES / ONE[:-3], "add", "-A")
git(config.WORKTREES / ONE[:-3], "commit", "-q", "-m", "work")
result = github.complete_task(ONE, "review")
self.assertEqual(result, {"merged": True, "swept": []})
self.assertEqual(self.stage_of(ONE), "done")
class TheSweepReachesGit(PhaseEnding):
"""With `BOARD_COMMIT_MOVES` on: a sweep that never left one working
tree is not a sweep."""
def setUp(self):
super().setUp()
self.patch(COMMIT_MOVES=True)
def commits(self) -> list[str]:
return git(self.repo, "log", "--format=%s", "main").stdout.splitlines()
def test_the_sweep_is_one_commit_that_names_what_moved(self):
self.run_phase()
self.complete()
sweep = [s for s in self.commits() if s.startswith("board: ") and "→ done" in s]
self.assertIn("board: 31, 32 → done with phase 40 (tester)", sweep)
self.assertNotIn("board: 31 → done (tester)", sweep,
"five cards are not five lines in the log")
self.assertIn("board: 40 → done (tester)", sweep,
"the card the person dragged still moves as itself")
def test_the_swept_files_are_committed_where_they_landed(self):
self.run_phase()
self.complete()
self.assertEqual(git(self.repo, "status", "--porcelain").stdout.strip(), "",
"nothing is left modified for a human to notice later")
listed = git(self.repo, "ls-files", "tasks/done").stdout.split()
for name in (PHASE, ONE, TWO):
self.assertIn(f"tasks/done/{name}", listed)
def test_the_commit_carries_the_prefix_sync_publishes_on(self):
"""`board: ` is what the piggyback guard looks for: a commit
without it stalls every later push."""
self.run_phase()
published: list[str] = []
self.addCleanup(state.COMMIT_HOOKS.clear)
state.COMMIT_HOOKS.append(published.append)
self.complete()
self.assertIn(ONE, published, "the sweep fires the same commit hook")
for line in self.commits():
if "→ done" in line:
self.assertTrue(line.startswith("board: "), line)
class TheRemoteIsClearedToo(PhaseEnding):
"""The third workspace a member leaves behind is on the remote."""
REMOTE = True
def remote_branches(self) -> list[str]:
out = git(self.repo, "ls-remote", "--heads", "origin").stdout
return [line.split("refs/heads/")[-1] for line in out.splitlines() if line]
def test_no_merged_member_leaves_a_branch_on_the_remote(self):
self.run_phase()
git(self.repo, "push", "-q", "origin",
f"task/{ONE[:-3]}", f"task/{TWO[:-3]}")
self.assertIn(f"task/{ONE[:-3]}", self.remote_branches())
self.complete()
self.assertEqual(self.remote_branches(), ["main"])
def test_a_member_the_phase_never_merged_keeps_its_remote_branch(self):
self.write_cards(listed=THREE_LISTED)
self.write(THREE, card("33 — The landing page"))
self.start()
self.advance()
self.advance()
taskfiles.move_task(THREE, "review", "in-progress")
git(self.repo, "push", "-q", "origin", f"task/{THREE[:-3]}")
self.complete("in-progress")
self.assertIn(f"task/{THREE[:-3]}", self.remote_branches())
class TeamModeEndsItOnOrigin(PhaseEnding):
"""With `BOARD_SYNC` on the merge is made by GitHub, so local main has
not seen it when the sweep runs. Nothing in the sweep may depend on
that: what the phase carried is answered by the phase branch, which is
still here, and the members' branches are deleted with `-D` for
exactly the reason the phase card's own is."""
REMOTE = True
def test_the_cards_and_their_workspaces_still_go(self):
self.run_phase()
git(self.repo, "push", "-q", "origin",
f"task/{ONE[:-3]}", f"task/{TWO[:-3]}")
self.patch(SYNC=True, COMMIT_MOVES=True)
result = self.complete()
self.assertTrue(any(call[:2] == ["pr", "merge"] for call in self.gh_calls()),
"the merge is origin's to make in team mode")
self.assertEqual(sorted(result["swept"]), sorted([ONE, TWO]))
self.assertEqual(self.stage_of(ONE), "done")
self.assertEqual(self.stage_of(TWO), "done")
self.assertFalse(self.branch_exists(f"task/{ONE[:-3]}"),
"a branch main has not caught up with is still gone")
remote = git(self.repo, "ls-remote", "--heads", "origin").stdout
self.assertNotIn(f"task/{ONE[:-3]}", remote)
self.assertIn("board: 31, 32 → done with phase 40 (tester)",
git(self.repo, "log", "--format=%s", "main").stdout)
class TheDocumentedEnding(unittest.TestCase):
"""AGENTS.md is the design record — the ending has to be in it."""
@classmethod
def setUpClass(cls):
cls.doc = (REPO / "AGENTS.md").read_text(encoding="utf-8")
def test_the_sweep_is_described(self):
self.assertIn("### Finishing a phase finishes its cards", self.doc)
def test_it_says_what_is_not_swept(self):
section = self.doc.split("### Finishing a phase finishes its cards")[1]
section = section.split("\n### ")[0]
for promise in ("halted", "walked back", "uncommitted", "single commit",
"Just move the card", "Archiving"):
self.assertIn(promise, section, f"the record must say: {promise}")
class TheSheetSaysSo(unittest.TestCase):
"""board.html has no frontend runner — these are the source-level
invariants of what the person is told before they click."""
@classmethod
def setUpClass(cls):
cls.html = (REPO / "manager" / "core" / "board.html").read_text(
encoding="utf-8")
def test_the_sheet_counts_the_cards_that_would_go_with_it(self):
self.assertIn("const swept = task.isPhase ? mergedIn(task).size : 0;",
self.html)
self.assertIn("card${swept === 1 ? '' : 's'} this phase merged go to done/",
self.html)
def test_just_moving_the_card_says_the_members_stay(self):
self.assertIn('and so does every card in the phase', self.html)
def test_the_toast_names_how_many_went(self):
self.assertIn("const went = (data.swept || []).length;", self.html)
if __name__ == "__main__":
unittest.main()