fix(owner): Unblock clears PR verdicts; Approve plan merges the plan PR

README.md

@@ -186,7 +186,16 @@ Every `pollSeconds` (default 300) the daemon wakes and runs one iteration:
    most failures are flakes, and a rerun costs runner time and no model tokens. The expensive half
    is in the pick — a red PR whose rerun is spent gets a builder **fix** round, ahead of review and
    validation, bounded by `limits.fixRounds` like any other, and `blocked` after that.
-5. **Pick.** Candidates are issues with Status Ready, label `agent-ready`, a `Parent epic: #N`
+5. **Approved plans.** For every open epic labelled `plan-approved` whose plan is not applied yet
+   (no `plan applied by foreman@<host>` comment), the foreman looks for its open plan PR, titled
+   `docs(plan): phase NN plan (#<epic>)`. Green checks, or none at all: it squash-merges the PR
+   and applies the plan in the same tick. Checks still running: it waits for the next tick. Red
+   checks or a conflict with the default branch: it comments
+   `plan PR #<pr> cannot merge at <sha>: <reason>. Fix it or merge it by hand (foreman@<host>)` on
+   the epic once per head commit, and the epic shows on **Needs you** until the PR is fixed or
+   merged. With no plan PR open, it applies the plan straight away. None of this starts a session,
+   so it runs ahead of the pick and an approval never waits behind a review.
+6. **Pick.** Candidates are issues with Status Ready, label `agent-ready`, a `Parent epic: #N`
    line in the body, no open claim, every "Depends on" issue closed, and a phase epic that is not
    `foreman:pause`d. Open PRs needing review or validation are candidates too, and so is an
    approved PR whose branch GitHub reports as `CONFLICTING`: that gets a **rebase** round — a
@@ -198,12 +207,12 @@ Every `pollSeconds` (default 300) the daemon wakes and runs one iteration:
    or more open `blocked` tasks starts **no new builds** until the owner unblocks one — review,
    fix, validate and rebase jobs still run there — because every blocked task is owner work and
    stacking more branches on the same files only breeds conflicts.
-6. **Claim.** The foreman comments `claimed by <host> at <iso> role=<role> round=<n>`, assigns
+7. **Claim.** The foreman comments `claimed by <host> at <iso> role=<role> round=<n>`, assigns
    itself, and (for a first-round build) sets Status In Progress. If another host's claim comment
    landed in the same window, the alphabetically first host keeps it; the others comment `released
    by <host>: conflict` and unassign. The ledger — issue comments, not the GitHub assignee — is
    authoritative, so two Macs can share one GitHub login.
-7. **Dispatch.** Creates or reuses a git worktree at `<workDir>/<issue>` — by default
+8. **Dispatch.** Creates or reuses a git worktree at `<workDir>/<issue>` — by default
    `<repoDir>/.worktrees/<issue>`, so worktrees stay inside the clone — on branch
    `feat/<issue>-<slug>`, runs the repository's `setup` command, composes the role prompt and the
    headless settings from the worktree's `.foreman/` (§5), runs the `session-env` and
@@ -212,23 +221,23 @@ Every `pollSeconds` (default 300) the daemon wakes and runs one iteration:
    `effort:<level>` label, else `high`) and a `wallClockMinutes` timeout.
    It posts `session <id> on <host> role=<role> attempt=<n>` before each attempt and runs
    `after-session` when the child ends, however it ends.
-8. **Outcome.** Parses the session's structured JSON outcome —
+9. **Outcome.** Parses the session's structured JSON outcome —
    `{"outcome": ..., "pr": <number|null>, "notes": "..."}` — and applies the labels and Status in
    the table below. On a validator `passed`/`failed` it first copies
    `<worktree>/.validation-artifacts/<pr>/` to `~/.foreman/<name>/artifacts/<pr>/`, because a
    headless session cannot write outside its worktree and the worktree is removed at merge. A
    session that ends without a valid outcome is retried up to `limits.attempts` (3) times; if all
    fail, the issue is labelled `blocked` with a log excerpt and unassigned.
-9. **Phase check.** If every task under a phase epic is closed, the phase-closer is dispatched.
+10. **Phase check.** If every task under a phase epic is closed, the phase-closer is dispatched.
    Then the planner, which is gated three ways: the epic must carry **`agent-ready`** (the owner's
    go-ahead — without it the foreman never plans anything), no approved phase may still be
    building, and no other epic's drafted plan may be waiting on the owner. When it dispatches, the
    epic is assigned and set In Progress so the hour-long session is visible on the board;
    `plan_drafted` then sets the epic In Review, adds `needs-owner`, removes `agent-ready` and
-   unassigns. Labelling the epic `plan-approved` applies the plan, clears `needs-owner` and puts
-   the epic back In Progress. Re-adding `agent-ready` asks for a re-plan. Applying a plan reads
-   `<plans.dir>/<date>-phase-NN-plan.issues.json` from the default branch on `origin`, so merging
-   the plan PR is enough — the clone never has to be pulled by hand. If the file is not there yet,
+   unassigns. Labelling the epic `plan-approved` merges the plan PR and applies the plan (step 5),
+   clears `needs-owner` and puts the epic back In Progress. Re-adding `agent-ready` asks for a
+   re-plan. Applying a plan reads `<plans.dir>/<date>-phase-NN-plan.issues.json` from the default
+   branch on `origin`, so the clone never has to be pulled by hand. If the file is not there yet,
    the foreman logs and retries next tick rather than recording the plan as applied.
 
 These run in order and stop after the first action that would start a `claude -p` session, so at
@@ -260,7 +269,7 @@ plan on retry instead of duplicating it.
 | (foreman) red CI, rerun budget unspent | rerun the failed jobs, `ci rerun by foreman@<host> for <sha>` (no session) |
 | (foreman) red CI, rerun spent | builder fix round; `blocked` once `limits.fixRounds` are gone |
 | phase-closer `phase_closed` | epic → In Review, `needs-owner` |
-| planner `plan_drafted` | epic → In Review, `needs-owner`, `agent-ready` removed, unassigned; nothing else until `plan-approved`, which applies the plan (tasks `agent-ready` + Ready), clears `needs-owner` and returns the epic to In Progress |
+| planner `plan_drafted` | epic → In Review, `needs-owner`, `agent-ready` removed, unassigned; nothing else until `plan-approved`, which merges the plan PR, applies the plan (tasks `agent-ready` + Ready), clears `needs-owner` and returns the epic to In Progress |
 
 ### Labels
 
@@ -275,7 +284,7 @@ Fixed names; `foreman init` creates them all.
 | `blocked` | Cannot proceed; the reason is a comment. Owner work. |
 | `needs-owner` | Waiting on the owner. |
 | `decision` | A question a role session could not answer. |
-| `plan-approved` | The owner approved the drafted plan; the next tick applies it. |
+| `plan-approved` | The owner approved the drafted plan; the next tick merges the plan PR and applies it. |
 | `signed-off` | The owner signed the phase off. |
 | `foreman:pause` | On an epic: no new claims under that phase, on any Mac. |
 | `reviewer:approved` / `reviewer:changes` | The reviewer's verdict, on the PR. |
@@ -330,6 +339,7 @@ reclaimed from <host> by <host> at <iso>
 merged by foreman@<host>
 ci rerun by foreman@<host> for <sha>
 unblocked by owner via foreman@<host>
+plan PR #<pr> cannot merge at <sha>: <reason>. Fix it or merge it by hand (foreman@<host>)
 ```
 
 `session … finished`, `released by`, `reclaimed from` and `merged by` are terminal: they close an
@@ -684,7 +694,8 @@ All of these work from any terminal on the Mac. Inside a configured `repoDir` yo
     bookkeeping), the running session, the in-flight pipeline (build → review → validate → CI →
     merge per issue) and the live event feed.
   - **Needs you** — every decision only the owner can make, grouped by the action: approve a
-    plan, sign off a phase, ready to plan, unblock, answer or close. The sidebar badge counts them,
+    plan, fix a plan PR that cannot merge, sign off a phase, ready to plan, unblock, answer or
+    close. The sidebar badge counts them,
     Ready to plan excepted.
   - **Waiting on** — what the foreman is waiting for by itself: CI, review rounds, dependencies,
     another Mac's claim, the phase gate, and setup problems to fix on GitHub.
@@ -750,9 +761,10 @@ All of these work from any terminal on the Mac. Inside a configured `repoDir` yo
   labels, so `gh issue edit <epic> --add-label <label>` is identical.
 - **Issues waiting on you:** the **Needs you** tab lists every open non-epic issue labelled
   `needs-owner`, `decision` or `blocked`, oldest first, with how long each has waited. `foreman status` prints the same list.
-  A `blocked` row has an **Unblock** button: it removes `blocked`, sets Status **Ready** and
-  comments `unblocked by owner via foreman@<host>` — fix whatever caused the block first, because
-  nothing else about the issue changes.
+  A `blocked` row has an **Unblock** button: it removes `blocked`, removes `reviewer:changes` and
+  `validator:failed` from the issue's PR, sets Status **Ready** and comments
+  `unblocked by owner via foreman@<host>` — fix whatever caused the block first, because nothing
+  else about the issue changes.
 - **Stop the launchd agent entirely:** `launchctl bootout gui/$(id -u)/com.apptreesoftware.foreman.widgets`,
   or `foreman -p widgets launchd uninstall`.
 
@@ -874,8 +886,8 @@ branch.
   (round 2 and later may block only on prior findings still open, regressions in the delta, or a
   real safety issue; anything else becomes a follow-up issue), and a merge conflict is never a
   change request — an approved PR that conflicts gets a rebase round that does not count.
-- **To unblock a task by hand:** fix or rebase the branch, push, remove `reviewer:changes` from the
-  PR and `blocked` from the issue (or use the page's **Unblock** button). The fix-round counter
+- **To unblock a task by hand:** fix or rebase the branch, push, remove `reviewer:changes` and
+  `validator:failed` from the PR and `blocked` from the issue (or use the page's **Unblock** button). The fix-round counter
   stays where it was, so the next reviewer verdict must be an approval or the task is blocked
   again.
 
@@ -905,7 +917,8 @@ branch.
   ever produce one. Add any CI workflow. `checks none` is not `checks failure`: it earns no rerun
   and no fix round either, so the PR simply sits there.
 - **`approved plan file not on origin/main; retrying next tick`, for ever.** The plan PR really is
-  not merged yet, or the plan file's `NN` does not match the epic's `phase:N` label. The foreman
+  not merged yet — its title is not `docs(plan): phase NN plan (#<epic>)`, so the foreman did not
+  find it to merge — or the plan file's `NN` does not match the epic's `phase:N` label. The foreman
   looks for `<plans.dir>/<date>-phase-NN-plan.issues.json` where `NN` is that label, zero-padded;
   the planner is told the same rule, so a mismatch means the plan was hand-edited.
 - **`~/.foreman/<name>/artifacts/<pr>/` is empty after a validation run.** The foreman only copies
@@ -983,12 +996,11 @@ foreman -p sandbox run --once        # the planner claims the epic and drafts a
 ```
 
 Expect `claimed by <host> … role=planner`, then a plan PR against the sandbox and the epic at
-**In Review** with `needs-owner`. Merge the plan PR, approve the plan, and let the loop apply it:
+**In Review** with `needs-owner`. Approve the plan and let the loop merge the plan PR and apply it:
 
 ```bash
-gh pr merge <plan pr> --squash --repo acme/foreman-sandbox
 gh issue edit 1 --repo acme/foreman-sandbox --add-label plan-approved
-foreman -p sandbox run --once        # applies the plan: task issues at Ready, agent-ready
+foreman -p sandbox run --once        # merges the plan PR, then task issues at Ready, agent-ready
 foreman -p sandbox run --once        # claims the first task for a builder
 foreman -p sandbox start
 foreman -p sandbox status --watch    # or: foreman -p sandbox logs -f

src/attention.test.ts

@@ -24,6 +24,7 @@ const owner = (epic: number, actions: OwnerItem["actions"]): OwnerItem => ({
   actions,
   since: null,
   review: null,
+  planPr: null,
 });
 
 const needs = (issue: number, over: Partial<NeedsYouItem> = {}): NeedsYouItem => ({
@@ -50,6 +51,15 @@ const wait = (
 });
 
 describe("describeAttention", () => {
+  it("counts an approved plan whose PR cannot merge, and lists its wait only on Needs you", () => {
+    const b = board({
+      owner: [{ ...owner(9, ["pause"]), planPr: 279, detail: "plan PR #279 cannot merge" }],
+      waiting: [wait("human", "epic #9", "epic #9 plan PR #279 cannot merge: checks failed")],
+    });
+    const a = describeAttention(b);
+    expect(a.needsYou).toBe(1);
+    expect(a.waitRows).toEqual([]);
+  });
   it("counts approve and sign-off epics and unpaired needs-you issues, not start_planning", () => {
     const b = board({
       owner: [

src/attention.ts

@@ -28,14 +28,14 @@ const list = (ns: string[]) =>
  */
 export function describeAttention(board: Board | null): Attention {
   if (!board) return { needsYou: 0, waiting: 0, banners: [], waitRows: [] };
-  const gated = board.owner.filter((o) =>
-    o.actions.some((a) => a === "approve_plan" || a === "sign_off"),
+  const gated = board.owner.filter(
+    (o) => o.planPr !== null || o.actions.some((a) => a === "approve_plan" || a === "sign_off"),
   );
   const needsYou = gated.length + board.needsYou.filter((n) => n.reviewOf === null).length;
   // Every issue or epic that has a row of its own on Needs you, Ready to plan included.
   const onNeedsYou = new Set([
     ...board.owner
-      .filter((o) => o.actions.some((a) => a !== "pause" && a !== "unpause"))
+      .filter((o) => o.planPr !== null || o.actions.some((a) => a !== "pause" && a !== "unpause"))
       .map((o) => o.epic),
     ...board.needsYou.map((n) => n.issue),
   ]);

src/cli/daemon.ts

@@ -276,13 +276,15 @@ export async function runDaemon(
     },
     needsYouItems: () => store.get().board?.needsYou ?? [],
     unblock: async (issue) => {
-      // The stored row carries the project item id the tick already read, so the gate costs one
-      // label edit, one status write and one comment — no extra GitHub read.
+      // The stored board carries the project item id and the PR the tick already read, so the
+      // gate costs label edits, one status write and one comment — no extra GitHub read.
       const item = store.get().board?.needsYou.find((n) => n.issue === issue) ?? null;
+      const pr = store.get().board?.issues.find((n) => n.issue === issue)?.pr ?? null;
       const message = await applyUnblock(gh, {
         issue,
         itemId: item?.itemId ?? null,
         host: cfg.host,
+        pr,
       });
       log("info", "unblocked", { issue });
       // Same reason as the owner gate: reflect it locally so the button goes now, and wake

src/ledger.test.ts

@@ -1,6 +1,14 @@
 import { describe, expect, it } from "vitest";
 import { comment, hoursAgo } from "../test/helpers.ts";
-import { ciRerunCount, fixRound, fmt, lastActivityAt, openClaim, parseClaim } from "./ledger.ts";
+import {
+  ciRerunCount,
+  fixRound,
+  fmt,
+  lastActivityAt,
+  openClaim,
+  parseClaim,
+  planPrStuckReported,
+} from "./ledger.ts";
 
 describe("ledger", () => {
   it("round-trips a claim", () => {
@@ -94,3 +102,24 @@ describe("ci rerun ledger", () => {
     expect(openClaim(cs)?.claim.role).toBe("builder");
   });
 });
+
+describe("plan PR stuck ledger", () => {
+  const sha = "0123456789abcdef0123456789abcdef01234567";
+  it("formats the line and finds it again by PR and head commit", () => {
+    const line = fmt.planPrStuck("mac-a", 279, sha, "checks failed");
+    expect(line).toBe(
+      `plan PR #279 cannot merge at ${sha}: checks failed. Fix it or merge it by hand (foreman@mac-a)`,
+    );
+    const cs = [comment(line)];
+    expect(planPrStuckReported(cs, 279, sha)).toBe(true);
+    expect(planPrStuckReported(cs, 279, "f".repeat(40))).toBe(false);
+    expect(planPrStuckReported(cs, 280, sha)).toBe(false);
+  });
+  it("does not close an open claim", () => {
+    const cs = [
+      comment(fmt.claimed("mac-a", hoursAgo(2), "planner", 1), hoursAgo(2)),
+      comment(fmt.planPrStuck("mac-a", 279, sha, "checks failed"), hoursAgo(1)),
+    ];
+    expect(openClaim(cs)).not.toBeNull();
+  });
+});

src/ledger.ts

@@ -30,6 +30,8 @@ export const fmt = {
     `session ${id} interrupted on ${host}: stopped by operator after ${minutes}m`,
   aborted: (host: string) => `released by ${host}: aborted by operator`,
   unblocked: (host: string) => `unblocked by owner via foreman@${host}`,
+  planPrStuck: (host: string, pr: number, sha: string, reason: string) =>
+    `plan PR #${pr} cannot merge at ${sha}: ${reason}. Fix it or merge it by hand (foreman@${host})`,
 };
 
 const CLAIM =
@@ -90,6 +92,16 @@ export function ciRerunCount(comments: Comment[], sha: string): number {
   return comments.filter((c) => CI_RERUN.exec(c.body.trim())?.[1] === sha).length;
 }
 
+const PLAN_PR_STUCK = /^plan PR #(\d+) cannot merge at ([0-9a-f]{7,40}):/;
+
+/** Whether the owner was already told this plan PR head cannot merge; one comment per push. */
+export function planPrStuckReported(comments: Comment[], pr: number, sha: string): boolean {
+  return comments.some((c) => {
+    const m = PLAN_PR_STUCK.exec(c.body.trim());
+    return m?.[1] === String(pr) && m[2] === sha;
+  });
+}
+
 export function lastActivityAt(comments: Comment[]): string | null {
   return (
     comments

src/loop.test.ts

@@ -659,6 +659,27 @@ describe("execute", () => {
     expect(calls).toContain("setStatus PVTI_200 In Review");
     expect(calls).toContain("addLabels issue 200 needs-owner");
   });
+  it("merge_plan squash-merges the plan PR and goes on to apply it in the same tick", async () => {
+    const { gh, calls } = fakeGh([
+      issue({ number: 200, labels: ["epic", "phase:2", "plan-approved"] }),
+    ]);
+    const r = await execute({ type: "merge_plan", epic: 200, pr: 279 }, ctx({ gh }));
+    expect(r).toBe("continue");
+    expect(calls).toEqual(["mergePR 279"]);
+  });
+  it("plan_pr_stuck tells the owner on the epic why the plan PR cannot merge", async () => {
+    const { gh, calls } = fakeGh([
+      issue({ number: 200, labels: ["epic", "phase:2", "plan-approved"] }),
+    ]);
+    const sha = "0123456789abcdef0123456789abcdef01234567";
+    await execute(
+      { type: "plan_pr_stuck", epic: 200, pr: 279, sha, reason: "checks failed" },
+      ctx({ gh }),
+    );
+    expect(calls).toEqual([
+      `comment issue 200 ${fmt.planPrStuck("mac-a", 279, sha, "checks failed")}`,
+    ]);
+  });
   it("apply_plan leaves the epic alone when the plan is not on origin/main yet", async () => {
     const { gh, calls } = fakeGh([
       issue({ number: 200, labels: ["epic", "phase:2", "plan-approved"] }),

src/loop.ts

@@ -1011,6 +1011,24 @@ export async function execute(
       await runRole(ctx, { issue, role, pr: null, round: 1, resumeSessionId: null, notes: "" });
       return "stop";
     }
+    case "merge_plan":
+      // The owner's approval is the review; the apply_plan queued after this one fetches origin
+      // and finds the plan file in the same tick.
+      await gh.mergePR(action.pr);
+      log("info", "plan PR merged", { epic: action.epic, pr: action.pr });
+      return "continue";
+    case "plan_pr_stuck":
+      await gh.comment(
+        "issue",
+        action.epic,
+        fmt.planPrStuck(cfg.host, action.pr, action.sha, action.reason),
+      );
+      log("warn", "plan PR cannot merge", {
+        epic: action.epic,
+        pr: action.pr,
+        reason: action.reason,
+      });
+      return "continue";
     case "apply_plan":
       // A plan whose file is not on origin/main yet is left for the next tick, and that is not
       // work: counting it would make the loop hot until the plan PR merges.