5d9b0f2 · 0ab2ba8 · d0ca8e2
@@ -361,7 +361,9 @@
asked, under **Settings → Autonomy**:
  missing reviewer never holds a pull silently. An agent's verdict stands
  only while the agent is a reviewer on the crew, and the reviewer gets two
  rounds of changes on a pull — a round is a head it asked changes on;
  past them its request is advice. A builder's run that answers a change
  past them its request is advice, and the third request hands the pull
  to a person in one card summarising the three reviews (not parked, not
  closed; a person's approval lands it). A builder's run that answers a change
  request without pushing parks the pull. A review run never resumes an
  earlier session, and the same verdict sent twice on one head is one. Nothing waits for a
  person: a pull whose check stays red past its tries, or whose branch
@@ -372,8 +374,8 @@
asked, under **Settings → Autonomy**:
  planned again within twenty minutes, and the planner is told the
  direction is never finished: when what it names is built, it proposes
  the next increment and leaves at least three ready issues. The reviewer
  gets two rounds of changes on a pull; after that it lands and what is
  left becomes an issue. The only pause is the day's ceiling. A person
  gets two rounds of changes on a pull; the third hands the pull to a
  person. The only pause is the day's ceiling. A person
  steers by editing the direction and the map; the crew does the rest. When nothing is
  ready and nothing is in flight, the map has run dry: the planner is
  given the direction.
@@ -15,6 +15,7 @@
import { emailVerified } from "../core/verify.ts";
import { rmSync } from "node:fs";
import type { App } from "../app.ts";
import { transact } from "../kernel/db.ts";
import { addComment } from "../actions/issues.ts";
import { announceRun } from "../actions/runs.ts";
import { crewRole, getAgent, type Agent } from "../core/agents.ts";
import type { ForgeEvent } from "../core/events.ts";
@@ -31,7 +32,13 @@
import { accountActive } from "../core/accounts.ts";
import { decideAndRecord } from "./ledger.ts";
import { getRepoById, type Repo } from "../core/repos.ts";
import { listReviews } from "../core/reviews.ts";
import { greenHead, sendToReview } from "./review.ts";
import {
  greenHead,
  handedToPerson,
  handoverNote,
  reviewRounds,
  sendToReview,
} from "./review.ts";
import {
  activeRunFor,
  hasQueuedRun,
@@ -433,6 +440,23 @@
function route(app: App, event: ForgeEvent): void {
      }
      // The crew is off: a verdict is read by people, and wakes no one.
      if (reviewerVerdict && repo.autonomy === "off") return;
      if (reviewerVerdict) {
        // The loop is bounded: the third round hands the pull to a person.
        const pull = getPull(app.db, repo.id, number);
        if (pull && handedToPerson(app.db, repo.id, pull)) {
          // One card: a re-sent verdict does not post a second.
          const last = listComments(app.db, issue.id, { kind: "comment" }).at(-1);
          if (!last?.body.startsWith("Handed to a person"))
            addComment(
              app,
              actor,
              repo,
              issue,
              handoverNote(reviewRounds(app.db, repo.id, pull)),
            );
      
        junipercrew
        1mo ago
        
      
      Only the last comment is checked, so a comment landing between two verdicts yields a second card. Fix: listComments(...).some((c) => c.body.startsWith("Handed to a person")).
    
          return;
        }
      }
      const agent = agentFor(app, issue);
      if (!agent) return;
      // Said while the agent works: it hears the words now, as its next
@@ -24,7 +24,7 @@
import { getHold, lastRelease } from "../core/holds.ts";
import { getIssueById } from "../core/issues.ts";
import { listPulls, pullDiff, type Pull, pullDir} from "../core/pulls.ts";
import { getRepoById, type Repo } from "../core/repos.ts";
import { reviewSummary } from "../core/reviews.ts";
import { listReviews, reviewSummary, type Review } from "../core/reviews.ts";
import { checksForHead } from "../core/runs.ts";
import { repoDir } from "../git/repos.ts";
import { createLog } from "../kernel/log.ts";
@@ -142,9 +142,55 @@
export function reviewAttempts(
  return { greenAt: since, attempts };
}
/** Rounds of changes the crew's reviewer asks on one pull before it is handed to a person. */
export const HANDOVER_ROUNDS = 3;
/**
 * The crew reviewer's rounds on a pull: its first request for changes on
 * each head, oldest first. Two requests on one head are one round.
 */
export function reviewRounds(db: Database, repoId: string, pull: Pull): Review[] {
  const reviewers = new Set(
    reviewersOf(db, repoId).map((a) => a.user.id),
  );
  const heads = new Set<string>();
  const rounds: Review[] = [];
  for (const r of listReviews(db, pull.id)) {
    if (r.state !== "changes_requested" || !reviewers.has(r.authorId)) continue;
    if (heads.has(r.commitOid)) continue;
    heads.add(r.commitOid);
    rounds.push(r);
  }
  return rounds;
}
/**
 * Whether the crew's review loop on this agent pull is spent: the
 * reviewer asked for changes `HANDOVER_ROUNDS` times and no approval is on
 * the head. Nothing more is sent or started by the crew; a person decides.
 */
export function handedToPerson(db: Database, repoId: string, pull: Pull): boolean {
  if (!pull.authorIsAgent) return false;
  if (reviewRounds(db, repoId, pull).length < HANDOVER_ROUNDS) return false;
  return reviewSummary(db, pull.id, pull.headOid).approvals === 0;
}
/** The one card that hands a pull over: each round's verdict, in a line or two. */
export function handoverNote(rounds: readonly Review[]): string {
  const lines = rounds.map((r, i) => {
    const said = r.body.trim().replace(/\s+/g, " ");
      
        junipercrew
        1mo ago
        
      
      The card promises 'push a fix', but handedToPerson stays true on the new head, so that push is never reviewed (sendToReview returns 'held'). Say that only approval or a decision moves it, or count rounds only since the last person's push.
    
    return `${i + 1}. ${r.authorName}: ${said === "" ? "(no words)" : said.length > 300 ? `${said.slice(0, 300)}…` : said}`;
  });
  return [
    `Handed to a person: the reviewer asked for changes ${rounds.length} times without approving, so the crew stops here. Nothing is closed or parked; approve it, push a fix, or say what to do.`,
    "",
    ...lines,
  ].join("\n");
}
/** Why an agent's green pull waits for a person, and since when. */
export interface ReviewWait {
  readonly why: "no reviewer" | "lapsed";
  readonly why: "no reviewer" | "lapsed" | "rounds";
  /** When review became owed: the head went green, or the pull was let go. */
  readonly since: string;
}
@@ -166,6 +212,8 @@
export function waitsForPerson(
  if (!pull.authorIsAgent || getHold(db, pull.id) !== undefined) return undefined;
  const seen = reviewSummary(db, pull.id, pull.headOid);
  if (seen.approvals > 0 || seen.changesRequested > 0) return undefined;
  if (handedToPerson(db, repoId, pull))
    return { why: "rounds", since: reviewRounds(db, repoId, pull).at(-1)!.createdAt };
  const history = reviewAttempts(db, pull);
  if (history === undefined) return undefined;
  if (reviewersOf(db, repoId).length === 0)
@@ -194,6 +242,7 @@
export async function sendToReview(
  if (reviewers.length === 0) return "none";
  const seen = reviewSummary(app.db, pull.id, pull.headOid);
  if (seen.approvals > 0 || seen.changesRequested > 0) return "done";
  if (handedToPerson(app.db, repo.id, pull)) return "held";
  const history = reviewAttempts(app.db, pull);
  if (history === undefined) return "wait";
  const stage = reviewStage(history.greenAt, history.attempts, now);
@@ -647,7 +647,9 @@
function lineOf(c: AskCandidate): string {
/** Why an agent's change waits for a person's review, in a clause. */
const reviewWhy = (wait: ReviewWait | undefined): string =>
  wait?.why === "lapsed"
  wait?.why === "rounds"
    ? `the reviewer asked for changes three times, since ${wait.since.slice(0, 10)}`
    : wait?.why === "lapsed"
    ? `the reviewer did not answer, since ${wait.since.slice(0, 10)}`
    : `no reviewer on the crew, since ${wait?.since.slice(0, 10) ?? "it went green"}`;
@@ -13,7 +13,8 @@
import { addToCrew, createAgent } from "../src/actions/crew.ts";
import { addComment, openIssue } from "../src/actions/issues.ts";
import { commitToBranch } from "../src/actions/repos.ts";
import { openPull, submitReview } from "../src/actions/pulls.ts";
import type { Pull } from "../src/core/pulls.ts";
import { getPull, type Pull } from "../src/core/pulls.ts";
import { handedToPerson, sendToReview, waitsForPerson } from "../src/crew/review.ts";
import { createRepo, updateRepoSettings } from "../src/actions/repos.ts";
import { setCrewRole } from "../src/core/agents.ts";
import { getIssueById, listComments, type Issue } from "../src/core/issues.ts";
@@ -316,6 +317,59 @@
describe("a crew reviewer's changes-requested wakes the builder", () => {
    });
  }
  test("the third round hands the pull to a person in one card, and starts nothing", async () => {
    const set = await updateRepoSettings(app, alice, fresh(), {
      autonomy: "autonomous",
      autonomyCeilingUsd: 100,
      autonomyBy: alice.id,
    });
    if (!set.ok) throw new Error(JSON.stringify(set.error));
    let pull = await masonPull("verdict-rounds");
    const ask = async (body: string) => {
      const sent = await submitReview(app, sentinel, fresh(), pull, {
        state: "changes_requested",
        body,
      });
      if (!sent.ok) throw new Error(String(sent.error));
      await settle();
    };
    const push = async (n: number) => {
      await commitToBranch(app, alice, repo, {
        branch: "feature/verdict-rounds",
        base: "main",
        message: `more ${n}`,
        files: [{ path: `more-${n}.txt`, content: `${n}\n` }],
      });
      pull = getPull(app.db, repo.id, pull.number)!;
    };
    const cards = () =>
      listComments(app.db, pull.id, { kind: "comment" }).filter((c) =>
        c.body.startsWith("Handed to a person"),
      );
    await ask("First: name the fence.");
    await push(1);
    await ask("Second: the test is missing.");
    await push(2);
    expect(feedback(pull).length).toBe(1);
    expect(handedToPerson(app.db, repo.id, pull)).toBe(false);
    await ask("Third: still no fence.");
    expect(feedback(pull).length).toBe(1);
    expect(handedToPerson(app.db, repo.id, pull)).toBe(true);
    expect(cards().length).toBe(1);
    for (const said of ["First: name", "Second: the test", "Third: still"])
      expect(cards()[0]!.body).toContain(said);
    // Not closed, and it waits where a person looks; a re-sent verdict adds no card.
    expect(getPull(app.db, repo.id, pull.number)!.state).toBe("open");
    expect(waitsForPerson(app.db, repo.id, pull)?.why).toBe("rounds");
    expect(await sendToReview(app, fresh(), pull)).toBe("held");
    await ask("Third again.");
    expect(cards().length).toBe(1);
    // A person's approval ends the handover.
    const ok = await submitReview(app, alice, fresh(), pull, { state: "approved" });
    if (!ok.ok) throw new Error(String(ok.error));
    expect(handedToPerson(app.db, repo.id, pull)).toBe(false);
  });
  test("in off, the verdict queues nothing", async () => {
    const set = await updateRepoSettings(app, alice, fresh(), {
      autonomy: "off",
		@@ -361,7 +361,9 @@ asked, under Settings → Autonomy:
361	361	missing reviewer never holds a pull silently. An agent's verdict stands
362	362	only while the agent is a reviewer on the crew, and the reviewer gets two
363	363	rounds of changes on a pull — a round is a head it asked changes on;
364		past them its request is advice. A builder's run that answers a change
	364	past them its request is advice, and the third request hands the pull
	365	to a person in one card summarising the three reviews (not parked, not
	366	closed; a person's approval lands it). A builder's run that answers a change
365	367	request without pushing parks the pull. A review run never resumes an
366	368	earlier session, and the same verdict sent twice on one head is one. Nothing waits for a
367	369	person: a pull whose check stays red past its tries, or whose branch
		@@ -372,8 +374,8 @@ asked, under Settings → Autonomy:
372	374	planned again within twenty minutes, and the planner is told the
373	375	direction is never finished: when what it names is built, it proposes
374	376	the next increment and leaves at least three ready issues. The reviewer
375		gets two rounds of changes on a pull; after that it lands and what is
376		left becomes an issue. The only pause is the day's ceiling. A person
	377	gets two rounds of changes on a pull; the third hands the pull to a
	378	person. The only pause is the day's ceiling. A person
377	379	steers by editing the direction and the map; the crew does the rest. When nothing is
378	380	ready and nothing is in flight, the map has run dry: the planner is
379	381	given the direction.
		@@ -15,6 +15,7 @@ import { emailVerified } from "../core/verify.ts";
15	15	import { rmSync } from "node:fs";
16	16	import type { App } from "../app.ts";
17	17	import { transact } from "../kernel/db.ts";
	18	import { addComment } from "../actions/issues.ts";
18	19	import { announceRun } from "../actions/runs.ts";
19	20	import { crewRole, getAgent, type Agent } from "../core/agents.ts";
20	21	import type { ForgeEvent } from "../core/events.ts";
		@@ -31,7 +32,13 @@ import { accountActive } from "../core/accounts.ts";
31	32	import { decideAndRecord } from "./ledger.ts";
32	33	import { getRepoById, type Repo } from "../core/repos.ts";
33	34	import { listReviews } from "../core/reviews.ts";
34		import { greenHead, sendToReview } from "./review.ts";
	35	import {
	36	greenHead,
	37	handedToPerson,
	38	handoverNote,
	39	reviewRounds,
	40	sendToReview,
	41	} from "./review.ts";
35	42	import {
36	43	activeRunFor,
37	44	hasQueuedRun,
		@@ -433,6 +440,23 @@ function route(app: App, event: ForgeEvent): void {
433	440	}
434	441	// The crew is off: a verdict is read by people, and wakes no one.
435	442	if (reviewerVerdict && repo.autonomy === "off") return;
	443	if (reviewerVerdict) {
	444	// The loop is bounded: the third round hands the pull to a person.
	445	const pull = getPull(app.db, repo.id, number);
	446	if (pull && handedToPerson(app.db, repo.id, pull)) {
	447	// One card: a re-sent verdict does not post a second.
	448	const last = listComments(app.db, issue.id, { kind: "comment" }).at(-1);
	449	if (!last?.body.startsWith("Handed to a person"))
	450	addComment(
	451	app,
	452	actor,
	453	repo,
	454	issue,
	455	handoverNote(reviewRounds(app.db, repo.id, pull)),
	456	);
		junipercrew 1mo ago Only the last comment is checked, so a comment landing between two verdicts yields a second card. Fix: `listComments(...).some((c) => c.body.startsWith("Handed to a person"))`.
	457	return;
	458	}
	459	}
436	460	const agent = agentFor(app, issue);
437	461	if (!agent) return;
438	462	// Said while the agent works: it hears the words now, as its next
		@@ -24,7 +24,7 @@ import { getHold, lastRelease } from "../core/holds.ts";
24	24	import { getIssueById } from "../core/issues.ts";
25	25	import { listPulls, pullDiff, type Pull, pullDir} from "../core/pulls.ts";
26	26	import { getRepoById, type Repo } from "../core/repos.ts";
27		import { reviewSummary } from "../core/reviews.ts";
	27	import { listReviews, reviewSummary, type Review } from "../core/reviews.ts";
28	28	import { checksForHead } from "../core/runs.ts";
29	29	import { repoDir } from "../git/repos.ts";
30	30	import { createLog } from "../kernel/log.ts";
		@@ -142,9 +142,55 @@ export function reviewAttempts(
142	142	return { greenAt: since, attempts };
143	143	}
144	144
	145	/** Rounds of changes the crew's reviewer asks on one pull before it is handed to a person. */
	146	export const HANDOVER_ROUNDS = 3;
	147
	148	/**
	149	* The crew reviewer's rounds on a pull: its first request for changes on
	150	* each head, oldest first. Two requests on one head are one round.
	151	*/
	152	export function reviewRounds(db: Database, repoId: string, pull: Pull): Review[] {
	153	const reviewers = new Set(
	154	reviewersOf(db, repoId).map((a) => a.user.id),
	155	);
	156	const heads = new Set<string>();
	157	const rounds: Review[] = [];
	158	for (const r of listReviews(db, pull.id)) {
	159	if (r.state !== "changes_requested" \|\| !reviewers.has(r.authorId)) continue;
	160	if (heads.has(r.commitOid)) continue;
	161	heads.add(r.commitOid);
	162	rounds.push(r);
	163	}
	164	return rounds;
	165	}
	166
	167	/**
	168	* Whether the crew's review loop on this agent pull is spent: the
	169	* reviewer asked for changes `HANDOVER_ROUNDS` times and no approval is on
	170	* the head. Nothing more is sent or started by the crew; a person decides.
	171	*/
	172	export function handedToPerson(db: Database, repoId: string, pull: Pull): boolean {
	173	if (!pull.authorIsAgent) return false;
	174	if (reviewRounds(db, repoId, pull).length < HANDOVER_ROUNDS) return false;
	175	return reviewSummary(db, pull.id, pull.headOid).approvals === 0;
	176	}
	177
	178	/** The one card that hands a pull over: each round's verdict, in a line or two. */
	179	export function handoverNote(rounds: readonly Review[]): string {
	180	const lines = rounds.map((r, i) => {
	181	const said = r.body.trim().replace(/\s+/g, " ");
		junipercrew 1mo ago The card promises 'push a fix', but handedToPerson stays true on the new head, so that push is never reviewed (sendToReview returns 'held'). Say that only approval or a decision moves it, or count rounds only since the last person's push.
	182	return `${i + 1}. ${r.authorName}: ${said === "" ? "(no words)" : said.length > 300 ? `${said.slice(0, 300)}…` : said}`;
	183	});
	184	return [
	185	`Handed to a person: the reviewer asked for changes ${rounds.length} times without approving, so the crew stops here. Nothing is closed or parked; approve it, push a fix, or say what to do.`,
	186	"",
	187	...lines,
	188	].join("\n");
	189	}
	190
145	191	/** Why an agent's green pull waits for a person, and since when. */
146	192	export interface ReviewWait {
147		readonly why: "no reviewer" \| "lapsed";
	193	readonly why: "no reviewer" \| "lapsed" \| "rounds";
148	194	/** When review became owed: the head went green, or the pull was let go. */
149	195	readonly since: string;
150	196	}
		@@ -166,6 +212,8 @@ export function waitsForPerson(
166	212	if (!pull.authorIsAgent \|\| getHold(db, pull.id) !== undefined) return undefined;
167	213	const seen = reviewSummary(db, pull.id, pull.headOid);
168	214	if (seen.approvals > 0 \|\| seen.changesRequested > 0) return undefined;
	215	if (handedToPerson(db, repoId, pull))
	216	return { why: "rounds", since: reviewRounds(db, repoId, pull).at(-1)!.createdAt };
169	217	const history = reviewAttempts(db, pull);
170	218	if (history === undefined) return undefined;
171	219	if (reviewersOf(db, repoId).length === 0)
		@@ -194,6 +242,7 @@ export async function sendToReview(
194	242	if (reviewers.length === 0) return "none";
195	243	const seen = reviewSummary(app.db, pull.id, pull.headOid);
196	244	if (seen.approvals > 0 \|\| seen.changesRequested > 0) return "done";
	245	if (handedToPerson(app.db, repo.id, pull)) return "held";
197	246	const history = reviewAttempts(app.db, pull);
198	247	if (history === undefined) return "wait";
199	248	const stage = reviewStage(history.greenAt, history.attempts, now);
		@@ -647,7 +647,9 @@ function lineOf(c: AskCandidate): string {
647	647
648	648	/** Why an agent's change waits for a person's review, in a clause. */
649	649	const reviewWhy = (wait: ReviewWait \| undefined): string =>
650		wait?.why === "lapsed"
	650	wait?.why === "rounds"
	651	? `the reviewer asked for changes three times, since ${wait.since.slice(0, 10)}`
	652	: wait?.why === "lapsed"
651	653	? `the reviewer did not answer, since ${wait.since.slice(0, 10)}`
652	654	: `no reviewer on the crew, since ${wait?.since.slice(0, 10) ?? "it went green"}`;
653	655
		@@ -13,7 +13,8 @@ import { addToCrew, createAgent } from "../src/actions/crew.ts";
13	13	import { addComment, openIssue } from "../src/actions/issues.ts";
14	14	import { commitToBranch } from "../src/actions/repos.ts";
15	15	import { openPull, submitReview } from "../src/actions/pulls.ts";
16		import type { Pull } from "../src/core/pulls.ts";
	16	import { getPull, type Pull } from "../src/core/pulls.ts";
	17	import { handedToPerson, sendToReview, waitsForPerson } from "../src/crew/review.ts";
17	18	import { createRepo, updateRepoSettings } from "../src/actions/repos.ts";
18	19	import { setCrewRole } from "../src/core/agents.ts";
19	20	import { getIssueById, listComments, type Issue } from "../src/core/issues.ts";
		@@ -316,6 +317,59 @@ describe("a crew reviewer's changes-requested wakes the builder", () => {
316	317	});
317	318	}
318	319
	320	test("the third round hands the pull to a person in one card, and starts nothing", async () => {
	321	const set = await updateRepoSettings(app, alice, fresh(), {
	322	autonomy: "autonomous",
	323	autonomyCeilingUsd: 100,
	324	autonomyBy: alice.id,
	325	});
	326	if (!set.ok) throw new Error(JSON.stringify(set.error));
	327	let pull = await masonPull("verdict-rounds");
	328	const ask = async (body: string) => {
	329	const sent = await submitReview(app, sentinel, fresh(), pull, {
	330	state: "changes_requested",
	331	body,
	332	});
	333	if (!sent.ok) throw new Error(String(sent.error));
	334	await settle();
	335	};
	336	const push = async (n: number) => {
	337	await commitToBranch(app, alice, repo, {
	338	branch: "feature/verdict-rounds",
	339	base: "main",
	340	message: `more ${n}`,
	341	files: [{ path: `more-${n}.txt`, content: `${n}\n` }],
	342	});
	343	pull = getPull(app.db, repo.id, pull.number)!;
	344	};
	345	const cards = () =>
	346	listComments(app.db, pull.id, { kind: "comment" }).filter((c) =>
	347	c.body.startsWith("Handed to a person"),
	348	);
	349	await ask("First: name the fence.");
	350	await push(1);
	351	await ask("Second: the test is missing.");
	352	await push(2);
	353	expect(feedback(pull).length).toBe(1);
	354	expect(handedToPerson(app.db, repo.id, pull)).toBe(false);
	355	await ask("Third: still no fence.");
	356	expect(feedback(pull).length).toBe(1);
	357	expect(handedToPerson(app.db, repo.id, pull)).toBe(true);
	358	expect(cards().length).toBe(1);
	359	for (const said of ["First: name", "Second: the test", "Third: still"])
	360	expect(cards()[0]!.body).toContain(said);
	361	// Not closed, and it waits where a person looks; a re-sent verdict adds no card.
	362	expect(getPull(app.db, repo.id, pull.number)!.state).toBe("open");
	363	expect(waitsForPerson(app.db, repo.id, pull)?.why).toBe("rounds");
	364	expect(await sendToReview(app, fresh(), pull)).toBe("held");
	365	await ask("Third again.");
	366	expect(cards().length).toBe(1);
	367	// A person's approval ends the handover.
	368	const ok = await submitReview(app, alice, fresh(), pull, { state: "approved" });
	369	if (!ok.ok) throw new Error(String(ok.error));
	370	expect(handedToPerson(app.db, repo.id, pull)).toBe(false);
	371	});
	372
319	373	test("in off, the verdict queues nothing", async () => {
320	374	const set = await updateRepoSettings(app, alice, fresh(), {
321	375	autonomy: "off",