diff --git a/manager/core/github.py b/manager/core/github.py index 26f7fdf..b17d105 100644 --- a/manager/core/github.py +++ b/manager/core/github.py @@ -74,17 +74,33 @@ def _woven(filename: str, stage: str) -> dict: return read_task(config.TASKS / stage / filename, stage) +def _remote_has_branch(rname: str, branch: str) -> bool: + """Whether the remote already carries this branch. Sync only ever + fetches origin/main, so a phase branch the running board pushed is not a + local ref on any other board even when it is published — asking the + remote directly is the only way another board sees it.""" + return bool(_run(["git", "ls-remote", "--heads", rname, branch], + timeout=30).stdout.strip()) + + def _pr_base(task: dict) -> str: """What a PR is opened against. A phase member's branch was cut from its phase's branch, so that is the only base whose diff is the member's own work — a PR into main would carry the whole phase, and invite a merge into main that this board exists not to make. Everything else, - the phase card included, goes into main.""" + the phase card included, goes into main. + + The phase branch is the board's own and is published as the phase runs, + so a board that did not run the phase may know it only through the + remote — that still makes it the base, not main.""" phase = task.get("phase") if phase: branch = f"phase/{phase['file'][:-3]}" if _branch_exists(branch): return branch + rname = remote() + if rname and _remote_has_branch(rname, branch): + return branch return "main" @@ -195,9 +211,12 @@ def _open_pr(filename: str) -> str: if ahead.isdigit() and int(ahead) > 0: 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") - else: + elif _branch_exists(base): # A phase branch is the board's own: publish it so the member's PR - # has a base on the remote to be opened against. + # has a base on the remote to be opened against. When only the + # remote carries it — a member PR opened from a board that did not + # run the phase — it is already there, and there is nothing local + # to push. _run(["git", "push", "-u", rname, base], timeout=180) push = _run(["git", "push", "-u", rname, branch], timeout=180) diff --git a/manager/core/phases.py b/manager/core/phases.py index e92b611..58213ea 100644 --- a/manager/core/phases.py +++ b/manager/core/phases.py @@ -114,15 +114,30 @@ def log_entries(text: str) -> list[str]: return entries -def _record(phase: dict, entry: str) -> None: +def _write_log(phase: dict, entry: str) -> bool: """One line into the log, where it reaches git like every other write the board makes to a task file. The stage is asked of the disk: a pass that finishes a phase moves the card, and the record is written to - wherever the card actually is.""" + wherever the card actually is. Returns whether the line landed.""" stage = taskfiles.find_stage_of(phase["file"]) or phase["stage"] stamp = time.strftime("%Y-%m-%d %H:%M") - taskfiles.append_to_section(phase["file"], stage, LOG_HEADING, - f"- {stamp} · {entry}", "phase log") + return taskfiles.append_to_section(phase["file"], stage, LOG_HEADING, + f"- {stamp} · {entry}", "phase log") + + +def _record(phase: dict, entry: str) -> None: + """Record a decision, and refuse to act if it cannot be written down. + + The log is the durable memory a restart reads to tell "this member has + already started" from "the phase has not reached it yet". An action that + outran its own record — a launch or a merge with no line behind it — + is exactly what a restarted board would repeat. So a log that will not + take the line is itself a halt, raised here before the action it was + meant to precede ever happens. The halt path writes best-effort + (`_write_log`) so that recording the halt can never raise in turn.""" + if not _write_log(phase, entry): + raise _Halt("could not write the phase log — refusing to act without " + "the record a restart reads") def _this_run(entries: list[str]) -> list[str]: @@ -354,11 +369,20 @@ def _freshen(phase: dict) -> None: def _mine(phase: dict) -> bool: """State syncs; reactions don't. Only the board whose user holds the phase card advances it — every replica renders the same phase and - launches nothing. Outside team mode there is one board, and it acts.""" + launches nothing. Outside team mode there is one board, and it acts. + + No local git identity is the one case that does not gate: it is exactly + where `agents.claim_for_launch` cannot write an assignee and so cannot + refuse a launch either. Gating the beat on it while the launch went + through would strand a phase — branch cut, run recorded — that then + never advances. So a board with no name is the lone actor here, the same + as it is for starting work.""" if not config.COMMIT_MOVES: return True me = taskfiles.actor_name() - return bool(me) and phase.get("assignee") == me + if not me: + return True + return phase.get("assignee") == me def _unfinished_dependencies(member: dict, snapshot: dict, @@ -428,7 +452,7 @@ def _halt(phase: dict, member: dict | None, reason: str) -> None: """Stop, and say so once. The log holds the halt from here on, so the next pass reads it rather than saying the same thing again.""" at = f" at {member['number']}" if member and member["number"] else "" - _record(phase, f"halted{at} — {reason}") + _write_log(phase, f"halted{at} — {reason}") # best effort: never re-raise _say(phase["file"], f"{phase['file']} halted{at} — {reason}") @@ -570,7 +594,9 @@ def _start(phase: dict, filename: str) -> dict: if result.returncode != 0: raise ValueError(f"could not cut {branch}: {result.stderr.strip()[:200]}") _push_phase(phase) - _record(phase, f"run started on {branch}") + if not _write_log(phase, f"run started on {branch}"): + raise ValueError(f"could not record the run on {filename} — its phase " + f"log must be writable to run the phase safely") _say(filename, f"phase {filename} is running on {branch}" + (f" — {note}" if note else "")) state.broadcast({"type": "board"}) diff --git a/tests/test_phase_runs.py b/tests/test_phase_runs.py index 749a080..2d803b4 100644 --- a/tests/test_phase_runs.py +++ b/tests/test_phase_runs.py @@ -651,6 +651,29 @@ class HandMovesDoNotDoubleLaunch(PhaseCase): self.assertEqual(self.stage_of(PHASE), "review") +class AnUnwritableLogHalts(PhaseCase): + """The log is the durable memory a restart reads; a launch or a merge + with no line behind it is exactly what a restarted board would repeat. + So a log that will not take the line halts before the action it was + meant to precede.""" + + def test_a_launch_never_outruns_its_record(self): + real = phases._write_log + self.addCleanup(setattr, phases, "_write_log", real) + # the one line that will not write is the member-started record + phases._write_log = lambda phase, entry: ( + False if entry == "31 started" else real(phase, entry)) + + self.start() + + self.assertEqual( + [r for r in state.AGENTS.values() if r["task"] == ONE], [], + "the member was never launched") + self.assertNotIn("31 started", self.log()) + self.assertTrue(any("halted" in s for s in self.summaries()), + "the phase halts instead of launching blind") + + class OneBoardRunsIt(PhaseCase): """State syncs; reactions don't — the assignee is where "who runs it" is written down, and a replica advances nothing.""" @@ -683,6 +706,22 @@ class OneBoardRunsIt(PhaseCase): self.assertEqual([m["state"] for m in snapshot["members"]], ["pending", "pending"], "but it still renders the phase") + def test_a_board_with_no_git_identity_is_not_gated_out(self): + """Team mode with no local git name is the one case that does not + gate: `claim_for_launch` cannot write an assignee there and so + cannot refuse a launch, and the beat must match it — else a phase + starts (branch cut, run recorded) and then advances nowhere.""" + self.addCleanup(setattr, taskfiles, "actor_name", taskfiles.actor_name) + taskfiles.actor_name = lambda: "" + + self.assertTrue(phases._mine({"file": PHASE, "assignee": "elena"})) + + def test_a_named_board_that_is_not_the_assignee_is_gated_out(self): + self.addCleanup(setattr, taskfiles, "actor_name", taskfiles.actor_name) + taskfiles.actor_name = lambda: "ronald" + + self.assertFalse(phases._mine({"file": PHASE, "assignee": "elena"})) + def test_starting_someone_elses_phase_refuses_and_names_them(self): self.write(PHASE, card("40 — Ship the site", status="In Progress", kind="Phase", @@ -745,6 +784,15 @@ class ThePhasePR(PhaseCase): git(self.repo, "rev-parse", f"origin/{PHASE_BRANCH}").returncode, 0, "a member's PR needs its base on the remote") + def test_a_member_pr_bases_on_a_phase_branch_only_the_remote_carries(self): + """A board that did not run the phase knows the phase branch only + through the remote — sync fetches origin/main and nothing else. That + branch is still the base a member's PR opens against, never main.""" + git(self.repo, "push", "-q", "origin", f"main:refs/heads/{PHASE_BRANCH}") + + self.assertFalse(self.branch_exists(PHASE_BRANCH), "no local phase branch") + self.assertEqual(github._pr_base({"phase": {"file": PHASE}}), PHASE_BRANCH) + class TheCardsBranchIsFound(PhaseCase): def test_a_phase_card_wears_its_own_branch(self):