fix(ship): the runner trusts the review child's own record of its post — a review posted a second ago no longer reads as unposted, and GitHub is asked patiently only when the record is silent

[justinhelmer]

A plan runner aborted a unit because it asked GitHub whether the review it had just been told about was posted — one second after the post, before GitHub's review list showed it. The review run now records its own post as a fact on its run record, the runner reads that fact, and GitHub is asked (patiently) only when the record has nothing to say.

What & why

Defect 6 of #992 (the runner's [agent] rows on #835 stay owed until a plan carries units through review to merges the runner made). Live on #1004: the review child finished at 01:55:39Z, the finish event woke the runner at 01:55:40Z, GitHub stamped the LGTM at 01:55:41Z, and read-record at 01:55:41–43Z asked GitHub's review list, read false, and settleReview aborted the unit — the approval could not be posted — while the approval stood on the pull request and auto-approve followed at 01:55:51Z.

The root cause is structural, not a timing accident: the review post-step ran in afterReply, after the seal in deliverAnswer that writes the run record and fires the finish event. Every record a runner read therefore predated the GitHub post by design; the chunked waits from defect 1's fix only made the runner fast enough to notice.

Three changes, each with red-first tests:

1. The review run records its post as a typed fact before it finishes. The post-step moves into the run loop, before registry.finish, in the coding post-step's position. A landed post publishes a review_posted event and rides the record as reviewPost: { posted: true, target, head, verdict }; a skip or failure publishes a review_not_posted note and rides as reviewPost: { posted: false, reason }. RunsService overlays the fact onto a finished row from the store like the verdict, so a reader woken by the finish sees it.
2. read-record prefers the record. The fact answers reviewPosted: true when the child posted this verdict at the reviewed head to the unit's pull request, and false with reviewPostReason for a recorded skip — GitHub never asked. Only a silent record (an older child, or a fact naming another head/verdict/pull request) asks GitHub, and then looks up to 3 times 2 s apart before answering false.
3. The machine's ending says how to recover in the runner's words. The abort reason carries the child's recorded reason; the report's existing unit-aware re-issue line supplies the recovery (the unit runs again when the plan is re-issued; a task unit still gets re-issue agent:ship … and include the PR URL). The old Re-run ship with the pull request URL sentence is gone.

The merge step's guard is untouched: it still re-verifies the approving review at the head on GitHub before the squash. The pre-check can afford patience because that guard stays strict.

Tour

1. read-record asks the record first, GitHub only when it is silent

The seam that failed live. The child's own record decides when it can; the GitHub look happens only when the record says nothing, and it is the patient one. The answer gains reviewPostReason when the record recorded why nothing was posted.

Look for: the fallback is entered only when reviewPostedByRecord answers undefined — a recorded false is final, never re-asked of GitHub.

switchboard/src/channels/adminCoordinator.ts

Lines 727 to 747 in 14b514c

	// Whether the verdict stands on the unit's pull request: the child's own
	// record of its post first (item 18) — it posted, or it recorded why not —
	// and GitHub only when the record is silent, looked at patiently: the
	// finish event wakes the runner within a second of the post, and GitHub's
	// review list can lag it. The merge step re-verifies the approval at the
	// head regardless, so the pre-check may be patient while the guard stays strict.
	let posted: { reviewPosted: boolean; reviewPostReason?: string } | undefined;
	if (record.verdict !== undefined && record.reviewHead !== undefined && typeof body.unit === "string") {
	const instance = await deps.instances.get(id.value);
	const row = instance ? (await deps.instances.listUnits(instance.id)).find((u) => u.unit === body.unit) : undefined;
	if (instance && row?.pr !== undefined) {
	const unitPr = { repo: instance.repo, number: row.pr.number };
	posted = reviewPostedByRecord(record, unitPr);
	if (posted === undefined) {
	const seen = await reviewPostedAtPatiently(deps, unitPr, record.verdict.verdict, record.reviewHead);
	if (seen !== undefined) posted = { reviewPosted: seen };
	}
	}
	}
	return json(200, {
	ok: true,

2. What the record is trusted to say

reviewPostedByRecord trusts a posted fact only when it names the unit's pull request, the reviewed head and the same verdict kind — anything else falls through to GitHub. A recorded skip is false with its reason.

switchboard/src/channels/adminCoordinator.ts

Lines 691 to 704 in 14b514c

	function reviewPostedByRecord(
	record: Pick<RunView, "reviewPost" | "reviewHead" | "verdict">,
	pr: { repo: string; number: number },
	): { reviewPosted: boolean; reviewPostReason?: string } | undefined {
	const post = record.reviewPost;
	if (post === undefined || record.reviewHead === undefined || record.verdict === undefined) return undefined;
	if (!post.posted) return { reviewPosted: false, reviewPostReason: post.reason };
	const same =
	post.target.repo === pr.repo &&
	post.target.number === pr.number &&
	sameCommit(post.head.toLowerCase(), record.reviewHead) &&
	post.verdict === record.verdict.verdict;
	return same ? { reviewPosted: true } : undefined;
	}

3. The patient GitHub look

Three looks, two seconds apart, stopping at the first true. The last look's answer stands: false when every look found nothing, undefined (absent from the answer, never a guess) when every look was silent. sleep is injectable so the tests record the pauses instead of waiting.

switchboard/src/channels/adminCoordinator.ts

Lines 628 to 637 in 14b514c

	/** How many times `read-record` looks at GitHub's review list for a review
	* child whose record carries no post of its own, and the pause between looks:
	* the list can lag a post it accepted a second ago, and the finish event that
	* wakes the runner arrives within that second. Three looks over a few
	* seconds cover the lag seen live; the merge step re-verifies the approval at
	* the head regardless, so this pre-check can afford patience and the guard
	* stays strict. */
	export const REVIEW_POSTED_CHECKS = 3;
	export const REVIEW_POSTED_RECHECK_MS = 2_000;

switchboard/src/channels/adminCoordinator.ts

Lines 669 to 684 in 14b514c

	async function reviewPostedAtPatiently(
	deps: AdminCoordinatorDeps,
	pr: { repo: string; number: number },
	verdict: ReviewVerdictKind,
	head: string,
	): Promise<boolean | undefined> {
	const sleep = deps.sleep ?? ((ms: number) => new Promise<void>((resolve) => setTimeout(resolve, ms)));
	let answer: boolean | undefined;
	for (let look = 1; look <= REVIEW_POSTED_CHECKS; look++) {
	answer = await reviewPostedAt(deps, pr, verdict, head);
	if (answer === true || look === REVIEW_POSTED_CHECKS) break;
	await sleep(REVIEW_POSTED_RECHECK_MS);
	}
	return answer;
	}

4. The post-step moves into the run loop

The structural half. The step now runs after the canonical answer is published and before the finally finishes the stream — so review_posted reaches the registry and reviewPost reaches registerFinishRecord. Only a review run reaches it; every other run's stream is unchanged (the span run.review_post_step joins the streamed table as a finishing-up span, like run.pr_post_step).

Look for: hardStopped: false is not a lie — the gate above it excludes hard stops, so a hard-stopped review posts nothing and records nothing, as before.

switchboard/src/core/dispatch/runLoop.ts

Lines 708 to 727 in 14b514c

	// Deterministic review post-step (runReviewPostStep in reviewRound.ts;
	// agent-review.md items 8, 10, 12, 15 and 18): a `review` run against a
	// resolved PR posts its findings back to that PR by default — no need to
	// ask — behind the reviewed-head guard (fail-closed) and pinned to the
	// verified head (or the carried one). HERE, in the run loop BEFORE the
	// finally below finish()es the stream — like the coding post-step above —
	// so the outcome is a fact of the record (`reviewPost`, the
	// `review_posted` event or the `review_not_posted` note) and a
	// coordinator woken by the finish reads whether the verdict landed
	// without asking GitHub, whose review list can lag a post it accepted a
	// second ago. Best-effort: a post failure is recorded and said in the
	// thread but never fails the run (the review lands in Slack regardless).
	// A HARD-stopped review has no findings — only the abort line — so
	// nothing is posted and nothing is recorded. The step reads the canonical
	// answer above: the GitHub body and the `answer` event are one dialect.
	// Only a review run reaches it: every other run's stream is as before.
	if (agent.name === "review" && run.control.requested !== "hard")
	reviewPost = await root.span("run.review_post_step", () =>
	runReviewPostStep({
	agent,

5. The step records how it ended

Every exit of runReviewPostStep passes through record: a landed post becomes the review_posted event with the pull request, the pinned head (the carried head after a rebase) and the verdict kind; every skip and failure becomes a review_not_posted note carrying the reason the outcome carries. Non-review and hard-stopped rounds publish nothing; without the publish seam the outcome is simply returned.

switchboard/src/core/reviewRound.ts

Lines 589 to 615 in 14b514c

	const record = (outcome: ReviewPostOutcome): ReviewPostOutcome => {
	if (input.publish && agent.name === "review" && !input.hardStopped) {
	const where = outcome.posted
	? undefined
	: repoCtx.repo && repoCtx.pr
	? `${repoCtx.repo}#${repoCtx.pr}`
	: undefined;
	input.publish(
	outcome.posted
	? {
	type: "review_posted",
	repo: outcome.target.repo,
	number: outcome.target.number,
	head: outcome.head,
	...(outcome.verdict !== undefined ? { verdict: outcome.verdict } : {}),
	at: systemClock(),
	}
	: {
	type: "run_note",
	kind: "review_not_posted",
	summary: `review not posted${where ? ` to ${where}` : ""}: ${outcome.reason}`,
	at: systemClock(),
	},
	);
	}
	return outcome;
	};

6. The typed fact and its event

ReviewPost is the outcome type and the record field, with a shape check for read-back and redaction of the skip's reason (it may carry GitHub's own words). ReviewPostOutcome becomes its alias, so the in-process ship loop's caller keeps compiling.

switchboard/src/core/reviewVerdict.ts

Lines 262 to 272 in 14b514c

	/** How a review run's post-step ended, as the run's record carries it
	* (docs/reference/specs/agent-review.md item 18; run-history item 2): the verdict
	* landed on a named pull request pinned to `head` (the verdict kind rides when
	* one was submitted — a review that posted without a verdict posts the
	* no-verdict line), or nothing landed and `reason` says why — a guard's
	* refusal, an opt-out, no pull request, GitHub's own error. A coordinator's
	* `read-record` answers `reviewPosted` from this before it asks GitHub, whose
	* review list can lag a post it accepted a second ago. */
	export type ReviewPost =
	| { posted: true; target: { repo: string; number: number }; head: string; verdict?: ReviewVerdictKind }
	| { posted: false; reason: string };

switchboard/src/core/runEvents.ts

Lines 449 to 469 in 14b514c

	| { type: "pr_opened"; url: string; number: number; created: boolean; seq?: number; at?: number }
	/** The review post-step's outcome when the verdict landed
	* (docs/reference/specs/agent-review.md item 18): the pull request it was
	* posted to, the head it was pinned to (the carried head after a rebase,
	* item 12) and the verdict kind when one was submitted. Published by the
	* post-step straight to the registry BEFORE the stream finishes — the
	* post-step runs inside the run loop, like the coding one — so the record
	* carries the post as a fact of the run and a coordinator woken by the
	* finish reads it there instead of asking GitHub, whose review list can
	* lag the post it just accepted. A post that did not land is a
	* `review_not_posted` note. Additive: unknown → ignored. */
	| {
	type: "review_posted";
	repo: string;
	number: number;
	head: string;
	verdict?: "approve" | "request_changes";
	seq?: number;
	at?: number;
	}
	/** One `agent:ship` round boundary (docs/reference/specs/agent-ship.md item 12): the

7. The machine's ending

The abort reason carries the child's recorded reason when there is one and states the fact; how to continue comes from renderUnitReport's unit-aware re-issue line, so a plan unit reads the unit runs again when the plan is re-issued and a task unit reads the agent:ship + PR URL form.

switchboard/src/core/ship/coordinator.ts

Lines 724 to 741 in 14b514c

	const notes = [roundNote(round, verdict.verdict)];
	if (verdict.verdict === "approve") {
	// Merge-ready stands on the POSTED approval: an approve whose post did
	// not land left no approving review on the pull request. The reason is
	// the child's own when it recorded one; how to continue is the report's
	// re-issue line, in the runner's words (`renderUnitReport`).
	if (facts.reviewPosted === false)
	return end(
	next,
	{
	kind: "aborted",
	reason: `⚠️ The review approved, but the approval could not be posted${facts.reviewPostReason !== undefined ? ` (${facts.reviewPostReason})` : ""} — the pull request carries no approving review.`,
	round,
	reviewRounds: next.reviewRounds,
	},
	notes,
	);
	const pr = next.pr!;

8. Tests — the record-first read and the patient fallback

GitHub shows no review; the record says posted → true, zero GitHub fetches, zero sleeps. Then: the record says skipped → false with the reason, GitHub never asked even though it shows a matching review. Then the fallback: the list surfaces the review on the third look → true after 3 fetches and 2 pauses of REVIEW_POSTED_RECHECK_MS; never shown → false after 3; silent → absent.

switchboard/src/channels/adminCoordinator.test.ts

Lines 1282 to 1306 in 14b514c

	it("read-record answers reviewPosted from the child's recorded post — true at the reviewed head with the verdict, on the unit's pull request — and never asks GitHub, whose review list may not surface the review yet", async () => {
	const HEAD = "a".repeat(40);
	// GitHub shows nothing: the review was posted a second ago.
	const h = await planHarness({ reviews: [] });
	await h.instances.putUnits([
	unitRow("U10", { threadKey: "slack:C1:2.0", pr: { number: 7, url: "https://github.com/acme/api/pull/7" } }),
	]);
	const tag = { parentInstanceId: PLAN_INSTANCE.id, idempotencyKey: "plan-fixture:U10/1/review" };
	await h.store.put(
	record("run-r1", {
	...tag,
	agent: "review",
	threadKey: "slack:C1:2.0",
	verdict: { verdict: "approve", summary: "clean", findings: [] },
	reviewHead: HEAD,
	reviewPost: { posted: true, target: { repo: "acme/api", number: 7 }, head: HEAD, verdict: "approve" },
	events: [{ type: "answer", text: "LGTM: clean", seq: 1 }],
	}),
	);
	const res = await call(h, "read-record", { parentInstanceId: PLAN_INSTANCE.id, runId: "run-r1", unit: "U10" });
	expect(res.body).toMatchObject({
	run: { id: "run-r1", verdict: { verdict: "approve" }, reviewHead: HEAD, reviewPosted: true },
	});
	expect("reviewPostReason" in (res.body as { run: Record<string, unknown> }).run).toBe(false);
	expect(h.reviewFetches()).toBe(0);

switchboard/src/channels/adminCoordinator.test.ts

Lines 1376 to 1399 in 14b514c

	it("with no recorded post (an older child) GitHub is asked up to three times, a pause apart, and the first sighting answers true; a review GitHub never shows answers false after the third; a GitHub that stays silent answers nothing", async () => {
	const HEAD = "a".repeat(40);
	const standing: PullRequestReview[] = [
	{ author: { login: "acme-switchboard[bot]", id: 4242 }, state: "COMMENTED", commitId: HEAD, body: "LGTM: clean" },
	];
	const seed = async (h: Awaited<ReturnType<typeof planHarness>>) => {
	await h.instances.putUnits([unitRow("U10", { threadKey: "slack:C1:2.0", pr: { number: 7, url: "u" } })]);
	await h.store.put(
	record("run-r1", {
	parentInstanceId: PLAN_INSTANCE.id,
	idempotencyKey: "plan-fixture:U10/1/review",
	agent: "review",
	verdict: { verdict: "approve", summary: "clean", findings: [] },
	reviewHead: HEAD,
	}),
	);
	return call(h, "read-record", { parentInstanceId: PLAN_INSTANCE.id, runId: "run-r1", unit: "U10" });
	};
	// The list surfaces the review on the third look.
	const late = await planHarness({ reviewsSequence: [[], [], standing] });
	expect((await seed(late)).body).toMatchObject({ run: { reviewPosted: true } });
	expect(late.reviewFetches()).toBe(3);
	expect(late.sleeps).toEqual([REVIEW_POSTED_RECHECK_MS, REVIEW_POSTED_RECHECK_MS]);
	// The first look already sees it: no pause.

9. Tests — the record is written after the post

End to end through dispatch(): the store sees the start tombstone, then the GitHub post, then the finish record carrying reviewPost — the ordering that was impossible before this change.

switchboard/src/core/dispatcher.test.ts

Lines 2015 to 2040 in 14b514c

	it("the record is written AFTER the post: the review_posted event and the reviewPost fact are on the record the store receives, and the record lands after the GitHub post was made (item 18)", async () => {
	const order: string[] = [];
	const deps = makeDeps(YAML_FIXTURE, verdictThenAnswer("approve", "ok"));
	deps.resolveRepoContext = () => ({ repo: "acme/api", ref: "patch-1", pr: 42, headSha: PR_HEAD });
	headExecutor(PR_HEAD);
	deps.postReviewComment = async () => void order.push("post");
	deps.fetchPrHead = async () => PR_HEAD;
	deps.runRegistry = new RunRegistry({ genId: () => "r-order", genToken: () => "t-order" });
	const inner = new InMemoryRunStore();
	const store: RunStore = {
	put: async (r) => {
	order.push(`record:${r.status}`);
	return inner.put(r);
	},
	get: (id) => inner.get(id),
	getSummary: (id) => inner.getSummary(id),
	list: (o) => inner.list(o),
	events: (id, o) => inner.events(id, o),
	delete: (id) => inner.delete(id),
	};
	deps.runHistoryWriter = createRunHistoryWriter({ store, warn: () => {}, sleep: async () => {} });
	await dispatch(deps, msg("agent:review https://github.com/acme/api/pull/42"), fakeIO().io);
	await deps.runHistoryWriter.settled();
	// The start tombstone (run-history item 27) lands at create; the finish
	// record — the one the runner reads — lands after the post.
	expect(order).toEqual(["record:interrupted", "post", "record:completed"]);

10. Tests — the step's fact and the machine's words

switchboard/src/core/reviewRound.test.ts

Lines 589 to 599 in 14b514c

	it("a successful post publishes `review_posted` with the pull request, the pinned head and the verdict — the same facts the outcome carries", async () => {
	const h = harness();
	const events: RunEvent[] = [];
	const out = await runReviewPostStep(base(h, events));
	expect(out).toEqual({ posted: true, target: { repo: "acme/api", number: 42 }, head: HEAD, verdict: "approve" });
	expect(events).toEqual([
	{ type: "review_posted", repo: "acme/api", number: 42, head: HEAD, verdict: "approve", at: expect.any(Number) },
	]);
	});
	it("a review carried across a rebase records the head it was pinned to — the current one — not the one it read", async () => {

switchboard/src/core/ship/coordinator.test.ts

Lines 639 to 659 in 14b514c

	it("an approve whose post did not land is an honest abort, never merge-ready — the report carries the child's recorded reason and says how to continue in the runner's words", () => {
	const d = fresh(input());
	throughRoundZero(d);
	runChild(
	d,
	"run-r1",
	finished({
	status: "completed",
	verdict: { verdict: "approve", summary: "x", findings: [] },
	reviewPosted: false,
	reviewPostReason: "digest covered 3 of 5 files",
	reviewHead: HEAD_A,
	}),
	T0 + 20 * MIN,
	);
	expect(d.action).toMatchObject({ type: "end", ending: { kind: "aborted" } });
	const report = renderUnitReport(d.state);
	expect(report).toContain("the approval could not be posted");
	expect(report).toContain("digest covered 3 of 5 files");
	expect(report).toContain("the unit runs again when the plan is re-issued");
	expect(report).not.toMatch(/Re-run ship/);

11. Specs

agent-review.md gains item 18 (the post as a recorded fact) with two bound rows; run-history.md items 2 and 21 name reviewPost and the rows bind the new tests; http-ingress.md item 9's read-record gains one sentence on the record-first answer and the patient fallback, and the row's criterion says the same; tracing.md item 18 moves the span from the log-only tail to the run.* list; run-visibility.md counts six side facts; live-view.md's parenthetical — the record carries no fact about whether a verdict reached GitHub — was made false by this change and now says the record carries it and the page does not read it yet.

switchboard/docs/reference/specs/agent-review.md

Line 41 in 14b514c

18. **The post is a recorded fact of the run — a reader never has to ask GitHub whether the verdict landed** (`runReviewPostStep` in [`reviewRound.ts`](../../../src/core/reviewRound.ts), called from the run loop; [run-history.md](run-history.md) items 2 and 21; [http-ingress.md](http-ingress.md) item 9): the post-step used to run after the reply and the seal that writes the record, so the record — and the finish event that wakes a plan runner within a second of it — always predated the GitHub post, and the runner's `read-record`, asking GitHub's review list about a review posted a second earlier, read `false` and aborted a unit whose approval stood on the pull request ten seconds later. The step now runs inside the run loop, after the `answer` event and BEFORE the stream finishes — the coding post-step's position — and records how it ended: a landed post publishes `review_posted { repo, number, head, verdict? }` (the pull request, the head it was pinned to — the carried head after a rebase, item 12 — and the verdict kind when one was submitted) and rides the record as `reviewPost: { posted: true, target, head, verdict? }`; a post that did not land — the reviewed-head guard (item 8), the digest-coverage guard (item 15), an opt-out, no pull request resolved, GitHub's own error — publishes a `review_not_posted` note naming the pull request (when one was resolved) and the reason, and rides the record as `reviewPost: { posted: false, reason }`, the reason redacted like every stored string. Only a review run records one; a hard-stopped review records nothing (the abort is the record's story) and a non-review run never reaches the step. The thread's notes are unchanged in wording and land before the verdict message now rather than after it. The plain path's outcome is otherwise byte-identical: the same guards, the same pinned post, the same Slack-only notes, the same log lines.

12. Remaining changes

• src/core/dispatch/reply.ts — afterReply is the reflection pass only (sync now); ReplyDeps loses postReviewComment/fetchPrHead; activityLine gains the review_posted case.
• src/core/dispatch/run.ts — RunDeps gains postReviewComment (the seam the run loop reads; CoreDeps shape unchanged).
• src/core/dispatcher.ts — passes requestText to the loop; the trimmed afterReply call; RunOutcome destructuring shrinks.
• src/core/dispatch/runLoop.ts — RunOutcome loses verdict/digest/observedHead/carried (nothing outside the loop read them once the post-step moved in); RunLoopContext.requestText.
• src/core/dispatch/record.ts — assembleRunRecord and registerFinishRecord carry reviewPost, the skip's reason redacted there.
• src/core/runRecord.ts — RunRecord.reviewPost, validated by isReviewPostShape; the summary row (RunListItem) inherits it, so the state Worker's summary_json carries it with no schema change.
• src/core/runsService.ts — RunView.reviewPost; storedArtifacts overlays it.
• src/core/runEvents.ts — the review_not_posted note kind beside the event.
• src/core/runEventLines.ts, src/core/runFriction.ts — the analyzer accepts the event and counts it as a side fact.
• src/core/trace/streamSpans.ts, src/core/trace/displayNames.ts — run.review_post_step streamed as finishing-up, "posting the review".
• src/core/coordinator/driver.ts — passes reviewPostReason through to the machine.
• src/core/ship/coordinator.ts — ChildFacts.reviewPostReason.
• src/core/dispatch/reply.test.ts — the afterReply tests keep only the reflection pass (the post-step tests live in the run loop's end-to-end tests now); src/core/dispatch/runLoop.test.ts — ctx gains requestText.
• src/core/reviewVerdict.test.ts, src/core/runRecord.test.ts, src/core/dispatch/record.test.ts, src/core/runsService.test.ts — the shape, the record, the assembly and the overlay carry reviewPost.
• src/core/ship/reviewChild.test.ts — two toEqual({ posted: true }) become toMatchObject (the outcome now carries the target, head and verdict); this file is on the parallel ship-loop removal's deletion list.
• docs/reference/specs/run-visibility.md, docs/reference/specs/live-view.md, docs/reference/specs/tracing.md, docs/reference/specs/run-history.md, docs/reference/specs/http-ingress.md — as in step 11.

Decisions

• Move the post before the finish rather than delay the seal. A publish on a finished run is a no-op, so the event could only exist if the post ran before registry.finish; delaying the seal would have left the fact as a record-only field with no stream event and no card/page witness. Moving the step also makes the two post-steps symmetric and retires the "deliberately after the registry finish" caveat that had only ever been a zero-behavior-change extraction constraint.
• Both an event and a typed field. The event is the stream's and the page's witness; the field is what the runner reads (via the RunsService overlay, like the verdict) and survives event-budget truncation. pr_opened is event-only today, which is why read-record reads it from events; the verdict artifacts set the other precedent, and the runner's read follows that one.
• Thread notes keep their wording and their own messages; they land before the verdict now. Appending them to the answer (the coding post-step's prNote pattern) would have changed a pinned Slack surface and several assertions for no gain; separate replies keep every existing note test valid.
• Only a review run records a fact; a hard stop records nothing. A coding run's record carrying reviewPost: { posted: false, reason: "no PR post was intended" } would be noise; a hard-stopped review's abort is its record's story.
• Pre-check patient, merge guard strict. The pre-check decides whether to enter the merge phase; being wrong costs a unit (the live defect). The merge guard decides whether to squash; being wrong costs a merge without an approval. Patience is cheap on the first and unacceptable on the second, so the guard is untouched.
• A recorded fact at another head, verdict or pull request is not trusted — GitHub decides. The test pinning this is green on main too (there was no fact to distrust); it guards the implementation against trusting a stale fact rather than pinning the defect, and is labelled as such below.
• reviewChild.test.ts is touched minimally. The in-process ship loop is being deleted in a parallel PR; two assertions loosen so verify is green here without widening the conflict.
• No frontend change. The run page ignores unknown events (runTimeline's default) and does not read reviewPost; live-view.md says so rather than pretending.

Validation

Red first against origin/main: 19 tests failed for the right reasons (missing exports, the old { posted: true } outcome shape, the old ending text); no red log is attached — the green CI run on this PR is the receipt for each row below.

Criterion	Proof	Receipt
A landed post publishes review_posted with the pull request, the pinned head (the carried head after a rebase) and the verdict; the outcome carries the same facts	[unit] src/core/reviewRound.test.ts::runReviewPostStep (explicit AgentDef decides the post)::the post as a recorded fact (item 18)::*	CI test on this PR
A guard's refusal and GitHub's own error publish a review_not_posted note with the reason; no pull request records the skip without a target; non-review and hard-stopped rounds publish nothing; no publish seam → same outcome	same describe	CI test
The record carries reviewPost redacted and validates; isRunRecord accepts posted and skipped shapes after a JSON round-trip and refuses malformed ones; the stored shape refuses a posted outcome without target/head and a skip without a reason	[unit] src/core/dispatch/record.test.ts::…::carries the review post…, src/core/runRecord.test.ts::isRunRecord — the review's verdict and head…::accepts the review post…, src/core/reviewVerdict.test.ts::the stored review post — isReviewPostShape and its redaction::*	CI test
A finished row still in the registry carries reviewPost from the store's summary row	[unit] src/core/runsService.test.ts::RunsService.getRun::a finished run still in the registry carries verdict, reviewHead, reviewPost, dispositions and handoff…	CI test
read-record answers reviewPosted: true from the fact with zero GitHub calls; a recorded skip answers false + reviewPostReason with zero GitHub calls	[unit] src/channels/adminCoordinator.test.ts::…::read-record answers reviewPosted from the child's recorded post…, ::read-record answers a recorded skip…	CI test
Silent record → GitHub asked up to 3 times, REVIEW_POSTED_RECHECK_MS apart, first true wins; never shown → false after 3; silent GitHub → absent	[unit] src/channels/adminCoordinator.test.ts::…::with no recorded post (an older child) GitHub is asked up to three times…	CI test
A fact at another head/verdict/pull request is not trusted; GitHub decides (guard test — green on main too, pins the fallback not the defect)	[unit] src/channels/adminCoordinator.test.ts::…::a recorded post at another head, with another verdict or on another pull request is not trusted…	CI test
End to end: a review run's finish record carries reviewPost and the review_posted event; the store receives the tombstone, then the post, then the finish record; a guard-refused post is recorded as a skip beside a review_not_posted note	[unit] src/core/dispatcher.test.ts::review post-step::head-moved note (item 10)::the review run's record carries…, ::the record is written AFTER the post…, ::a post the reviewed-head guard refused is recorded as a skip…	CI test
The aborted report carries the child's reason and the runner's re-issue vocabulary, never "Re-run ship"; a task unit gets the agent:ship + PR URL form	[unit] src/core/ship/coordinator.test.ts::the unit pipeline — every ending…::an approve whose post did not land…, ::an approve GitHub shows no post for…	CI test
The plain review path is otherwise unchanged: every existing dispatcher review post-step test (guards, notes, pinned posts, opt-outs, no-gaps) is green with the step in its new position	[unit] src/core/dispatcher.test.ts::review post-step::*, ::no gaps…::*	CI test
Specs bound and covering: specs:check (2703 proofs), specs:coverage --changed HEAD --test-guard clean; npm run verify green locally (368 files / 6790 tests, lint, format, typecheck, PR title check)	[gate]	CI checks on this PR
Human-gated / live: a plan re-issued on this build carries a unit whose review child's approval is read from its record within a second of the finish and the runner merges it	[agent] re-issue 2026-09-14-001-feat-title-gate-in-the-child-contract-plan after deploy	receipt to be posted on #835

🤖 Generated with Claude Code

[coreplane-switchboard]

LGTM: Correct fix for the review-posted race: the post-step moves inside the run loop so the record carries the post as a fact, the runner trusts the record and only asks GitHub patiently when it's silent — thoroughly tested and spec-updated in the same diff.

• [nit] F1 src/core/dispatch/runLoop.ts:723 — An unexpected throw from the post-step now fails the whole run (it runs inside the loop's try), unlike the old best-effort position
• [nit] F2 src/channels/adminCoordinator.ts:633 — read-record's patient loop can hold the step ~4s per call when GitHub stays silent — fine today, worth a note on the step's latency budget

Verdict: approve — the race fix is sound, the record-first design is well-guarded, and the tests cover every path (posted, skipped, GitHub-lagging, stale record, silent GitHub).

What the change does, verified:

• runReviewPostStep moves from afterReply (after the seal/record) into the run loop, before the stream finishes — mirroring the coding post-step — so the finish record carries reviewPost (review_posted event / review_not_posted note). dispatcher.test.ts proves ordering (record:interrupted → post → record:completed).
• read-record answers reviewPosted from the child's record when it matches the unit's PR, head, and verdict (reviewPostedByRecord, using sameCommit); a stale/mismatched record falls back to GitHub. With no record it asks GitHub up to 3 times, 2s apart (REVIEW_POSTED_CHECKS/REVIEW_POSTED_RECHECK_MS), silent → unknown, never a guess.
• ReviewPost is shape-checked on read (isReviewPostShape), the skip reason redacted at the record seam, and the ship coordinator's abort message now carries the recorded reason. Specs (agent-review item 18, http-ingress item 9, run-history, run-visibility, tracing) are updated in the same diff with validation rows bound to the new tests.

Findings (both nits, non-blocking):

• F1 src/core/dispatch/runLoop.ts:~723 — the step now runs inside the loop's try, so an unexpected throw (e.g. from an injected publish) would mark the run failed, where the old afterReply position was after the record landed. In practice every risky call inside the step is caught (post try/catch, .catch() on replies/fetch), so this is theoretical.
• F2 src/channels/adminCoordinator.ts:~633 — the patient loop can hold a read-record call ~4s (3 fetches + 2 sleeps) when GitHub stays silent; fine for the runner's cadence, just worth remembering if the step ever gets an HTTP timeout.

Test-guard dispositions: the four reply.test.ts check lines and the runsService.test.ts rename are allowed by same-diff spec changes; the coordinator.test.ts retitle ("an approve whose post did not land…") is a refactor with verification strengthened (the new title adds assertions on the recorded reason and re-issue wording) — intact. The reply.test.ts coverage removed there is re-proven end-to-end in dispatcher.test.ts and reviewRound.test.ts, per the deleted tests' own comment.

[github-actions]

Auto-approved: coreplane-switchboard[bot] reviewed this PR and posted an LGTM verdict (see its review). This repository opted in through its REVIEW_BOT_LOGIN and REVIEW_BOT_ID variables.

[justinhelmer]

Review round 1 at 14b514c: LGTM with two nits. Gate is minor; a nits-only round is left as is — both findings are recorded here as skipped, not ignored.

• F1 (runLoop.ts — a throw inside the step would now fail the run): skipped under the gate. runReviewPostStep's contract is "never throws" — the post is in a try/catch, every reply and the head fetch carry .catch(), and the one new call, registry.publish, is synchronous and a silent no-op on a finished run. The coding post-step sits in the same try under the same contract.
• F2 (adminCoordinator.ts — the patient look can hold read-record ~4 s when GitHub stays silent): skipped under the gate. The step is bounded (3 looks × 2 s + three fetches) and the Workflow's read-record step has retries and no timeout; if a step timeout is ever added, REVIEW_POSTED_CHECKS/REVIEW_POSTED_RECHECK_MS are the knobs.

Not merging: the live row on #835 (a re-issued plan whose review child's approval is read from its record within a second of the finish) is owed after deploy.

[coreplane-switchboard]

No verdict submitted — not approving.

Noted — nothing further from me. Both dispositions match my read: F1's "never throws" contract holds (every risky call inside the step is caught, and the coding post-step's identical placement is precedent), and F2's ~4s worst case is bounded and safe given the workflow step's retry posture. Round 1 stands as approved at 14b514c0; makes sense to hold the merge until the live row on #835 is in hand post-deploy.