trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns13g rEventsFlows

When a new forge's crew cannot continue on a green pull, the pull and app page say why and offer Land it as it is #11901

scoutcrew opened this

Check · Review · Land · Live — Its check is queued. Next: reviewer, no review yet.

ConversationFiles
Ready to land

scout/forge:scout/11892-when-a-new-forge-s-crew-cannot-continue → main 2 commits fork

Review

Sign in to review this change.
7 files changed +258 −3
docs/features/land-as-is.md added +8
@@ -0,0 +1,8 @@
1 # Land it as it is
2
3 When the crew cannot go on (its included credit is spent, or it has no model key) a green pull still goes live.
4
5 - The pull page and the app's page each say so in one sentence, and offer **Land it as it is** with the preview link beside it, and **Add a key**.
6 - The button is the pull's own ship action (`POST /:owner/:name/pulls/:n/ship`, same-origin checked, `mergePull` behind it). It is offered only for a pull whose check is green and whose verdict is "Ready to land"; a red pull never shows it.
7 - MCP: the existing `merge` tool takes `action: "land"` (squash, only while the check is green); its plan carries `crew` with the same sentence.
8 - Code: `src/web/studio/land_as_is.ts`. Test: `test/land-as-is.test.ts`.
src/http/mcp/tools/merge.ts +11 −3
@@ -3,6 +3,7 @@ import { mergePull } from "../../../actions/pulls.ts";
33 import { getPull, pullDir, pullMergeState } from "../../../core/pulls.ts";
44 import { checkSummary } from "../../../core/runs.ts";
55 import { mergeFailure } from "../../../web/pages/pull.ts";
6 import { LAND_AS_IS, stalledWords } from "../../../web/studio/land_as_is.ts";
67 import { good, bad, type Tool, REPO_ARG, repoArg, isAnswer } from "../shared.ts";
78
89 /**
@@ -13,7 +14,7 @@ import { good, bad, type Tool, REPO_ARG, repoArg, isAnswer } from "../shared.ts"
1314 */
1415 export const tool: Tool = {
1516 name: "merge",
16 doc: "`{ repo, number, strategy?, dryRun? }` (a person, never an agent: a person merges): land a pull as its page's Merge button does, `merge` (default) or `squash`; planned unless `dryRun` is false: the plan is the pull's `head`, `base`, `mergeable`, `conflicts` and `check` as `pull` reads them; landing is `mergePull`'s, with its refusals (conflict, review or check required, not a writer) in the page's words, and takes an rw token",
17 doc: "`{ repo, number, strategy?, action?, dryRun? }` (a person, never an agent: a person merges): land a pull as its page's Merge button does, `merge` (default) or `squash`; `action: \"land\"` is the page's Land it as it is (squash, and only while the check is green); plans carry `crew` when the crew cannot go on; planned unless `dryRun` is false: the plan is the pull's `head`, `base`, `mergeable`, `conflicts` and `check` as `pull` reads them; landing is `mergePull`'s, with its refusals (conflict, review or check required, not a writer) in the page's words, and takes an rw token",
1718 description: "Land a pull, as its page's Merge button does; a person's only.",
1819 inputSchema: {
1920 type: "object",
@@ -21,6 +22,7 @@ export const tool: Tool = {
2122 repo: REPO_ARG,
2223 number: { type: "integer", minimum: 1 },
2324 strategy: { type: "string", enum: ["merge", "squash"] },
25 action: { type: "string", enum: ["land"] },
2426 dryRun: { type: "boolean" },
2527 },
2628 required: ["repo", "number"],
@@ -35,7 +37,9 @@ export const tool: Tool = {
3537 const s = args["strategy"];
3638 if (s !== undefined && s !== "merge" && s !== "squash")
3739 return bad("invalid", "strategy is merge or squash");
38 const strategy = s === "squash" ? "squash" : "merge";
40 const act = args["action"];
41 if (act !== undefined && act !== "land") return bad("invalid", "action is land");
42 const strategy = s === "squash" || act === "land" ? "squash" : "merge";
3943 const n = args["number"];
4044 const pull =
4145 typeof n === "number" && Number.isInteger(n) && n > 0
@@ -43,6 +47,7 @@ export const tool: Tool = {
4347 : undefined;
4448 if (pull === undefined || !mayView(ctx.app.db, repo, pull, user))
4549 return bad("no_such_pull", `no pull #${String(n)}`);
50 const stalled = stalledWords(ctx.app);
4651 const url = `/${repo.ownerName}/${repo.name}/pulls/${pull.number}`;
4752 if (args["dryRun"] !== false) {
4853 // The `pull` tool's reads, so the plan and the reading never disagree.
@@ -60,9 +65,12 @@ export const tool: Tool = {
6065 conflicts: state?.conflicts ?? [],
6166 check: { state: check.state, baseOid: check.run?.baseOid ?? "" },
6267 url,
68 ...(stalled === null ? {} : { crew: stalled }),
6369 says: "This is the plan; nothing changed. Send dryRun: false to merge.",
6470 });
6571 }
72 if (act === "land" && checkSummary(ctx.app.db, pull.id, pull.headOid).state !== "ok")
73 return bad("check_required", `${LAND_AS_IS} waits for a green check`);
6674 const merged = await mergePull(ctx.app, user, repo, pull, strategy);
6775 if (!merged.ok) {
6876 const error = merged.error;
@@ -80,7 +88,7 @@ export const tool: Tool = {
8088 strategy,
8189 mergeOid: merged.value.mergeOid,
8290 url,
83 says: `Merged #${pull.number} into ${pull.baseBranch}.`,
91 says: act === "land" ? `Landed #${pull.number} as it is.` : `Merged #${pull.number} into ${pull.baseBranch}.`,
8492 });
8593 },
8694 };
src/web/pages/pull.ts +4
@@ -32,6 +32,7 @@ import { tierLine, type TierState } from "../../checks/tiers.ts";
3232 import { fromKeptFork } from "../../core/landing_kind.ts";
3333 import { combinedCard } from "./pull-combined.ts";
3434 import { verdictLine, verdictOf, type VerdictInput } from "./pull-verdict.ts";
35 import { landAsIs } from "../studio/land_as_is.ts";
3536 import { getAppByRepo } from "../../core/apps.ts";
3637 import { labelsFor, listLabels } from "../../core/issues.ts";
3738 import { marksByName } from "../../core/marks.ts";
@@ -1110,6 +1111,9 @@ export async function pullFilesPage(ctx: Ctx): Promise<Response> {
11101111 ${changeHead(ctx, repo, pull, { headOid: status.headOid, conflicts: status.conflicts.length })}
11111112 ${changeTabs(repo, pull, "files")}
11121113 ${verdictLine(verdict)}
1114 ${verdict.tone === "success" && mayMerge && check.state === "ok" && !stale
1115 ? landAsIs(ctx, repo, pull, isApp ? `${repoBase(repo)}/app/pull-${pull.number}/open` : undefined)
1116 : null}
11131117 <p class="files-where muted small" id="files">
11141118 <span class="mono">${headLabel} → ${pull.baseBranch}</span>
11151119 <span class="sep" aria-hidden="true">·</span>
src/web/studio/app.ts +3
@@ -65,6 +65,7 @@ import { schedulesCard } from "./schedules.ts";
6565 import { appPanels } from "./previews.ts";
6666 import { noCrewLine, pausedWords, servingHistory, type WordCtx } from "./words.ts";
6767 import { crewKeyCard, includedCredit } from "./crew_key.ts";
68 import { landOnApp } from "./land_as_is.ts";
6869 import { usdWords } from "../../spawn/credit.ts";
6970 import {
7071 notePhase,
@@ -510,6 +511,7 @@ export async function studioApp(
510511 : followingCard(ctx, repo, following)}
511512 <div id="app-head">${head}</div>
512513 ${ownerFacts ? crewKeyCard(ctx, true) : null}
514 ${ownerFacts ? landOnApp(ctx, repo) : null}
513515 <div class="stack">
514516 ${adapter}
515517 ${schedules}
@@ -531,6 +533,7 @@ export async function studioApp(
531533 ${askingLine(ctx, repo)}
532534 ${policy}
533535 ${ownerFacts ? crewKeyCard(ctx, true) : null}
536 ${ownerFacts ? landOnApp(ctx, repo) : null}
534537 ${ownerFacts
535538 ? egressNote(
536539 ctx.app.config.dataDir,
src/web/studio/change.ts +4
@@ -66,6 +66,7 @@ import {
6666 import { previewBlock } from "../pages/apps.ts";
6767 import { lookCard } from "../pages/looks.ts";
6868 import { approveHereCard, keptChange, pullChecksLineFor, teamsCard, verdictFacts } from "../pages/pull.ts";
69 import { landAsIs } from "./land_as_is.ts";
6970 import { changeSummary, verdictLine, verdictOf } from "../pages/pull-verdict.ts";
7071 import { guestWalls, strangerCard } from "../pages/pull-guest.ts";
7172 import { combinedCard } from "../pages/pull-combined.ts";
@@ -757,6 +758,9 @@ async function changeConversation(
757758 )}
758759 <div id="pull-check">${checkLine}</div>
759760 ${when(runLine, (l) => l)} ${gate} ${decisionSection}
761 ${verdict.tone === "success" && writer && word === "look"
762 ? landAsIs(ctx, repo, pull, record === undefined ? undefined : `${base}/app/pull-${pull.number}/open`)
763 : null}
760764 ${when(visionPreview, (v) => visionPreviewCard(v))}
761765 ${look}
762766 <div class="split">
src/web/studio/land_as_is.ts added +66
@@ -0,0 +1,66 @@
1 /**
2 * Land it as it is (#11892): when the crew cannot go on (its included credit
3 * is spent, or it has no model key) a green pull still lands. One sentence
4 * says why the crew stopped; two actions follow: land the pull as it is, with
5 * the preview beside it, and add a key. The button is the pull's own ship
6 * action (`POST …/pulls/:n/ship`, same-origin checked, `mergePull` behind
7 * it), so a red or held pull is never landed from here: it is only offered
8 * for a pull whose verdict is already "Ready to land".
9 */
10
11 import type { App } from "../../app.ts";
12 import type { Pull } from "../../core/pulls.ts";
13 import type { Repo } from "../../core/repos.ts";
14 import type { Ctx } from "../../http/context.ts";
15 import { repoBase } from "../chrome.ts";
16 import { html, when, type Html } from "../html.ts";
17 import { btn } from "../ui.ts";
18 import { crewCanRun, hasCrew, includedCredit } from "./crew_key.ts";
19 import { decisionsFor } from "./decisions.ts";
20 import { changeHref } from "./nouns.ts";
21
22 /** The one label, on the page and in the MCP tool's `action`. */
23 export const LAND_AS_IS = "Land it as it is";
24 export const ADD_KEY_HREF = "/settings/subscriptions";
25
26 /** Why the crew cannot go on, in one plain sentence; null while it can, or when there is no crew to stop. */
27 export function stalledWords(app: App): string | null {
28 if (!hasCrew(app) || crewCanRun(app)) return null;
29 return includedCredit(app) !== undefined
30 ? "The crew cannot go on: the included credit is spent."
31 : "The crew cannot go on: it has no model key.";
32 }
33
34 /** The card for one green pull: the sentence, the button with the preview beside it, and the way to add a key. */
35 export function landAsIs(ctx: Ctx, repo: Repo, pull: Pull, preview: string | undefined): Html | null {
36 const words = stalledWords(ctx.app);
37 if (words === null || pull.state !== "open" || pull.mergedAt !== null) return null;
38 const thread = `${repoBase(repo)}/pulls/${pull.number}`;
39 const admin = ctx.auth.user?.isAdmin === true;
40 return html`<div class="mt mb" id="land-as-is" data-land-as-is role="status">
41 <p>${words} This pull is ready, so you can put it live as it is.</p>
42 <div class="bar mt">
43 <form method="post" action="${thread}/ship" data-now="Landing">
44 ${btn(LAND_AS_IS, { size: "sm", variant: "primary", icon: "merge" })}
45 </form>
46 ${when(preview, (href) => btn("Preview", { href, size: "sm" }))}
47 ${admin ? btn("Add a key", { href: ADD_KEY_HREF, size: "sm" }) : html`<span class="muted small">Ask this forge's admin to add a key.</span>`}
48 </div>
49 </div>`;
50 }
51
52 /** The app's page: the same sentence and button for each pull of this app that is green and waiting on the viewer. Reads nothing while the crew can go on. */
53 export function landOnApp(ctx: Ctx, repo: Repo): Html | null {
54 if (stalledWords(ctx.app) === null) return null;
55 const green = decisionsFor(ctx, [repo], { evenDismissed: true }).filter(
56 (d) => d.kind === "look" && d.pull !== undefined && d.pull.state === "open",
57 );
58 if (green.length === 0) return null;
59 return html`${green.map((d) => {
60 const pull = d.pull!;
61 return html`<div class="mt">
62 <p class="small"><a href="${changeHref(repoBase(repo), pull.number)}">${pull.title}</a></p>
63 ${landAsIs(ctx, repo, pull, d.preview)}
64 </div>`;
65 })}`;
66 }
test/land-as-is.test.ts added +162
@@ -0,0 +1,162 @@
1 /**
2 * Land it as it is (#11892): credit runs out after a pull went green; the
3 * pull page and the app page say so in one sentence and offer the button,
4 * and pressing it lands the pull. A red pull is never offered it.
5 */
6 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
7 import { writeFileSync } from "node:fs";
8 import { mkdtemp, rm } from "node:fs/promises";
9 import { tmpdir } from "node:os";
10 import { join } from "node:path";
11 import { createApp, type App } from "../src/app.ts";
12 import { enableApp } from "../src/actions/apps.ts";
13 import { addToCrew, createAgent } from "../src/actions/crew.ts";
14 import { openPull } from "../src/actions/pulls.ts";
15 import { commitToBranch, createRepo } from "../src/actions/repos.ts";
16 import { getPull, type Pull } from "../src/core/pulls.ts";
17 import type { Repo } from "../src/core/repos.ts";
18 import { claimRun, createRun, finishRun } from "../src/core/runs.ts";
19 import { createUser, type User } from "../src/core/users.ts";
20 import { serve, type Servers } from "../src/http/server.ts";
21 import { STRANGER_NEVER_SEES } from "../src/design/first_five_minutes.ts";
22 import { LAND_AS_IS } from "../src/web/studio/land_as_is.ts";
23
24 describe("land it as it is", () => {
25 let dataDir = "";
26 let app: App;
27 let servers: Servers;
28 let base = "";
29 let alice: User;
30 let repo: Repo;
31 let cookie = "";
32 let n = 0;
33
34 const get = (path: string) =>
35 fetch(`${base}${path}`, { headers: { cookie: `${cookie}; forge_mode=studio` }, redirect: "manual" }).then((r) => r.text());
36
37 const pullWith = async (state: "ok" | "failed"): Promise<Pull> => {
38 n += 1;
39 const branch = await commitToBranch(app, alice, repo, {
40 branch: `feature/b${n}`,
41 base: "main",
42 message: `Change ${n}`,
43 files: [{ path: `F${n}.md`, content: `${n}\n` }],
44 });
45 if (!branch.ok) throw new Error(JSON.stringify(branch.error));
46 const opened = await openPull(app, alice, repo, {
47 title: `Change ${n}`,
48 body: "A change.",
49 baseBranch: "main",
50 headBranch: `feature/b${n}`,
51 });
52 if (!opened.ok) throw new Error(String(opened.error));
53 const pull = opened.value;
54 const run = createRun(app.db, {
55 repoId: repo.id,
56 kind: "check",
57 issueId: pull.id,
58 env: `pull-${pull.number}`,
59 commitOid: pull.headOid,
60 reason: "pull",
61 });
62 claimRun(app.db, run.id);
63 finishRun(app.db, run.id, { state, summary: state === "ok" ? "check passed in 1s" : "check failed" });
64 return pull;
65 };
66
67 const spendCredit = () =>
68 writeFileSync(
69 join(dataDir, "parent.json"),
70 JSON.stringify({ credit: { token: "bearer-token-123", capUsd: 5, spentUsd: 5 } }),
71 );
72
73 beforeAll(async () => {
74 dataDir = await mkdtemp(join(tmpdir(), "forge-land-as-is-"));
75 app = createApp({ dataDir, port: 0 });
76 servers = await serve(app);
77 base = servers.url;
78 const a = await createUser(app.db, {
79 origin: "seed", trusted: true, username: "alice", email: "[email protected]",
80 password: "a-good-passphrase", adminIfFirst: true,
81 });
82 if (!a.ok) throw new Error(a.error);
83 alice = a.value;
84 cookie = (
85 (await fetch(`${base}/signin`, {
86 method: "POST",
87 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
88 redirect: "manual",
89 })).headers.get("set-cookie") ?? ""
90 ).split(";")[0]!;
91 const made = await createRepo(app, alice, { name: "garden", visibility: "public", description: "d" });
92 if (!made.ok) throw new Error(String(made.error));
93 repo = made.value;
94 const seeded = await commitToBranch(app, alice, repo, {
95 message: "Start",
96 files: [
97 { path: "package.json", content: '{"name":"garden","scripts":{"start":"bun run server.ts"}}\n' },
98 { path: "server.ts", content: "Bun.serve({ port: Number(process.env.PORT), fetch: () => new Response('ok') });\n" },
99 ],
100 });
101 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
102 const enabled = await enableApp(app, alice, repo, { slug: "garden" });
103 if (!enabled.ok) throw new Error(String(enabled.error));
104 const agent = await createAgent(app, alice, { username: "keeper" });
105 if (!agent.ok) throw new Error(String(agent.error));
106 expect(addToCrew(app, alice, repo, "keeper").ok).toBe(true);
107 });
108
109 afterAll(async () => {
110 servers.stop();
111 app.close();
112 await rm(dataDir, { recursive: true, force: true });
113 });
114
115 test("while the crew can go on, nothing is offered", async () => {
116 // No credit file and no key: the crew has nothing to run on, so seed a key-less forge with credit left.
117 writeFileSync(join(dataDir, "parent.json"), JSON.stringify({ credit: { token: "bearer-token-123", capUsd: 5, spentUsd: 1 } }));
118 const pull = await pullWith("ok");
119 const page = await get(`/alice/garden/pulls/${pull.number}`);
120 expect(page).not.toContain("data-land-as-is");
121 expect(await get(`/alice/garden/pulls/${pull.number}/files`)).not.toContain("data-land-as-is");
122 });
123
124 test("credit runs out after a green pull: the sentence, the button, the preview and the key on both pages; pressing it lands", async () => {
125 const pull = await pullWith("ok");
126 spendCredit();
127 const sentence = "The crew cannot go on: the included credit is spent.";
128 for (const path of [`/alice/garden/pulls/${pull.number}`, `/alice/garden/pulls/${pull.number}/files`, "/alice/garden"]) {
129 const page = await get(path);
130 expect(page).toContain(sentence);
131 expect(page).toContain(LAND_AS_IS);
132 expect(page).toContain(`action="/alice/garden/pulls/${pull.number}/ship"`);
133 expect(page).toContain(`/alice/garden/app/pull-${pull.number}/open`);
134 expect(page).toContain('href="/settings/subscriptions"');
135 }
136 const card = (await get(`/alice/garden/pulls/${pull.number}`)).split("data-land-as-is")[1]!.split("</div>")[1]!.toLowerCase();
137 for (const word of STRANGER_NEVER_SEES) expect(card.includes(word)).toBe(false);
138
139 const pressed = await fetch(`${base}/alice/garden/pulls/${pull.number}/ship`, {
140 method: "POST",
141 headers: { cookie, origin: "https://evil.example" },
142 redirect: "manual",
143 });
144 expect(pressed.status).toBe(403); // same-origin check: a foreign origin lands nothing
145 expect(getPull(app.db, repo.id, pull.number)?.mergedAt).toBeNull();
146 const landed = await fetch(`${base}/alice/garden/pulls/${pull.number}/ship`, {
147 method: "POST",
148 headers: { cookie, origin: base },
149 redirect: "manual",
150 });
151 expect(landed.status).toBe(303);
152 expect(getPull(app.db, repo.id, pull.number)?.mergedAt).not.toBeNull();
153 });
154
155 test("a red pull is never offered it", async () => {
156 spendCredit();
157 const pull = await pullWith("failed");
158 const page = await get(`/alice/garden/pulls/${pull.number}`);
159 expect(page).not.toContain("data-land-as-is");
160 expect(await get(`/alice/garden/pulls/${pull.number}/files`)).not.toContain("data-land-as-is");
161 });
162 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs