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

The studio can look at your change but cannot ship it #225

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

scout/forge:scout/211-the-studio-can-look-at-your-change-but-c → main 1 commit fork

  • Merged. scout/211-the-studio-can-look-at-your-change-but-c was merged into main as 04930ac.

Review

Sign in to review this change.
2 files changed +256 −15
src/web/studio/change.ts +59 −15
@@ -10,7 +10,7 @@
1010 import { appsRuntime } from "../../apps/runtime.ts";
1111 import type { AppRecord } from "../../core/apps.ts";
1212 import { pullStatus, type Pull } from "../../core/pulls.ts";
13 import type { Repo } from "../../core/repos.ts";
13 import { canWrite, type Repo } from "../../core/repos.ts";
1414 import { checkSummary } from "../../core/runs.ts";
1515 import { repoDir } from "../../git/repos.ts";
1616 import type { Ctx } from "../../http/context.ts";
@@ -31,7 +31,12 @@ import { previewCard } from "../pages/apps.ts";
3131 import { lookCard } from "../pages/looks.ts";
3232 import { pullState } from "../pages/pulls.ts";
3333 import { composer, conversation, originGlyph } from "../pages/thread.ts";
34 import { decisionActions, decisionForm, decisionsFor } from "./asks.ts";
34 import {
35 decisionActions,
36 decisionForm,
37 decisionsFor,
38 type Decision,
39 } from "./asks.ts";
3540 import { pullWord, wordBadge, type WordCtx } from "./words.ts";
3641
3742 /** What a conflicted change's own page says: the base it no longer merges
@@ -99,9 +104,43 @@ export async function studioPull(
99104 if (production?.runId)
100105 deploy = { id: production.runId, commitOid: production.commitOid };
101106 }
102 const decision = decisionsFor(ctx, [repo]).find(
107 const crewDecision = decisionsFor(ctx, [repo]).find(
103108 (d) => d.pull?.id === pull.id,
104109 );
110 // A person's own change carries no crew decision (`decisionsFor` reads
111 // agent pulls only — #208's territory), but the change's own page still
112 // owes its writer the same route and the same words, whoever opened it.
113 // Ready to offer means: nothing to fix first (or it does, and that fix is
114 // the conflict wording below), and shipping would not be refused for want
115 // of a green check under this repository's own setting (`requireChecks`,
116 // the same flag `shipSubmit` reads).
117 const writer = ctx.auth.user !== undefined && canWrite(db, repo, ctx.auth.user);
118 const openForOwner = writer && status !== undefined;
119 const ownReady =
120 crewDecision === undefined &&
121 openForOwner &&
122 (conflicted || word === "look" || !repo.requireChecks);
123 const own: Decision | undefined = ownReady
124 ? {
125 kind: "look",
126 id: `own:${pull.id}`,
127 repo,
128 pull,
129 who: pull.authorName,
130 says:
131 word === "look"
132 ? "The check is green. Look at the preview, then ship it or say what is off."
133 : "This app does not wait on the check to ship. Look at the preview, then ship it or say what is off.",
134 short: word === "look" ? "the check is green" : "ready to ship",
135 }
136 : undefined;
137 const decision = crewDecision ?? own;
138 // Nothing to offer yet, and nothing wrong either — the check just has not
139 // gone green, and this repository waits for that before it ships.
140 const ownNotReadyNote =
141 crewDecision === undefined && openForOwner && !conflicted && !ownReady
142 ? note
143 : undefined;
105144
106145 const head = html`<div class="issue-head">
107146 <h1>${pull.title} <span class="number">#${pull.number}</span></h1>
@@ -141,11 +180,13 @@ export async function studioPull(
141180 // it would only be refused. The button that helps posts to the very same
142181 // route (`/ship`): `shipSubmit` already turns a conflict there into the
143182 // crew bringing the branch up to date, so nothing new is written, only said
144 // sooner and in plain words.
145 const decisionSection = when(decision, (d) =>
146 card(
147 conflicted && d.kind === "look" && d.pull
148 ? html`<p>${d.who} finished "${d.pull.title}", but ${conflictWords(
183 // sooner and in plain words. The same wording and the same button serve a
184 // person's own change (`own`, above) exactly as they serve the crew's — one
185 // card, one vocabulary, whoever opened the change.
186 const decisionBody =
187 decision !== undefined
188 ? conflicted && decision.kind === "look" && decision.pull
189 ? html`<p>${decision.who} finished "${decision.pull.title}", but ${conflictWords(
149190 pull.baseBranch,
150191 status!.conflicts,
151192 )}.</p>
@@ -154,13 +195,16 @@ export async function studioPull(
154195 ${btn("Bring up to date", { size: "sm", variant: "primary" })}
155196 </form>
156197 </div>`
157 : html`<p>${d.says}</p>
158 <div class="bar mt">${decisionActions(ctx, d)}</div>`,
159 {
160 head: html`${icon("alert")}
161 <h2>Decide</h2>`,
162 },
163 ),
198 : html`<p>${decision.says}</p>
199 <div class="bar mt">${decisionActions(ctx, decision)}</div>`
200 : ownNotReadyNote !== undefined
201 ? html`<p class="muted small">Not ready to ship — ${ownNotReadyNote}.</p>`
202 : undefined;
203 const decisionSection = when(decisionBody, (body) =>
204 card(body, {
205 head: html`${icon("alert")}
206 <h2>Decide</h2>`,
207 }),
164208 );
165209
166210 const talk = conversation(ctx, repo, pull, { openVerb: "opened this" });
test/studio.test.ts +197
@@ -4574,6 +4574,203 @@ describe("a stranger sees the app, not the owner's books", () => {
45744574 });
45754575 });
45764576
4577 /**
4578 * Issue #211: the studio's own Decide card ships a person's own change too,
4579 * not only the crew's — the same `/ship` route, the same conflict wording
4580 * (`change.ts:145-157` before this), offered to a writer, whoever opened it.
4581 */
4582 describe("a person's own change ships from its own studio page", () => {
4583 let dataDir: string;
4584 let app: App;
4585 let servers: Servers;
4586 let base: string;
4587 let alice: User;
4588 let repo: Repo;
4589 let ownerCookie: string;
4590 let readerCookie: string;
4591
4592 const signIn = async (username: string): Promise<string> =>
4593 (
4594 (
4595 await fetch(`${base}/signin`, {
4596 method: "POST",
4597 body: new URLSearchParams({
4598 username,
4599 password: "a-good-passphrase",
4600 }),
4601 redirect: "manual",
4602 })
4603 ).headers.get("set-cookie") ?? ""
4604 ).split(";")[0]!;
4605
4606 const get = (path: string, cookie: string) =>
4607 fetch(`${base}${path}`, {
4608 headers: { cookie: `${cookie}; forge_mode=studio` },
4609 redirect: "manual",
4610 });
4611 const post = (path: string, cookie: string) =>
4612 fetch(`${base}${path}`, {
4613 method: "POST",
4614 headers: { cookie, origin: base },
4615 redirect: "manual",
4616 });
4617
4618 beforeAll(async () => {
4619 dataDir = await mkdtemp(join(tmpdir(), "forge-own-ship-"));
4620 app = createApp({ dataDir, port: 0 });
4621 servers = await serve(app);
4622 base = servers.url;
4623 const a = await createUser(app.db, {
4624 username: "alice",
4625 email: "[email protected]",
4626 password: "a-good-passphrase",
4627 adminIfFirst: true,
4628 });
4629 if (!a.ok) throw new Error(a.error);
4630 alice = a.value;
4631 const r = await createUser(app.db, {
4632 username: "eve",
4633 email: "[email protected]",
4634 password: "a-good-passphrase",
4635 });
4636 if (!r.ok) throw new Error(r.error);
4637 ownerCookie = await signIn("alice");
4638 readerCookie = await signIn("eve");
4639 const made = await createRepo(app, alice, {
4640 name: "garden",
4641 visibility: "public",
4642 description: "grown by hand, not by the crew",
4643 });
4644 if (!made.ok) throw new Error(String(made.error));
4645 repo = made.value;
4646 const seeded = await commitToBranch(app, alice, repo, {
4647 message: "Start",
4648 files: [
4649 {
4650 path: "package.json",
4651 content:
4652 '{"name":"garden","scripts":{"start":"bun run server.ts"}}\n',
4653 },
4654 {
4655 path: "server.ts",
4656 content:
4657 "Bun.serve({ port: Number(process.env.PORT), fetch: () => new Response('ok') });\n",
4658 },
4659 ],
4660 });
4661 if (!seeded.ok) throw new Error(JSON.stringify(seeded.error));
4662 const enabled = await enableApp(app, alice, repo, { slug: "garden" });
4663 if (!enabled.ok) throw new Error(String(enabled.error));
4664 });
4665
4666 afterAll(async () => {
4667 servers.stop();
4668 app.close();
4669 await rm(dataDir, { recursive: true, force: true });
4670 });
4671
4672 test("a green change alice opened herself offers Ship to a writer, and nothing to a reader; shipping lands it and the page reads shipped", async () => {
4673 const branch = await commitToBranch(app, alice, repo, {
4674 branch: "feature/bench",
4675 base: "main",
4676 message: "Add a bench",
4677 files: [{ path: "BENCH.md", content: "a place to sit\n" }],
4678 });
4679 if (!branch.ok) throw new Error(JSON.stringify(branch.error));
4680 const opened = await openPull(app, alice, repo, {
4681 title: "Add a bench",
4682 body: "For sitting.",
4683 baseBranch: "main",
4684 headBranch: "feature/bench",
4685 });
4686 if (!opened.ok) throw new Error(String(opened.error));
4687 const pull = opened.value;
4688 const run = createRun(app.db, {
4689 repoId: repo.id,
4690 kind: "check",
4691 issueId: pull.id,
4692 env: `pull-${pull.number}`,
4693 commitOid: pull.headOid,
4694 reason: "pull",
4695 });
4696 claimRun(app.db, run.id);
4697 finishRun(app.db, run.id, { state: "ok", summary: "check passed in 1s" });
4698
4699 const readerPage = await (
4700 await get(`/alice/garden/pulls/${pull.number}`, readerCookie)
4701 ).text();
4702 expect(readerPage).toContain('data-word="look"');
4703 expect(readerPage).not.toContain("<h2>Decide</h2>");
4704 expect(readerPage).not.toContain("Ship");
4705
4706 const ownerPage = await (
4707 await get(`/alice/garden/pulls/${pull.number}`, ownerCookie)
4708 ).text();
4709 expect(ownerPage).toContain("<h2>Decide</h2>");
4710 expect(ownerPage).toContain("Ship");
4711 expect(ownerPage).toContain("The check is green.");
4712
4713 const shipped = await post(
4714 `/alice/garden/pulls/${pull.number}/ship`,
4715 ownerCookie,
4716 );
4717 expect(shipped.status).toBe(303);
4718 expect(getPull(app.db, repo.id, pull.number)?.mergedAt).not.toBeNull();
4719 const after = await (
4720 await get(`/alice/garden/pulls/${pull.number}`, ownerCookie)
4721 ).text();
4722 expect(after).toContain('data-word="shipped"');
4723 });
4724
4725 test("a conflicted change alice opened herself offers the bring-up-to-date wording, never a Ship that would be refused", async () => {
4726 const base1 = await commitToBranch(app, alice, repo, {
4727 message: "Add a file two branches will fight over",
4728 files: [{ path: "CONFLICT.md", content: "base\n" }],
4729 });
4730 if (!base1.ok) throw new Error(JSON.stringify(base1.error));
4731 const branch = await commitToBranch(app, alice, repo, {
4732 branch: "feature/own-conflict",
4733 base: "main",
4734 message: "Alice's edit",
4735 files: [{ path: "CONFLICT.md", content: "base\nalice\n" }],
4736 });
4737 if (!branch.ok) throw new Error(JSON.stringify(branch.error));
4738 const onMain = await commitToBranch(app, alice, repo, {
4739 message: "Main's edit",
4740 files: [{ path: "CONFLICT.md", content: "base\nmain\n" }],
4741 });
4742 if (!onMain.ok) throw new Error(JSON.stringify(onMain.error));
4743
4744 const opened = await openPull(app, alice, repo, {
4745 title: "Alice's conflicting change",
4746 body: "This will not merge cleanly.",
4747 baseBranch: "main",
4748 headBranch: "feature/own-conflict",
4749 });
4750 if (!opened.ok) throw new Error(String(opened.error));
4751 const pull = opened.value;
4752 const run = createRun(app.db, {
4753 repoId: repo.id,
4754 kind: "check",
4755 issueId: pull.id,
4756 env: `pull-${pull.number}`,
4757 commitOid: pull.headOid,
4758 reason: "pull",
4759 });
4760 claimRun(app.db, run.id);
4761 finishRun(app.db, run.id, { state: "ok", summary: "check passed in 1s" });
4762
4763 const studio = await (
4764 await get(`/alice/garden/pulls/${pull.number}`, ownerCookie)
4765 ).text();
4766 expect(studio).toContain("<h2>Decide</h2>");
4767 expect(studio).toContain("conflicts with main");
4768 expect(studio).toContain("bring the branch up to date");
4769 expect(studio).toContain("Bring up to date");
4770 expect(studio).not.toContain("Ship");
4771 });
4772 });
4773
45774774 describe("the studio's crew panel on a forge with no agents yet", () => {
45784775 let dataDir: string;
45794776 let app: App;

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs