Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions app/e2e/publish-failed.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { expect, mockOnly, test } from "./fixtures";

// An attempts run whose pick's pull request didn't open (#138): Review's
// Compare says publishing failed, not the agents' work, and offers to retry
// or finish without one. A run on a repository GitHub doesn't host
// finishes with its branch pushed and says so.

test.beforeEach(async ({ app }) => {
mockOnly("the mock's runs");
await app.open();
await app.page.getByTestId("nav-review").click();
});

test("a pull request that didn't open can be retried from Compare, with the pick kept", async ({ app }) => {
const page = app.page;
await page.getByRole("button", { name: "search: 2 attempts · publishing failed" }).click();
const note = page.getByRole("region", { name: "The pull request didn't open" });
await expect(note).toContainText("The agents' work is done and your pick is kept; only publishing failed: gh pr create failed.");
await expect(note).toContainText("HTTP 401: Bad credentials");
// The pick is still the pick, and there is nothing to pick again.
await expect(page.getByText("picked", { exact: true })).toBeVisible();
await expect(page.getByRole("button", { name: "Pick this one" })).toHaveCount(0);
await note.getByRole("button", { name: "Retry publishing" }).click();
await expect(page.getByText("Trying the pull request again")).toBeVisible();
const pr = page.getByRole("region", { name: "Pull request" });
await expect(pr).toContainText("Pull request: opened");
await expect(pr.getByRole("button", { name: "Open pull request" })).toBeVisible();
await expect(note).toHaveCount(0);
});

test("a pull request that didn't open can be skipped: the run finishes without one", async ({ app }) => {
const page = app.page;
await page.getByRole("button", { name: "search: 2 attempts · publishing failed" }).click();
const note = page.getByRole("region", { name: "The pull request didn't open" });
await note.getByRole("button", { name: "Finish without a pull request" }).click();
await expect(page.getByText("The pick is kept, and the other attempts are archived.")).toBeVisible();
await expect(page.getByRole("region", { name: "Pull request" })).toContainText("Finished without a pull request (gh pr create failed).");
await expect(page.getByText(/succeeded · started/)).toBeVisible();
});

test("an attempts run on a Gitea repository says its branch is pushed and links to the forge", async ({ app }) => {
const page = app.page;
await page.getByRole("button", { name: "invoices: 2 attempts" }).click();
const pr = page.getByRole("region", { name: "Pull request" });
await expect(pr).toContainText("This repository is on git.acme.dev, not GitHub, so Shipyard didn't open a pull request.");
await expect(pr.getByRole("button", { name: "Open git.acme.dev" })).toBeVisible();
await expect(page.getByText(/succeeded · started/)).toBeVisible();
});
97 changes: 93 additions & 4 deletions app/src/lib/mock-runs.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { ApiError, type BerthEvent, type Run, type RunCandidate, type RunStep, type RunSummary } from "@/lib/api";

// Runs in the demo: the shop world on devl has a loop going, an attempts run
// of three agents waiting for you to pick one in Review's Compare, a flow's
// run, a review and a finished broadcast. Deciding, cancelling and starting
// of three agents waiting for you to pick one in Review's Compare, one whose
// pick's pull request didn't open (retry, or finish without one), one on a
// Gitea repository that finished with its branch pushed and no pull
// request, a flow's run, a review and a finished broadcast. Deciding, cancelling and starting
// runs work, so the loops panel, Automations → Runs, Compare and the phone
// all have something to show.

Expand Down Expand Up @@ -155,12 +157,81 @@ const gpuAttempts = (): Run => ({
steps: [step("1", "judge", "succeeded", 10, 8, { id: "judge", output: "Winner: attempt 1. passes; adds a migration it doesn't need" })],
});

// An attempts run's candidate in another task's worktrees.
const renamed = (c: RunCandidate, name: string, picked?: boolean): RunCandidate => {
const wt = `${name}-a${c.index + 1}`;
return { ...c, worktree: wt, path: `/home/me/work/shop-${wt}`, branch: wt, session: `${wt}-${c.agent}-x7k2-0i${c.index}0`, picked };
};

// The steps of an attempts run of two that got as far as its pull request.
const pickedSteps = (name: string, start: number, pr: RunStep, tidy?: RunStep): RunStep[] => [
step("0", "map", "succeeded", start, start - 30, { id: "attempts", output: "1. attempt 1 (claude): check passed\n2. attempt 2 (codex): check passed" }),
step("1", "judge", "succeeded", start - 30, start - 31, { id: "judge", output: "Winner: attempt 1. passes with the smaller diff" }),
step("2", "if", "succeeded", start - 31, start - 33, { id: "choose", children: [step("2.t", "then", "", start - 31, start - 33, { children: [step("2.t.0", "gate", "succeeded", start - 31, start - 33, { id: "pick", output: `Pick the best attempt at ${name}` })] })] }),
step("3", "if", pr.status === "failed" ? "running" : "succeeded", start - 33, pr.status === "failed" ? undefined : start - 34, { id: "pr", children: [step("3.t", "then", "", start - 33, undefined, { children: [pr] })] }),
...(tidy ? [tidy] : []),
];

// Picked, but its pull request didn't open (GitHub said no): publishing
// failed, not the agents' work, and it waits to retry or finish without one.
const publishRun = (): Run => ({
id: "r_mshp9pb4x",
template: "attempts",
title: "search: 2 attempts",
status: "waiting_gate",
origin: "laptop:mac",
cursor: "3.t.0",
created: at(60),
updated: at(4),
params: { location: "shop", name: "search", prompt: "Make catalog search fast for 100k products.", then: { pr: { draft: true } } },
gate: {
path: "3.t.0.publish1",
title: "The pull request didn't open",
text: "The agents' work is done and your pick is kept; only publishing failed: gh pr create failed.\n\nHTTP 401: Bad credentials (https://api.github.com/graphql)\nTry authenticating with: gh auth login\n\nRetry once it's fixed, or finish without a pull request: the run goes on and archives the other attempts.",
approve_label: "Retry publishing",
reject_label: "Finish without a pull request",
},
candidates: [
renamed(candidate(0, "claude", true, 64, 12, 1, "passes with the smaller diff", "perf: index the catalog for search"), "search", true),
renamed(candidate(1, "codex", true, 118, 40, 2, "passes, but caches more than it needs to", "perf: cache search results"), "search"),
],
steps: pickedSteps("search", 60, step("3.t.0", "pr", "failed", 26, 25.9, { id: "open", attempt: 1, exit_code: 1, error: "gh pr create failed", output: "HTTP 401: Bad credentials (https://api.github.com/graphql)\nTry authenticating with: gh auth login" })),
});

// Picked on a repository hosted on Gitea: gh can't open a pull request
// there, so the branch is pushed, the step says so, and the run finished.
const giteaRun = (): Run => ({
id: "r_mshp9gt3a",
template: "attempts",
title: "invoices: 2 attempts",
status: "succeeded",
origin: "laptop:mac",
created: at(150),
updated: at(110),
finished: at(110),
params: { location: "shop", name: "invoices", prompt: "Add PDF invoices.", then: { pr: { draft: true } } },
candidates: [
renamed(candidate(0, "claude", true, 88, 6, 1, "passes with a test for the PDF", "feat: PDF invoices"), "invoices", true),
renamed(candidate(1, "codex", false, 140, 20, 2, "its check fails on the totals", "feat: invoices as PDF"), "invoices"),
],
steps: pickedSteps(
"invoices",
150,
step("3.t.0", "pr", "skipped", 116, 115.9, {
id: "open",
output:
"This repository is on git.acme.dev, not GitHub, so Shipyard didn't open a pull request. The branch invoices-a1 is pushed to origin; open one there: https://git.acme.dev/acme/shop/compare/main...invoices-a1. (If git.acme.dev is GitHub Enterprise, run \"gh auth login --hostname git.acme.dev\" on this box.)",
}),
step("4", "if", "succeeded", 115.9, 110, { id: "tidy", children: [step("4.t", "then", "", 115.9, 110, { children: [step("4.t.0", "cleanup", "succeeded", 115.9, 110, { id: "archive", output: "archived invoices-a2" })] })] }),
),
});

let gpuRuns: Run[] | undefined;
let runs: Run[] | undefined;
let round = 2;

function world(): Run[] {
if (!runs) runs = [attemptsRun(), loopRun(round), flowRun(), reviewRun()];
if (!runs) runs = [attemptsRun(), publishRun(), loopRun(round), flowRun(), reviewRun(), giteaRun()];
return runs;
}

Expand Down Expand Up @@ -202,6 +273,23 @@ export function runsCall(box: string, method: string, path: string, body: unknow
const r = all.find((x) => x.id === m![1]);
const d = body as { approve: boolean; pick?: number };
if (!r?.gate) return Promise.reject(new ApiError("that run is not waiting at a gate", 409));
if (r.gate.approve_label) {
// The publish gate: a retry opens it this time; finishing without
// one is a skip. Either way the run archives the rest and is done.
const pr = r.steps.find((s) => s.kind === "if" && s.id === "pr")?.children?.[0]?.children?.[0];
if (pr)
Object.assign(
pr,
d.approve
? { status: "succeeded", attempt: 2, error: undefined, output: "https://github.com/acme/shop/pull/318", ended: new Date().toISOString() }
: { status: "skipped", error: undefined, output: "Finished without a pull request (gh pr create failed).", ended: new Date().toISOString() },
);
const loser = r.candidates?.find((c) => !c.picked)?.worktree;
r.steps.push(step("4", "if", "succeeded", 0, 0, { id: "tidy", children: [step("4.t", "then", "", 0, 0, { children: [step("4.t.0", "cleanup", "succeeded", 0, 0, { id: "archive", output: `archived ${loser}` })] })] }));
Object.assign(r, { status: "succeeded", finished: new Date().toISOString(), updated: new Date().toISOString(), gate: undefined });
emit({ type: "run.finished", box, data: { run: r.id, status: r.status } });
return delay({ decided: r.id, approve: d.approve });
}
const pick = d.pick ?? r.gate.default ?? 0;
for (const c of r.candidates ?? []) c.picked = d.approve && c.index === pick;
Object.assign(r, { status: d.approve ? "succeeded" : "failed", finished: new Date().toISOString(), gate: undefined, error: d.approve ? undefined : "rejected" });
Expand Down Expand Up @@ -248,7 +336,8 @@ export function runsCall(box: string, method: string, path: string, body: unknow
{ path: "src/ledger/entry.ts", code: "M", added: Math.round(c.diff.added * 0.2), removed: Math.round(c.diff.removed * 0.6) },
{ path: "test/refunds.test.ts", code: "M", added: Math.round(c.diff.added * 0.25), removed: Math.round(c.diff.removed * 0.4) },
];
return delay({ run: summary(r), gate: r.gate, judge: r.steps.find((s) => s.kind === "judge")?.output, candidates: (r.candidates ?? []).map((c) => ({ ...c, review: { files: files(c) } })) });
const pr = r.steps.find((s) => s.kind === "if" && s.id === "pr")?.children?.[0]?.children?.[0];
return delay({ run: summary(r), gate: r.gate, judge: r.steps.find((s) => s.kind === "judge")?.output, pr, candidates: (r.candidates ?? []).map((c) => ({ ...c, review: { files: files(c) } })) });
}
if (method === "GET" && path === "autofix") return delay([{ path: "/home/me/work/shop-checkout-fix", ci: true, review: false, max: 3 }]);
if (method === "PUT" && path === "autofix") return delay([body]);
Expand Down
3 changes: 3 additions & 0 deletions app/src/lib/orchestrate-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,9 @@ export interface RunCompare {
run: RunSummary;
gate?: Run["gate"];
judge?: string;
// The pull request step: opened, skipped (a host without them, or
// finished without one) or failed, with what it said.
pr?: Run["steps"][number];
candidates: (NonNullable<Run["candidates"]>[number] & { review?: { files?: { path: string; added?: number; removed?: number; status?: string }[]; commits?: { sha: string; subject: string }[] } })[];
}

Expand Down
10 changes: 6 additions & 4 deletions app/src/views/automations/flows/runs-tab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -320,11 +320,11 @@ export function RunTimeline({ box, id, summary }: { box: string; id: string; sum
</>
) : (
<Button size="xs" onClick={() => void decide(true)}>
Approve
{run.gate.approve_label ?? "Approve"}
</Button>
)}
<Button size="xs" variant="ghost" onClick={() => void decide(false)}>
Reject
<Button size="xs" variant={run.gate.reject_label ? "outline" : "ghost"} onClick={() => void decide(false)}>
{run.gate.reject_label ?? "Reject"}
</Button>
</div>
</div>
Expand Down Expand Up @@ -358,7 +358,9 @@ function Steps({ steps, t0, span, depth }: { steps: RunStep[]; t0: number; span:
return (
<div className={cn("space-y-1", depth > 0 && "border-l pl-3")}>
{steps
.filter((s) => s.status !== "skipped")
// A step its when rule skipped says nothing; one that chose not to
// act (a pull request on Gitea) says why.
.filter((s) => s.status !== "skipped" || s.output)
.map((s) => (
<Step key={s.path} s={s} t0={t0} span={span} depth={depth} />
))}
Expand Down
73 changes: 70 additions & 3 deletions app/src/views/review/compare-view.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ArrowLeftIcon, CheckIcon, Columns2Icon, CrownIcon, GitCompareArrowsIcon, SquareTerminalIcon, XIcon } from "lucide-react";
import { ArrowLeftIcon, CheckIcon, Columns2Icon, CrownIcon, ExternalLinkIcon, GitCompareArrowsIcon, GitPullRequestIcon, RotateCcwIcon, SquareTerminalIcon, XIcon } from "lucide-react";
import { useEffect, useState } from "react";

import { AgentIcon } from "@/components/agent-glyph";
Expand All @@ -16,6 +16,7 @@ import { openCompare } from "@/lib/compare-actions";
import { usePrefs } from "@/lib/prefs";
import { tokens } from "@/views/automations/flows/runs-tab";
import { ErrorText } from "@/components/error-note";
import { openUrl } from "@/lib/open-url";

// CompareStrip lists attempts runs with candidates to compare, at the top of
// Review: the ones waiting for a pick first.
Expand All @@ -28,13 +29,60 @@ export function CompareStrip({ runs, onOpen }: { runs: BoxRun[]; onOpen(r: BoxRu
{runs.map((r) => (
<Button key={`${r.box}/${r.id}`} size="xs" variant={r.status === "waiting_gate" ? "default" : "outline"} onClick={() => onOpen(r)}>
{r.title ?? r.id}
{r.status === "waiting_gate" && " · pick one"}
{r.status === "waiting_gate" && (r.gate?.approve_label ? " · publishing failed" : r.gate && !r.gate.pick ? " · needs you" : " · pick one")}
</Button>
))}
</div>
);
}

// PullRequestNote says what became of the pick's pull request: that it
// didn't open (publishing failed, not the agents' work), with retry and
// finish without one; or, once the run is past it, what the step said (on
// Gitea, say, the branch is pushed and the link is the forge's own).
function PullRequestNote({ data, where, busy, onPublish }: { data: RunCompare; where?: string; busy: boolean; onPublish(retry: boolean): void }) {
const gate = data.gate;
if (gate && !gate.pick && gate.approve_label) {
return (
<section aria-label={gate.title} className="mb-4 rounded-xl border border-warning/40 bg-warning/8 px-4 py-2.5 text-body">
<p className="flex items-center gap-1.5 font-medium">
<GitPullRequestIcon className="size-3.5 shrink-0 text-warning-foreground" />
{gate.title}
{where && <span className="font-normal text-muted-foreground">on {where}</span>}
</p>
{gate.text && <p className="mt-1 whitespace-pre-wrap text-desc text-muted-foreground">{gate.text}</p>}
<div className="mt-2 flex flex-wrap items-center gap-1.5">
<Button size="xs" disabled={busy} onClick={() => onPublish(true)}>
<RotateCcwIcon />
{gate.approve_label}
</Button>
<Button size="xs" variant="outline" disabled={busy} onClick={() => onPublish(false)}>
{gate.reject_label}
</Button>
</div>
</section>
);
}
const pr = data.pr;
if (gate || !pr?.output || (pr.status !== "succeeded" && pr.status !== "skipped")) return null;
const link = /https?:\/\/[^\s'"<>)]+/.exec(pr.output)?.[0]?.replace(/[.,;]+$/, "");
return (
<section aria-label="Pull request" className="mb-4 flex flex-wrap items-center gap-x-3 gap-y-1.5 rounded-xl border bg-muted/30 px-4 py-2.5 text-body">
<GitPullRequestIcon className="size-3.5 shrink-0 text-muted-foreground" />
<span className="min-w-0 flex-1">
<span className="font-medium">Pull request{where ? ` on ${where}` : ""}: </span>
<span className="text-muted-foreground">{pr.status === "succeeded" ? "opened" : pr.output}</span>
</span>
{link && (
<Button size="xs" variant="outline" onClick={() => void openUrl(link)}>
<ExternalLinkIcon />
{pr.status === "succeeded" ? "Open pull request" : `Open ${new URL(link).host}`}
</Button>
)}
</section>
);
}

// CompareView is Review's Compare mode for an attempts run: a column per
// attempt, with its check, diffstat, commits, the judge's ranking and
// reasons, and the files it changed. Picking one decides the run's gate;
Expand Down Expand Up @@ -69,7 +117,7 @@ export function CompareView({ box, id, onClose }: { box: string; id: string; onC
setBusy(true);
try {
for (const r of all) {
if (!r.data.gate) continue;
if (!r.data.gate?.pick) continue;
const mine = at && r.box === at.box && r.id === at.id;
await runsApi.decide(r.box, r.id, approve ? { approve: true, pick: mine ? at.index : -1 } : { approve: false });
scheduleRuns(r.box, 0);
Expand All @@ -83,6 +131,22 @@ export function CompareView({ box, id, onClose }: { box: string; id: string; onC
}
};

// A pull request that didn't open waits at its own gate: the pick is in,
// so this only publishes again or lets the run finish without one.
const publish = async (r: (typeof all)[number], retry: boolean) => {
setBusy(true);
try {
await runsApi.decide(r.box, r.id, { approve: retry });
scheduleRuns(r.box, 0);
toastManager.add({ type: "success", title: retry ? "Trying the pull request again" : "Finished without a pull request", description: retry ? undefined : "The pick is kept, and the other attempts are archived." });
await load();
} catch (err) {
toastManager.add({ type: "error", title: "Couldn't decide", description: errorMessage(err) });
} finally {
setBusy(false);
}
};

const data = all.find((r) => r.box === box && r.id === id)?.data ?? all[0]?.data;
const canPick = all.some((r) => r.data.gate?.pick);
const multi = all.length > 1;
Expand Down Expand Up @@ -147,6 +211,9 @@ export function CompareView({ box, id, onClose }: { box: string; id: string; onC
)}
{data && (
<div className="min-h-0 flex-1 overflow-auto p-6">
{all.map((r) => (
<PullRequestNote key={`${r.box}/${r.id}`} data={r.data} where={multi ? r.box : undefined} busy={busy} onPublish={(retry) => void publish(r, retry)} />
))}
{all
.filter((r) => r.data.judge)
.map((r) => (
Expand Down
Loading
Loading