Public: anyone can read this; the crew can change it.
The studio can look at your change but cannot ship it #225
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/211-the-studio-can-look-at-your-change-but-c → main 1 commit fork
2 files
changed +256
−15
src/web/studio/change.ts +59 −15
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | import { appsRuntime } from "../../apps/runtime.ts"; |
| 11 | 11 | import type { AppRecord } from "../../core/apps.ts"; |
| 12 | 12 | 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"; | |
| 14 | 14 | import { checkSummary } from "../../core/runs.ts"; |
| 15 | 15 | import { repoDir } from "../../git/repos.ts"; |
| 16 | 16 | import type { Ctx } from "../../http/context.ts"; |
| @@ -31,7 +31,12 @@ import { previewCard } from "../pages/apps.ts"; | ||
| 31 | 31 | import { lookCard } from "../pages/looks.ts"; |
| 32 | 32 | import { pullState } from "../pages/pulls.ts"; |
| 33 | 33 | 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"; | |
| 35 | 40 | import { pullWord, wordBadge, type WordCtx } from "./words.ts"; |
| 36 | 41 | |
| 37 | 42 | /** What a conflicted change's own page says: the base it no longer merges |
| @@ -99,9 +104,43 @@ export async function studioPull( | ||
| 99 | 104 | if (production?.runId) |
| 100 | 105 | deploy = { id: production.runId, commitOid: production.commitOid }; |
| 101 | 106 | } |
| 102 | const decision = decisionsFor(ctx, [repo]).find( | |
| 107 | const crewDecision = decisionsFor(ctx, [repo]).find( | |
| 103 | 108 | (d) => d.pull?.id === pull.id, |
| 104 | 109 | ); |
| 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; | |
| 105 | 144 | |
| 106 | 145 | const head = html`<div class="issue-head"> |
| 107 | 146 | <h1>${pull.title} <span class="number">#${pull.number}</span></h1> |
| @@ -141,11 +180,13 @@ export async function studioPull( | ||
| 141 | 180 | // it would only be refused. The button that helps posts to the very same |
| 142 | 181 | // route (`/ship`): `shipSubmit` already turns a conflict there into the |
| 143 | 182 | // 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( | |
| 149 | 190 | pull.baseBranch, |
| 150 | 191 | status!.conflicts, |
| 151 | 192 | )}.</p> |
| @@ -154,13 +195,16 @@ export async function studioPull( | ||
| 154 | 195 | ${btn("Bring up to date", { size: "sm", variant: "primary" })} |
| 155 | 196 | </form> |
| 156 | 197 | </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 | }), | |
| 164 | 208 | ); |
| 165 | 209 | |
| 166 | 210 | 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", () => { | ||
| 4574 | 4574 | }); |
| 4575 | 4575 | }); |
| 4576 | 4576 | |
| 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 | ||
| 4577 | 4774 | describe("the studio's crew panel on a forge with no agents yet", () => { |
| 4578 | 4775 | let dataDir: string; |
| 4579 | 4776 | let app: App; |
new above
offline