From 755d40dd3388dee6fc56a7cbd8e5e0085aa68752 Mon Sep 17 00:00:00 2001 From: istos Date: Thu, 30 Jul 2026 11:18:17 +0200 Subject: [PATCH] etiquette: the actor's board acts, every other replica renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 19 gave every board the same truth; this makes exactly one of them react to it. State syncs; reactions don't. - watch.py: attribution is now the trigger gate. _actor returns (who, remote), and a move a pull applied — the arrivals sync files — renders and narrates but opens no PR. A plain mv on this disk still acts: inert means "happened elsewhere", not "unattributed". - github.py: the file-carried gates behind that rule, so the rare double is harmless rather than loud. The **PR:** line commits itself in team mode (taskfiles.commit_edit, sharing the move's pathspec-scoped commit), so it reaches the other boards instead of sitting in one working tree — where it would also stall sync outright; and a `gh pr create` that races anyway adopts the PR GitHub already holds. - No board finishes the actor's half-done side effect on its own: the startup reconcile stands down in team mode and a review card with a branch and no PR carries ↑ open PR (POST /api/pr/open), which is a person deciding rather than N boards guessing. _open_pr raises its reasons now, so the automatic path narrates them and the explicit one toasts them. - agents.py: the claim gates work launches. A card someone else holds refuses, naming them; ▸ take over is the deliberate second path (armed like everything that costs tokens) and reassigns via taskfiles' set_assignee; an unheld card claims itself on launch. Only in team mode — with BOARD_COMMIT_MOVES off nothing writes an assignee, so nothing reads one as a lock. - github.complete_task: with BOARD_SYNC on, merge & clean up runs `gh pr merge` and lets the beat deliver the result, so local main only ever fast-forwards and no board makes a merge commit of its own. A branch without a PR is refused with a pointer to ↑ open PR. Sync off keeps the local merge path exactly as it was. Verified with tests/test_actor_acts.py: two real clones of a real bare upstream and a stub gh — the replica that only renders, the PR line that travels, the double that adopts, the takeover that reassigns, and both merge paths. Co-Authored-By: Claude Opus 5 --- manager/core/agents.py | 38 ++- manager/core/board.html | 50 +++- manager/core/github.py | 217 ++++++++++---- manager/core/httpd.py | 11 +- manager/core/taskfiles.py | 77 ++++- manager/core/watch.py | 37 ++- tests/test_actor_acts.py | 616 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 955 insertions(+), 91 deletions(-) create mode 100644 tests/test_actor_acts.py diff --git a/manager/core/agents.py b/manager/core/agents.py index 35840ec..198fbf8 100644 --- a/manager/core/agents.py +++ b/manager/core/agents.py @@ -21,7 +21,7 @@ from pathlib import Path import config import events import state -from taskfiles import find_stage_of, move_task, read_task +from taskfiles import actor_name, find_stage_of, move_task, read_task, set_assignee def _clean_log(text: str, cap: int = 3000) -> str: @@ -190,10 +190,44 @@ def _fresh_branch_point() -> tuple[str | None, str | None]: return "origin/main", None -def start_agent(filename: str, stage: str) -> dict: +def _claim_for_launch(filename: str, stage: str, takeover: bool) -> None: + """One agent per task is a board-memory rule; across machines the card + file is the only thing every board can see, so the claim is what gates + a launch here. + + Someone else's card refuses — naming who holds it — unless this is the + deliberate takeover, which reassigns the card to whoever asked. An + unclaimed card claims itself on launch: starting work is as much a + commitment as the move that usually writes the line. + + Only in team mode. With `BOARD_COMMIT_MOVES` off nothing writes the + assignee, so nothing may refuse on it either — the launch is exactly + what it was before. + """ + if not config.COMMIT_MOVES: + return + path = config.TASKS / stage / filename + holder = read_task(path, stage).get("assignee") + me = actor_name() + if holder and me and holder != me and not takeover: + raise ValueError(f"{holder} holds {filename} — take it over deliberately " + f"(the card's ▸ take over), or clear the Assignee line") + if not me: + return # no identity to write; git has no name here + if holder == me: + return + set_assignee(filename, stage, me) + state.record_board_event({ + "kind": "agent", "actor": "board", "file": filename, + "summary": (f"{me} took {filename} over from {holder}" if holder + else f"{me} claimed {filename} by starting work on it")}) + + +def start_agent(filename: str, stage: str, takeover: bool = False) -> dict: # Moving a card to in-progress is the commitment; only then does work start. _validate(filename, stage, {"in-progress"}, "work starts from in-progress/ — move the card there first") + _claim_for_launch(filename, stage, takeover) stem = filename[:-3] branch = f"task/{stem}" diff --git a/manager/core/board.html b/manager/core/board.html index ddcbc9e..036a787 100644 --- a/manager/core/board.html +++ b/manager/core/board.html @@ -892,9 +892,17 @@ function cardFor(task) { } } else { if (task.stage === 'in-progress') { - actions.push({ glyph: '▸', label: 'start work', confirm: 'start it?', busy: 'starting…', - title: 'A worktree, a branch, and a headless Claude on this task', - run: () => fireAgent(task, '/api/agent/start') }); + // someone else's card is never started by accident: the action says + // whose it is, and firing it is the deliberate takeover + const held = task.assignee && S.state.me && task.assignee !== S.state.me + ? task.assignee : null; + actions.push(held + ? { glyph: '▸', label: 'take over', confirm: `take from ${held}?`, busy: 'starting…', + title: `${held} holds this card — starting work takes it over and reassigns it to you`, + run: () => fireAgent(task, '/api/agent/start', { takeover: true }) } + : { glyph: '▸', label: 'start work', confirm: 'start it?', busy: 'starting…', + title: 'A worktree, a branch, and a headless Claude on this task', + run: () => fireAgent(task, '/api/agent/start') }); } else if (task.stage === 'review') { actions.push({ glyph: '↩', label: 'back', busy: 'moving…', title: 'Send it back for more work', run: () => move(task.file, 'review', 'in-progress') }); @@ -902,7 +910,17 @@ function cardFor(task) { actions.push({ glyph: '↺', label: 'reopen', busy: 'reopening…', title: 'Put it back in the queue', run: () => move(task.file, 'done', 'to-do') }); } - actions.push(stillTrue); + // work in review with no PR: no board opens one behind your back, so + // the card offers it instead of the relevance check + if (task.stage === 'review' && !task.pr + && (S.state.branches || []).includes(task.file.replace(/\.md$/, ''))) { + actions.push({ glyph: '↑', label: 'open PR', confirm: 'open it?', busy: 'opening…', + title: 'Push the branch and open its PR — the board does this when a card ' + + 'enters review, and this is how you ask for it afterwards', + run: () => openPR(task) }); + } else { + actions.push(stillTrue); + } } if (actions.length) el.classList.add('has-acts'); @@ -1142,11 +1160,11 @@ async function fireAction(btn, key, act) { return ok; } -async function fireAgent(task, url) { +async function fireAgent(task, url, extra) { toast(`starting on ${task.file}…`); const res = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ file: task.file, stage: task.stage }), + body: JSON.stringify({ file: task.file, stage: task.stage, ...(extra || {}) }), }); const data = await res.json(); if (!res.ok) { toast(data.error || 'that did not start', true); return false; } @@ -1192,6 +1210,17 @@ async function parkDrive() { loadState(); } +async function openPR(task) { + const res = await fetch('/api/pr/open', { + method: 'POST', headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ file: task.file }), + }); + const data = await res.json(); + toast(res.ok ? `PR opened for ${task.file}` : (data.error || 'the PR did not open'), !res.ok); + await loadState(); + return res.ok; +} + async function askCopilot(task) { const res = await fetch('/api/pr/copilot', { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -1244,8 +1273,13 @@ function completeSheet(task, from) { `
` + `` + `` + - `` + + // team mode merges on origin: the board never makes a merge commit of + // its own, so every replica's main keeps fast-forwarding + `` + `
`; wrap.classList.add('open'); wrap.addEventListener('click', (e) => { if (e.target === wrap) closeSheet(); }); diff --git a/manager/core/github.py b/manager/core/github.py index b3c80c9..47ab0d8 100644 --- a/manager/core/github.py +++ b/manager/core/github.py @@ -4,6 +4,13 @@ Copilot reviews, and polling PR state for cards sitting in review/. All of it is mechanical `git` + `gh` — no Claude involvement. The PR url is written into the task file (`**PR:** `), keeping the file the single source of truth; only the volatile review/check state lives in memory. + +With replicas watching one truth, *who* opens a PR matters: the trigger is +the actor's board (watch.py refuses to fire on a move a pull applied) and +the `**PR:**` line is the backstop behind it — carried by the file, so a +second attempt from anywhere finds the PR already there, and a `gh pr +create` that races anyway adopts the open PR instead of erroring. Polling +is read-only and every board does it. """ from __future__ import annotations @@ -19,7 +26,7 @@ from pathlib import Path import config import drive as drive_mod import state -from taskfiles import STATUS_RE, find_stage_of, move_task, read_task +from taskfiles import STATUS_RE, commit_edit, find_stage_of, move_task, read_task PR_STATE: dict[str, dict] = {} # filename -> {verdict, detail, url, ts} _OPENING: set[str] = set() # filenames with a PR-open in flight @@ -47,6 +54,10 @@ def _branch_exists(branch: str) -> bool: def _write_pr_line(filename: str, url: str) -> None: + """The url joins the header — and in team mode commits itself, so the + gate that stops a second board opening a second PR travels to the other + boards rather than sitting in one working tree (where it would also + stall sync, which never runs over uncommitted changes).""" stage = find_stage_of(filename) if not stage: return @@ -59,59 +70,80 @@ def _write_pr_line(filename: str, url: str) -> None: else: text = f"**PR:** {url}\n\n" + text path.write_text(text, encoding="utf-8") + commit_edit(filename, stage, "PR opened") + + +class _Quiet(ValueError): + """A reason not worth the ticker: no branch, or a PR already open. The + automatic path swallows these; the explicit action still shows them.""" def maybe_open_pr(filename: str) -> None: - """Card entered review/ — open a PR for its branch if one can be opened. + """Card entered review/ on *this* board — open a PR for its branch if + one can be opened. - Quiet when there is simply no branch (hand-written tasks); loud in the - ticker when a PR *should* be possible but something stands in the way. + Quiet when there is simply no branch (hand-written tasks) or a PR is + already on the card; loud in the ticker when a PR *should* be possible + but something stands in the way. """ if filename in _OPENING: return _OPENING.add(filename) try: _open_pr(filename) + except _Quiet: + pass + except ValueError as exc: + state.record_board_event({ + "kind": "agent", "actor": "board", "file": filename, + "summary": str(exc)}) finally: _OPENING.discard(filename) -def _open_pr(filename: str) -> None: +def open_pr_now(filename: str) -> str: + """The explicit action behind ↑ open PR: no board ever completes a + half-done side effect on its own (the actor's board may have died + between moving the card and opening the PR), so a person asks for it — + and hears the reason when it cannot happen.""" + if filename in _OPENING: + raise ValueError(f"a PR for {filename} is already being opened") + _OPENING.add(filename) + try: + return _open_pr(filename) + finally: + _OPENING.discard(filename) + + +def _open_pr(filename: str) -> str: branch = f"task/{filename[:-3]}" if not _branch_exists(branch): - return # nothing to publish — a hand-moved card without agent work + # nothing to publish — a hand-moved card without agent work + raise _Quiet(f"{filename} has no {branch} branch — nothing to open a PR from") stage = find_stage_of(filename) if stage != "review": - return + raise _Quiet(f"{filename} is not in review/ — PRs open from there") task = read_task(config.TASKS / stage / filename, stage) if task.get("pr"): - return # already open + raise _Quiet(f"{filename} already has a PR: {task['pr']}") rname = remote() if rname is None or not gh_available(): - state.record_board_event({ - "kind": "agent", "actor": "board", "file": filename, - "summary": f"no PR for {filename}: " + - ("no git remote configured" if rname is None else "gh is not installed")}) - return + raise ValueError(f"no PR for {filename}: " + + ("no git remote configured" if rname is None + else "gh is not installed")) # The PR's diff is computed against the remote main — refuse to open one # that would drag unpushed main commits along with it. _run(["git", "fetch", rname, "main"], timeout=120) ahead = _run(["git", "rev-list", "--count", f"{rname}/main..main"]).stdout.strip() if ahead.isdigit() and int(ahead) > 0: - state.record_board_event({ - "kind": "agent", "actor": "board", "file": filename, - "summary": f"won't open a PR for {filename}: main is {ahead} commits " - f"ahead of {rname} — push main first, then move the card again"}) - return + raise ValueError(f"won't open a PR for {filename}: main is {ahead} commits " + f"ahead of {rname} — push main first, then move the card again") push = _run(["git", "push", "-u", rname, branch], timeout=180) if push.returncode != 0: - state.record_board_event({ - "kind": "agent", "actor": "board", "file": filename, - "summary": f"push failed for {branch}: {push.stderr.strip()[:140]}"}) - return + raise ValueError(f"push failed for {branch}: {push.stderr.strip()[:140]}") body = (f"Task: `{filename}` — tracked in `.task-manager/tasks/review/`.\n\n" f"Opened by the board when the card moved to review.") @@ -121,10 +153,20 @@ def _open_pr(filename: str) -> None: result = _run([config.GH_BIN, "pr", "create", "--head", branch, "--base", "main", "--title", task["title"], "--body", body], timeout=120) if result.returncode != 0: + # The rare double-fire: two attempts crossed and GitHub already has + # the PR. Adopt it — one PR still exists, and the card learns its + # url. Anything else is a real failure. + adopted = _existing_pr(branch) if _already_exists(result) else "" + if not adopted: + raise ValueError(f"PR creation failed for {branch}: " + f"{result.stderr.strip()[:140]}") + _write_pr_line(filename, adopted) state.record_board_event({ "kind": "agent", "actor": "board", "file": filename, - "summary": f"PR creation failed for {branch}: {result.stderr.strip()[:140]}"}) - return + "summary": f"{filename}'s PR was already open — adopted it: {adopted}"}) + state.broadcast({"type": "board"}) + _poll_pr(filename, adopted) + return adopted url = next((l.strip() for l in result.stdout.splitlines() if "/pull/" in l), result.stdout.strip()) _write_pr_line(filename, url) state.record_board_event({ @@ -132,6 +174,20 @@ def _open_pr(filename: str) -> None: "summary": f"PR opened for {filename}: {url}"}) state.broadcast({"type": "board"}) _poll_pr(filename, url) # first CI/review snapshot without waiting a cycle + return url + + +def _already_exists(result: subprocess.CompletedProcess) -> bool: + return "already exists" in (result.stderr + result.stdout).lower() + + +def _existing_pr(branch: str) -> str: + """The url of the PR already open for this branch, if gh can name it.""" + found = _run([config.GH_BIN, "pr", "view", branch, "--json", "url", + "--jq", ".url"], timeout=60) + if found.returncode != 0: + return "" + return next((l.strip() for l in found.stdout.splitlines() if "/pull/" in l), "") def _agent_log_tail(filename: str, cap: int = 1500) -> str: @@ -326,9 +382,15 @@ def open_pr_async(filename: str) -> None: def complete_task(filename: str, stage: str) -> dict: """The user chose "merge & clean up" on a move to done: park the drive - if it is this task's, merge the branch into main, push (which marks the - PR merged), remove the worktree and branches, then move the card. - Every step narrates; a conflict aborts cleanly and the card stays.""" + if it is this task's, merge the branch, remove the worktree and + branches, then move the card. Every step narrates; a conflict aborts + cleanly and the card stays. + + Where the merge happens depends on team mode. Single-player merges into + the local main and pushes it, exactly as it always did; with + `BOARD_SYNC` on the merge is made on origin through `gh pr merge`, so + local main only ever fast-forwards to it — the discipline the whole + sync design rests on.""" if stage not in config.STAGE_DIRS or stage == "done": raise ValueError("complete runs on a live-stage card") if not (config.TASKS / stage / filename).is_file(): @@ -348,37 +410,18 @@ def complete_task(filename: str, stage: str) -> dict: merged = False if _branch_exists(branch): - current = _run(["git", "branch", "--show-current"]).stdout.strip() - if current != "main": - raise ValueError(f"the repo is on '{current}', not main — switch first") - result = _run(["git", "merge", "--no-edit", branch], timeout=120) - if result.returncode != 0: - _run(["git", "merge", "--abort"]) - detail = (result.stdout.strip() or result.stderr.strip())[-160:] - raise ValueError(f"merge conflict — resolve by hand ({detail})") + if config.SYNC: + _merge_on_origin(filename, stage, branch) + else: + _merge_locally(filename, branch) merged = True - state.record_board_event({ - "kind": "agent", "actor": "board", "file": filename, - "summary": f"merged {branch} into main"}) - - rname = remote() - if rname: - push = _run(["git", "push", rname, "main"], timeout=180) - if push.returncode != 0: - state.record_board_event({ - "kind": "agent", "actor": "board", "file": filename, - "summary": f"merged locally but the push failed — push main " - f"yourself ({push.stderr.strip()[:100]})"}) - else: - _run(["git", "push", rname, "--delete", branch], timeout=60) - state.record_board_event({ - "kind": "agent", "actor": "board", "file": filename, - "summary": f"pushed main (PR marked merged) and deleted {branch} on {rname}"}) worktree = config.WORKTREES / stem if worktree.exists(): _run(["git", "worktree", "remove", "--force", str(worktree)]) - _run(["git", "branch", "-d", branch]) + # -D under sync: main here has not merged the branch yet (origin + # did), so the safe delete would refuse something already landed. + _run(["git", "branch", "-D" if config.SYNC else "-d", branch]) PR_STATE.pop(filename, None) state.record_board_event({ "kind": "agent", "actor": "board", "file": filename, @@ -389,6 +432,65 @@ def complete_task(filename: str, stage: str) -> dict: return {"merged": merged} +def _merge_locally(filename: str, branch: str) -> None: + """Single-player: merge into the checkout's own main and push it.""" + current = _run(["git", "branch", "--show-current"]).stdout.strip() + if current != "main": + raise ValueError(f"the repo is on '{current}', not main — switch first") + result = _run(["git", "merge", "--no-edit", branch], timeout=120) + if result.returncode != 0: + _run(["git", "merge", "--abort"]) + detail = (result.stdout.strip() or result.stderr.strip())[-160:] + raise ValueError(f"merge conflict — resolve by hand ({detail})") + state.record_board_event({ + "kind": "agent", "actor": "board", "file": filename, + "summary": f"merged {branch} into main"}) + + rname = remote() + if rname: + push = _run(["git", "push", rname, "main"], timeout=180) + if push.returncode != 0: + state.record_board_event({ + "kind": "agent", "actor": "board", "file": filename, + "summary": f"merged locally but the push failed — push main " + f"yourself ({push.stderr.strip()[:100]})"}) + else: + _run(["git", "push", rname, "--delete", branch], timeout=60) + state.record_board_event({ + "kind": "agent", "actor": "board", "file": filename, + "summary": f"pushed main (PR marked merged) and deleted {branch} on {rname}"}) + + +def _merge_on_origin(filename: str, stage: str, branch: str) -> None: + """Team mode: the merge commit is made by GitHub, on origin. + + Replicas keep converging only while local main advances by + fast-forward, so the board never creates a merge commit of its own — + it asks origin for one and lets the sync beat deliver it. Needs merge + rights on the repo for whoever clicks, which the local path did not. + """ + task = read_task(config.TASKS / stage / filename, stage) + url = task.get("pr") + if not url: + raise ValueError( + f"{filename} has no PR, and with BOARD_SYNC on the merge is made on " + f"origin — open a PR for {branch} first (↑ open PR on the card)") + if not gh_available(): + raise ValueError("gh is not installed — with BOARD_SYNC on the merge runs on origin") + number = url.rstrip("/").rsplit("/", 1)[-1] + result = _run([config.GH_BIN, "pr", "merge", number, "--merge"], timeout=180) + if result.returncode != 0: + detail = (result.stderr.strip() or result.stdout.strip())[-160:] + raise ValueError(f"origin would not merge the PR — resolve it on GitHub ({detail})") + state.record_board_event({ + "kind": "agent", "actor": "board", "file": filename, + "summary": f"merged {filename}'s PR on origin — local main fast-forwards " + f"on the next sync beat"}) + rname = remote() + if rname: + _run(["git", "push", rname, "--delete", branch], timeout=60) + + def task_branches() -> list[str]: """Stems of all task/* branches — the UI uses this to say honestly whether a review card has work attached.""" @@ -400,7 +502,14 @@ def task_branches() -> list[str]: def reconcile() -> None: """Catch up on moves the watcher never saw (board was down): any card already sitting in review/ with a branch but no PR gets its PR opened - now. Runs once at startup.""" + now. Runs once at startup. + + Not in team mode. A replica cannot tell whose move it missed, so every + board starting up would race to open the same PR — and the card that + needs one wears the explicit ↑ open PR action instead, which is a + person deciding rather than N boards guessing.""" + if config.SYNC: + return time.sleep(3) # let the server settle first directory = config.TASKS / "review" if not directory.is_dir(): diff --git a/manager/core/httpd.py b/manager/core/httpd.py index 56dd2b5..0cfe1eb 100644 --- a/manager/core/httpd.py +++ b/manager/core/httpd.py @@ -40,6 +40,9 @@ def state_payload() -> dict: "commands": config.commands(), "commandRuns": commands.public(), "checks": config.checks(), + # who this board is, so a card can tell "yours" from "someone + # else's". Empty outside team mode: nothing claims anything there. + "me": taskfiles.actor_name() if config.COMMIT_MOVES else "", "archivedCount": taskfiles.archived_count(), "sync": sync.status(), "boardEvents": board_events, @@ -167,7 +170,10 @@ class Handler(BaseHTTPRequestHandler): self._json(200, {"ok": True}) elif path == "/api/agent/start": payload = self._read_body() - agent = agents.start_agent(payload["file"], payload["stage"]) + # takeover: the second, deliberate click on a card someone + # else holds — never the default a stale card face sends + agent = agents.start_agent(payload["file"], payload["stage"], + bool(payload.get("takeover"))) self._json(200, {"agent": agent}) elif path == "/api/agent/review": payload = self._read_body() @@ -181,6 +187,9 @@ class Handler(BaseHTTPRequestHandler): payload = self._read_body() agent = agents.start_pr_fix(payload["file"], payload["stage"]) self._json(200, {"agent": agent}) + elif path == "/api/pr/open": + payload = self._read_body() + self._json(200, {"url": github.open_pr_now(payload["file"])}) elif path == "/api/pr/copilot": payload = self._read_body() url = github.request_copilot(payload["file"]) diff --git a/manager/core/taskfiles.py b/manager/core/taskfiles.py index 3d42ce8..6c9e6bf 100644 --- a/manager/core/taskfiles.py +++ b/manager/core/taskfiles.py @@ -181,35 +181,82 @@ def _set_assignee(text: str, name: str) -> str: return TITLE_RE.sub(lambda m: f"{m.group(0)}\n\n**Assignee:** {name}", text, count=1) -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 touching only this task file. +def set_assignee(filename: str, stage: str, name: str) -> None: + """Write who holds a card where it stands, replacing whoever held it. - Staging is scoped to the file's own paths (`git add` then a pathspec - commit), so a developer's unrelated staged changes are neither committed - nor unstaged. Hooks are skipped: this is the board's bookkeeping, not a - code change. Anything going wrong is narrated — the card has already - moved on disk, which is the source of truth. + A move's claim never overwrites — the first claim sticks. This is the + other door: a launch claiming an unheld card, or the deliberate + takeover of someone else's. It commits like every other board edit, so + the new owner travels to the other boards. + """ + path = config.TASKS / stage / filename + text = path.read_text(encoding="utf-8") + updated = (ASSIGNEE_RE.sub(f"**Assignee:** {name}", text, count=1) + if ASSIGNEE_RE.search(text) else _set_assignee(text, name)) + if updated == text: + return + path.write_text(updated, encoding="utf-8") + commit_edit(filename, stage, f"claimed by {name}") + + +def _commit(filename: str, message: str, spec: list[str], failure: str) -> bool: + """One commit touching only this task file's paths. + + Staging is scoped to those paths (`git add` then a pathspec commit), so + a developer's unrelated staged changes are neither committed nor + unstaged. Hooks are skipped: this is the board's bookkeeping, not a code + change. Anything going wrong is narrated — what the commit records has + already happened on disk, which is the source of truth. """ - message = f"board: {number or filename[:-3]} → {target} ({who or 'board'})" try: - spec = [str(dst)] - tracked = _git("ls-files", "--", str(src)) - if tracked.returncode == 0 and tracked.stdout.strip(): - spec.insert(0, str(src)) # git knew the old path: record its removal result = _git("add", "-A", "--", *spec) if result.returncode == 0: result = _git("commit", "--no-verify", "-m", message, "--", *spec, timeout=60) if result.returncode == 0: state.task_committed(filename) # sync (when on) publishes it - return + return True lines = (result.stderr or result.stdout).strip().splitlines() detail = lines[-1] if lines else f"git exited {result.returncode}" except (subprocess.SubprocessError, OSError) as exc: detail = str(exc) state.record_board_event({ "kind": "agent", "actor": "board", "file": filename, - "summary": f"{filename} moved, but committing it failed: {detail[:140]}"}) + "summary": f"{failure}: {detail[:140]}"}) + return False + + +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.""" + spec = [str(dst)] + try: + tracked = _git("ls-files", "--", str(src)) + if tracked.returncode == 0 and tracked.stdout.strip(): + spec.insert(0, str(src)) # git knew the old path: record its removal + except (subprocess.SubprocessError, OSError): + pass + _commit(filename, f"board: {number or filename[:-3]} → {target} ({who or 'board'})", + spec, f"{filename} moved, but committing it failed") + + +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. + + Team mode's own bookkeeping, so it carries the `board: ` prefix sync's + piggyback guard looks for, and it fires the same commit hook a move + does. With `BOARD_COMMIT_MOVES` off it does nothing at all: the edit + stays in the working tree for a human to commit, exactly as before. + """ + 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"{what} ({actor_name() or 'board'})", + [str(path)], + f"{filename}: {what} recorded, but committing it failed") def move_task(filename: str, source: str, target: str, actor: str = "you") -> dict: diff --git a/manager/core/watch.py b/manager/core/watch.py index 14a3680..926f61c 100644 --- a/manager/core/watch.py +++ b/manager/core/watch.py @@ -3,6 +3,12 @@ Catches moves the HTTP API never saw — a file dragged by hand, an agent, another tool, or a pull from origin/main — and attributes them via the expectations registered in state and the arrivals registered by sync. + +Attribution is also the trigger gate. **State syncs; reactions don't**: a +move a pull applied is somebody else's action reaching this replica, so it +renders and narrates and nothing else — the side effects (opening a PR) +belong to the board whose user made the move. Every future automation hung +off a stage transition asks the same question here. """ from __future__ import annotations @@ -23,14 +29,20 @@ def _board_sig() -> dict[str, set[str]]: return sig -def _actor(filename: str, stage: str) -> str: - """Who did this. A move this board made is claimed from the - expectations; one a pull brought carries its commit author's name; a - plain mv on this disk is nobody in particular.""" +def _actor(filename: str, stage: str) -> tuple[str, bool]: + """Who did this, and whether it happened somewhere else. + + 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. + """ actor = state.claim_expected(filename, stage) if actor == "disk": - return sync.arrived_actor(filename) or "disk" - return actor + who = sync.arrived_actor(filename) + if who: + return who, True + return actor, False def narrate(prev: dict[str, set[str]], cur: dict[str, set[str]]) -> None: @@ -39,19 +51,22 @@ 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 = _actor(f, stage) + actor, remote = _actor(f, stage) state.record_board_event({ "kind": "move", "file": f, "from": prev_loc[f], "to": stage, - "actor": actor, + "actor": actor, "remote": remote, "summary": f"{f} moved {prev_loc[f]} → {stage} ({actor})", }) - if stage == "review": - # a card entering review with a work branch gets a PR + if stage == "review" and not remote: + # a card entering review with a work branch gets a PR — on + # the actor's board only, or the team gets one PR attempt + # per replica github.open_pr_async(f) elif f not in prev_loc: - actor = _actor(f, stage) + actor, remote = _actor(f, stage) state.record_board_event({ "kind": "new", "file": f, "to": stage, "actor": actor, + "remote": remote, "summary": f"{f} appeared in {stage}/" + (f" ({actor})" if actor != "disk" else ""), }) diff --git a/tests/test_actor_acts.py b/tests/test_actor_acts.py new file mode 100644 index 0000000..b7745aa --- /dev/null +++ b/tests/test_actor_acts.py @@ -0,0 +1,616 @@ +"""Replica etiquette (task 20): the actor's board acts, everyone else renders. + +State syncs; reactions don't. These cases run against real clones of a real +bare upstream, with a stub `gh` standing in for GitHub — the point of the +card is who does what to whom, so nothing is mocked except the network's +far end and the SSE fan-out. + + python3 -m unittest discover -s tests -v +""" + +from __future__ import annotations + +import json +import os +import shutil +import stat +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 github # noqa: E402 +import state # noqa: E402 +import sync # noqa: E402 +import taskfiles # noqa: E402 +import watch # noqa: E402 + +FILENAME = "20-a-shared-card.md" +STEM = FILENAME[:-3] +BRANCH = f"task/{STEM}" +PR_URL = "https://github.com/acme/widget/pull/7" + +# gh, as far as these tests are concerned: it logs every invocation and +# answers from GH_MODE. Real subprocess, real argv — only GitHub is fake. +FAKE_GH = f'''#!/usr/bin/env python3 +import json, os, sys + +args = sys.argv[1:] +with open(os.environ["GH_LOG"], "a") as fh: + fh.write(json.dumps(args) + "\\n") +mode = os.environ.get("GH_MODE", "ok") + +if args[:2] == ["pr", "create"]: + if mode == "exists": + sys.stderr.write('a pull request for branch "{BRANCH}" into branch ' + '"main" already exists: {PR_URL}\\n') + sys.exit(1) + if mode == "create-fails": + sys.stderr.write("something else went wrong\\n") + sys.exit(1) + print("{PR_URL}") +elif args[:2] == ["pr", "view"] and "--jq" in args: + print("{PR_URL}") +elif args[:2] == ["pr", "view"]: + print(json.dumps({{"reviews": [], "reviewRequests": [], + "statusCheckRollup": [], "state": "OPEN", + "mergeable": "MERGEABLE"}})) +elif args[:2] == ["pr", "merge"]: + if mode == "unmergeable": + sys.stderr.write("Pull request is not mergeable: the base branch " + "policy prohibits the merge.\\n") + sys.exit(1) + print("Merged pull request #7") +else: + sys.exit(0) +''' + + +def git(cwd: Path, *args: str) -> subprocess.CompletedProcess: + return subprocess.run(["git", "-C", str(cwd), *args], + capture_output=True, text=True) + + +def card(status: str, assignee: str | None = None, pr: str | None = None) -> str: + header = f"**Status:** {status}\n**Priority:** High\n" + if assignee: + header += f"**Assignee:** {assignee}\n" + if pr: + header += f"**PR:** {pr}\n" + return ("# 20 — A card two boards can see\n\n" + header + + "\nBody text long enough that git sees a rename rather than a\n" + "delete and an add when the file moves between stage directories.\n") + + +class Boards(unittest.TestCase): + """One bare upstream, two clones — 'ada' and 'elena'. config.REPO/TASKS + point at whichever board is acting.""" + + SYNC = True + + def setUp(self): + # resolve(): macOS tempdirs sit behind the /var → /private/var + # symlink and git reports the resolved path. + self.tmp = Path(tempfile.mkdtemp(prefix="bench-actor-")).resolve() + self.addCleanup(shutil.rmtree, self.tmp, True) + self.origin = self.tmp / "origin.git" + subprocess.run(["git", "init", "-q", "--bare", "-b", "main", str(self.origin)], + check=True, capture_output=True) + + self.ada = self._clone("ada") + for slug in config.STAGE_DIRS: + (self.ada / "tasks" / slug).mkdir(parents=True) + (self.ada / "code.txt").write_text("shipped\n", encoding="utf-8") + git(self.ada, "add", "-A") + git(self.ada, "commit", "-q", "-m", "root") + git(self.ada, "push", "-q", "origin", "main") + self.elena = self._clone("elena") + + self.gh_log = self.tmp / "gh.log" + gh = self.tmp / "gh" + gh.write_text(FAKE_GH, encoding="utf-8") + gh.chmod(gh.stat().st_mode | stat.S_IEXEC) + os.environ["GH_LOG"] = str(self.gh_log) + self.addCleanup(os.environ.pop, "GH_LOG", None) + self.addCleanup(os.environ.pop, "GH_MODE", None) + + self.patch(SYNC=self.SYNC, COMMIT_MOVES=True, FETCH_TIMEOUT=10.0, + SESSIONS_DIR=self.tmp / "sessions", GH_BIN=str(gh), + WORKTREES=self.tmp / "worktrees", + REPO=self.ada, TASKS=self.ada / "tasks") + self.use(self.ada) + + state.BOARD_EVENTS.clear() + state.EXPECTED_MOVES.clear() + state.COMMIT_HOOKS.clear() + self.addCleanup(state.COMMIT_HOOKS.clear) + sync.ARRIVED.clear() + sync._NOTES.clear() + self.addCleanup(sync._NOTES.clear) + self.addCleanup(sync.ARRIVED.clear) + github.PR_STATE.pop(FILENAME, None) + self.addCleanup(github.PR_STATE.pop, FILENAME, None) + + self.broadcasts: list[dict] = [] + self.addCleanup(setattr, state, "broadcast", state.broadcast) + state.broadcast = self.broadcasts.append + + def _clone(self, who: str) -> Path: + path = self.tmp / who + subprocess.run(["git", "clone", "-q", str(self.origin), str(path)], + check=True, capture_output=True) + git(path, "config", "user.name", who) + git(path, "config", "user.email", f"{who}@example.com") + return path + + def patch(self, **values) -> None: + for attr, value in values.items(): + self.addCleanup(setattr, config, attr, getattr(config, attr)) + setattr(config, attr, value) + + def mode(self, value: str) -> None: + os.environ["GH_MODE"] = value + + # — acting as one board or the other — + + def use(self, board: Path) -> None: + config.REPO, config.TASKS = board, board / "tasks" + + def place(self, board: Path, stage: str, text: str, *, commit: bool = True) -> None: + (board / "tasks" / stage / FILENAME).write_text(text, encoding="utf-8") + if commit: + git(board, "add", "-A") + git(board, "commit", "-q", "-m", f"board: 20 → {stage} (setup)") + git(board, "push", "-q", "origin", "main") + + def work_branch(self, board: Path) -> None: + """A worktree with a commit on it: what an agent leaves behind.""" + worktree = config.WORKTREES / STEM + git(board, "worktree", "add", "-q", "-b", BRANCH, str(worktree)) + (worktree / "feature.txt").write_text("the work\n", encoding="utf-8") + git(worktree, "add", "-A") + git(worktree, "commit", "-q", "-m", "the work") + + def stage_of(self, board: Path) -> str | None: + for slug in config.STAGE_DIRS: + if (board / "tasks" / slug / FILENAME).is_file(): + return slug + return None + + def text(self, board: Path) -> str: + return (board / "tasks" / self.stage_of(board) / FILENAME).read_text( + encoding="utf-8") + + def sig(self, board: Path) -> dict[str, set[str]]: + return {slug: {p.name for p in (board / "tasks" / slug).glob("*.md")} + for slug in config.STAGE_DIRS + if (board / "tasks" / slug).is_dir()} + + def elsewhere(self) -> None: + """Switch processes, not just directories. Two boards are two + programs: the expectations one holds in memory (I am about to move + this file) the other never saw, and only the commit reaches it. + The registries are module globals here, so say so explicitly.""" + state.EXPECTED_MOVES.clear() + sync.ARRIVED.clear() + + def gh_calls(self) -> list[list[str]]: + if not self.gh_log.is_file(): + return [] + return [json.loads(line) for line in + self.gh_log.read_text(encoding="utf-8").splitlines() if line.strip()] + + def gh_verbs(self) -> list[str]: + return [" ".join(c[:2]) for c in self.gh_calls()] + + def summaries(self) -> list[str]: + return [e["summary"] for e in state.BOARD_EVENTS] + + +class RemoteMovesAreInert(Boards): + """A move that arrived over origin renders and narrates. Nothing else.""" + + def setUp(self): + super().setUp() + self.place(self.ada, "in-progress", card("In Progress", "ada")) + self.use(self.elena) + sync.pull_now() + self.elsewhere() # the fixture's own pull is not evidence + self.use(self.ada) + self.opened: list[str] = [] + self.addCleanup(setattr, github, "open_pr_async", github.open_pr_async) + github.open_pr_async = self.opened.append + + def narrate(self, board: Path, action, *, fresh: bool = False) -> list[dict]: + self.use(board) + if fresh: + self.elsewhere() + before = self.sig(board) + action() + after = self.sig(board) + state.BOARD_EVENTS.clear() + watch.narrate(before, after) + return [e for e in state.BOARD_EVENTS if e["kind"] == "move"] + + def test_the_actors_board_opens_the_pr(self): + moves = self.narrate(self.ada, lambda: taskfiles.move_task( + FILENAME, "in-progress", "review")) + + self.assertEqual(self.opened, [FILENAME]) + self.assertEqual(moves[0]["actor"], "you") + self.assertFalse(moves[0]["remote"]) + + def test_the_same_move_arriving_at_a_replica_triggers_nothing(self): + self.use(self.ada) + taskfiles.move_task(FILENAME, "in-progress", "review") + self.assertEqual(sync.push_now(), "ok") + self.opened.clear() + + moves = self.narrate(self.elena, sync.pull_now, fresh=True) + + self.assertEqual(self.stage_of(self.elena), "review", + "the replica renders the move") + self.assertEqual(moves[0]["actor"], "ada") + self.assertTrue(moves[0]["remote"]) + self.assertEqual(self.opened, [], + "the side effect belongs to the board that acted") + + def test_a_plain_hand_move_on_this_disk_still_acts(self): + """Inert means "arrived from elsewhere", not "unattributed": a mv in + this checkout is still this board's user doing something.""" + moves = self.narrate(self.ada, lambda: shutil.move( + str(self.ada / "tasks" / "in-progress" / FILENAME), + str(self.ada / "tasks" / "review" / FILENAME))) + + self.assertEqual(moves[0]["actor"], "disk") + self.assertFalse(moves[0]["remote"]) + self.assertEqual(self.opened, [FILENAME]) + + def test_an_undone_move_coming_back_is_inert_too(self): + """The loser of a race has its file reverted by the rebase — which + the watcher sees as a move. It arrived; it acts on nothing.""" + self.use(self.ada) + taskfiles.move_task(FILENAME, "in-progress", "review") + sync.push_now() + self.use(self.elena) + self.elsewhere() + before = self.sig(self.elena) + taskfiles.move_task(FILENAME, "in-progress", "backlog") + self.assertEqual(sync.push_now(), "pulled") + self.opened.clear() + + state.BOARD_EVENTS.clear() + watch.narrate(before, self.sig(self.elena)) + + self.assertEqual(self.stage_of(self.elena), "review") + moves = [e for e in state.BOARD_EVENTS if e["kind"] == "move"] + self.assertEqual((moves[0]["actor"], moves[0]["remote"]), ("ada", True)) + self.assertEqual(self.opened, []) + + +class ThePRLineTravels(Boards): + """The `**PR:**` line is the backstop behind the actor-only trigger, so + it has to reach the other boards — which means committing it.""" + + def setUp(self): + super().setUp() + self.place(self.ada, "review", card("Review", "ada")) + self.work_branch(self.ada) + self.use(self.elena) + sync.pull_now() + self.use(self.ada) + + def test_opening_a_pr_writes_and_commits_the_url(self): + github.maybe_open_pr(FILENAME) + + self.assertIn(f"**PR:** {PR_URL}", self.text(self.ada)) + self.assertEqual(git(self.ada, "status", "--porcelain", + "--untracked-files=no").stdout, "", + "an uncommitted task file would stall sync outright") + subject = git(self.ada, "log", "-1", "--pretty=%s").stdout.strip() + self.assertEqual(subject, "board: 20 PR opened (ada)") + self.assertTrue(subject.startswith(sync.BOARD_COMMIT), + "sync's piggyback guard only publishes board commits") + + def test_the_url_reaches_the_other_board(self): + github.maybe_open_pr(FILENAME) + self.assertEqual(sync.push_now(), "ok") + + self.use(self.elena) + self.assertEqual(sync.pull_now(), "pulled") + + self.assertIn(f"**PR:** {PR_URL}", self.text(self.elena)) + self.assertEqual( + taskfiles.read_task(self.elena / "tasks" / "review" / FILENAME, + "review")["pr"], PR_URL, + "the replica's poller adopts the PR from the file, read-only") + + def test_a_second_attempt_is_a_no_op_not_a_second_pr(self): + github.maybe_open_pr(FILENAME) + state.BOARD_EVENTS.clear() + + github.maybe_open_pr(FILENAME) + + self.assertEqual(self.gh_verbs().count("pr create"), 1) + self.assertEqual([s for s in self.summaries() if "failed" in s], []) + + def test_a_double_that_crosses_on_github_adopts_the_open_pr(self): + """The rare double-fire: the file gate lost the race but GitHub + holds the line — one PR still exists, and the card learns its url.""" + self.mode("exists") + + github.maybe_open_pr(FILENAME) + + self.assertIn(f"**PR:** {PR_URL}", self.text(self.ada)) + self.assertTrue(any("adopted it" in s for s in self.summaries())) + self.assertEqual([s for s in self.summaries() if "failed" in s], []) + + def test_a_real_failure_is_still_a_failure(self): + self.mode("create-fails") + + github.maybe_open_pr(FILENAME) + + self.assertNotIn("**PR:**", self.text(self.ada)) + self.assertTrue(any("PR creation failed" in s for s in self.summaries())) + + def test_with_the_gate_off_the_line_is_left_for_a_human(self): + self.patch(SYNC=False, COMMIT_MOVES=False) + head = git(self.ada, "rev-parse", "HEAD").stdout + + github.maybe_open_pr(FILENAME) + + self.assertIn(f"**PR:** {PR_URL}", self.text(self.ada)) + self.assertEqual(git(self.ada, "rev-parse", "HEAD").stdout, head, + "single-player commits tasks/ by hand, as it always did") + + +class TheExplicitOpenPR(Boards): + """Nobody finishes the actor's half-done side effect automatically — + a person asks for it, and hears why when it cannot happen.""" + + def setUp(self): + super().setUp() + self.place(self.ada, "review", card("Review", "ada")) + self.use(self.ada) + + def test_it_opens_the_pr_and_returns_the_url(self): + self.work_branch(self.ada) + + self.assertEqual(github.open_pr_now(FILENAME), PR_URL) + self.assertIn(f"**PR:** {PR_URL}", self.text(self.ada)) + + def test_it_says_why_when_there_is_nothing_to_open(self): + with self.assertRaises(ValueError) as caught: + github.open_pr_now(FILENAME) + self.assertIn("no task/", str(caught.exception)) + + def test_it_says_so_when_the_card_already_has_one(self): + self.work_branch(self.ada) + github.open_pr_now(FILENAME) + + with self.assertRaises(ValueError) as caught: + github.open_pr_now(FILENAME) + self.assertIn("already has a PR", str(caught.exception)) + + def test_startup_reconcile_stands_down_in_team_mode(self): + """Every replica would race to open the same PR at startup.""" + self.work_branch(self.ada) + + github.reconcile() + + self.assertEqual(self.gh_verbs(), []) + self.assertNotIn("**PR:**", self.text(self.ada)) + + +class ClaimsGateLaunches(Boards): + """Ownership means something: work does not start on someone else's + card by accident.""" + + def setUp(self): + super().setUp() + self.place(self.ada, "in-progress", card("In Progress", "ada")) + self.use(self.elena) + sync.pull_now() + self.use(self.elena) # elena's board is the one clicking + + def test_someone_elses_card_refuses_and_names_them(self): + with self.assertRaises(ValueError) as caught: + agents.start_agent(FILENAME, "in-progress") + + self.assertIn("ada holds", str(caught.exception)) + self.assertFalse((config.WORKTREES / STEM).exists(), + "the refusal comes before any worktree is made") + self.assertEqual(git(self.elena, "rev-parse", "--verify", "--quiet", + BRANCH).returncode, 1) + + def test_the_deliberate_takeover_reassigns_the_card(self): + agents._claim_for_launch(FILENAME, "in-progress", True) + + self.assertIn("**Assignee:** elena", self.text(self.elena)) + self.assertEqual(self.text(self.elena).count("**Assignee:**"), 1) + self.assertEqual(git(self.elena, "log", "-1", "--pretty=%s").stdout.strip(), + "board: 20 claimed by elena (elena)") + self.assertTrue(any("took" in s and "over from ada" in s + for s in self.summaries())) + + def test_the_takeover_reaches_the_other_board(self): + agents._claim_for_launch(FILENAME, "in-progress", True) + self.assertEqual(sync.push_now(), "ok") + + self.use(self.ada) + self.assertEqual(sync.pull_now(), "pulled") + self.assertIn("**Assignee:** elena", self.text(self.ada)) + + def test_an_unclaimed_card_claims_on_launch(self): + self.place(self.elena, "in-progress", card("In Progress"), commit=False) + + agents._claim_for_launch(FILENAME, "in-progress", False) + + self.assertIn("**Assignee:** elena", self.text(self.elena)) + self.assertTrue(any("claimed" in s for s in self.summaries())) + + def test_your_own_card_launches_untouched(self): + self.place(self.elena, "in-progress", card("In Progress", "elena"), + commit=False) + head = git(self.elena, "rev-parse", "HEAD").stdout + + agents._claim_for_launch(FILENAME, "in-progress", False) + + self.assertEqual(git(self.elena, "rev-parse", "HEAD").stdout, head, + "nothing to record: it was already yours") + + def test_the_gate_off_refuses_nobody(self): + """Single-player never writes an assignee, so it never reads one as + a lock — a hand-written line stays decoration.""" + self.patch(SYNC=False, COMMIT_MOVES=False) + + agents._claim_for_launch(FILENAME, "in-progress", False) + + self.assertIn("**Assignee:** ada", self.text(self.elena)) + + +class MergesGoThroughOrigin(Boards): + """With replicas, local main advances only by fast-forward — so the + merge commit is made on origin, not here.""" + + def setUp(self): + super().setUp() + self.place(self.ada, "review", card("Review", "ada", PR_URL)) + self.work_branch(self.ada) + self.use(self.ada) + git(self.ada, "push", "-q", "-u", "origin", BRANCH) + + def merges_on_main(self, board: Path) -> int: + out = git(board, "rev-list", "--count", "--merges", "main").stdout.strip() + return int(out) if out.isdigit() else 0 + + def test_the_merge_is_made_on_origin(self): + head = git(self.ada, "rev-parse", "main").stdout.strip() + + result = github.complete_task(FILENAME, "review") + + self.assertTrue(result["merged"]) + self.assertIn("pr merge", self.gh_verbs()) + self.assertEqual(self.merges_on_main(self.ada), 0, + "the board never makes a merge commit of its own") + self.assertEqual( + git(self.ada, "rev-list", "--count", f"{head}..main").stdout.strip(), + "1", "only the card's own move commit — main is otherwise untouched") + self.assertEqual(self.stage_of(self.ada), "done") + self.assertTrue(any("merged" in s and "on origin" in s + for s in self.summaries())) + + def test_the_worktree_and_local_branch_go(self): + github.complete_task(FILENAME, "review") + + self.assertFalse((config.WORKTREES / STEM).exists()) + self.assertEqual(git(self.ada, "rev-parse", "--verify", "--quiet", + BRANCH).returncode, 1, + "the branch is deleted even though local main never " + "merged it — origin did") + + def test_a_pr_origin_will_not_merge_leaves_the_card_alone(self): + self.mode("unmergeable") + + with self.assertRaises(ValueError) as caught: + github.complete_task(FILENAME, "review") + + self.assertIn("origin would not merge", str(caught.exception)) + self.assertEqual(self.stage_of(self.ada), "review") + self.assertEqual(git(self.ada, "rev-parse", "--verify", "--quiet", + BRANCH).returncode, 0) + + def test_no_pr_is_a_refusal_that_names_the_way_out(self): + self.place(self.ada, "review", card("Review", "ada"), commit=False) + + with self.assertRaises(ValueError) as caught: + github.complete_task(FILENAME, "review") + + self.assertIn("open PR", str(caught.exception)) + self.assertEqual(self.stage_of(self.ada), "review") + + +class TheLocalMergeIsUntouched(Boards): + """With sync off, merge & clean up is the path it always was.""" + + SYNC = False + + def setUp(self): + super().setUp() + self.patch(COMMIT_MOVES=False) + self.place(self.ada, "review", card("Review", None, PR_URL)) + self.work_branch(self.ada) + self.use(self.ada) + + def test_it_merges_locally_and_pushes_main(self): + github.complete_task(FILENAME, "review") + + self.assertIn("the work", git(self.ada, "log", "--format=%s", "main").stdout) + self.assertEqual(self.gh_verbs(), [], "gh is not part of this path") + self.assertEqual(git(self.origin, "rev-parse", "main").stdout, + git(self.ada, "rev-parse", "main").stdout) + self.assertEqual(self.stage_of(self.ada), "done") + self.assertTrue(any("merged" in s and "into main" in s + for s in self.summaries())) + + def test_startup_reconcile_still_catches_up(self): + """The single-player behaviour the team-mode stand-down replaces.""" + self.place(self.ada, "review", card("Review"), commit=False) + + github.reconcile() + + self.assertIn(f"**PR:** {PR_URL}", self.text(self.ada)) + + +class TheCardFace(unittest.TestCase): + """board.html is a single file with no frontend runner — these are the + source-level invariants of the surface this card adds.""" + + @classmethod + def setUpClass(cls): + cls.html = (REPO / "manager" / "core" / "board.html").read_text(encoding="utf-8") + cls.httpd = (REPO / "manager" / "core" / "httpd.py").read_text(encoding="utf-8") + + def test_the_board_knows_who_it_is(self): + self.assertIn('"me": taskfiles.actor_name() if config.COMMIT_MOVES else ""', + self.httpd) + + def test_someone_elses_card_offers_takeover_not_start_work(self): + self.assertIn("const held = task.assignee && S.state.me && task.assignee !== S.state.me", + self.html) + self.assertIn("label: 'take over', confirm: `take from ${held}?`", self.html) + self.assertIn("fireAgent(task, '/api/agent/start', { takeover: true })", self.html) + + def test_the_takeover_is_armed_like_every_costly_action(self): + """confirm: … is what makes wireAction demand a second click.""" + index = self.html.index("label: 'take over'") + self.assertIn("confirm:", self.html[index:index + 120]) + + def test_the_server_refuses_a_takeover_it_was_not_asked_for(self): + self.assertIn("bool(payload.get(\"takeover\"))", self.httpd) + + def test_a_review_card_without_a_pr_can_ask_for_one(self): + self.assertIn("label: 'open PR', confirm: 'open it?', busy: 'opening…'", self.html) + self.assertIn("run: () => openPR(task)", self.html) + self.assertIn('/api/pr/open', self.html) + self.assertIn('elif path == "/api/pr/open":', self.httpd) + + def test_the_open_pr_action_needs_a_branch_and_no_pr(self): + index = self.html.index("label: 'open PR'") + guard = self.html[self.html.index("task.stage === 'review' && !task.pr"):index] + self.assertIn("S.state.branches", guard) + + def test_the_completion_sheet_tells_the_truth_in_team_mode(self): + self.assertIn("(S.state.sync || {}).enabled", self.html) + self.assertIn("the PR on GitHub", self.html) + self.assertIn("local main fast-forwards on the next sync beat", self.html) + + +if __name__ == "__main__": + unittest.main()