Public: anyone can read this; the crew can change it.
A stranger reaches their first pull on a public app from the app's page: get in, fork, send #12159
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9574-a-stranger-reaches-their-first-pull-on-a → main 6 commits fork 4 threads · 4 unresolved
4 files
changed +189
−1
docs/features/guests.md +10
| @@ -95,3 +95,13 @@ only the clear page and #9571. | ||
| 95 | 95 | |
| 96 | 96 | Not yet recorded. When the owner or a writer comments the choice on #9573, add |
| 97 | 97 | it here: the option, the date, who chose it. The issue is closed only then. |
| 98 | ||
| 99 | ## The way in from an app's page | |
| 100 | ||
| 101 | A public app's page shows a stranger three rows under "Help build this": Get in | |
| 102 | (`/signup`, or a forge of their own at `/forges/new`), Fork it, Send a pull. A | |
| 103 | signed-in reader who cannot write the app gets one row with the button "Fork and | |
| 104 | send a pull". The owner, a writer and an archived app show nothing | |
| 105 | (`offersWayIn` in `src/web/studio/contribute_steps.ts`). Every link is a page | |
| 106 | that already exists; the section adds no route and widens nothing. The terminal | |
| 107 | reaches the same step with the "Make it yours" command (`src/tui/screens/yours.ts`). |
src/web/studio/app.ts +3
| @@ -64,6 +64,7 @@ import { adapterCard } from "./adapter.ts"; | ||
| 64 | 64 | import { jobsLink } from "./jobs.ts"; |
| 65 | 65 | import { schedulesCard } from "./schedules.ts"; |
| 66 | 66 | import { appPanels } from "./previews.ts"; |
| 67 | import { contributeSteps } from "./contribute_steps.ts"; | |
| 67 | 68 | import { noCrewLine, pausedWords, servingHistory, type WordCtx } from "./words.ts"; |
| 68 | 69 | import { crewKeyCard, includedCredit } from "./crew_key.ts"; |
| 69 | 70 | import { landOnApp } from "./land_as_is.ts"; |
| @@ -514,6 +515,7 @@ export async function studioApp( | ||
| 514 | 515 | ${ownerFacts ? crewKeyCard(ctx, true) : null} |
| 515 | 516 | ${ownerFacts ? landOnApp(ctx, repo) : null} |
| 516 | 517 | ${pausedLine(ctx, repo)} |
| 518 | ${contributeSteps(ctx, repo)} | |
| 517 | 519 | <div class="stack"> |
| 518 | 520 | ${adapter} |
| 519 | 521 | ${schedules} |
| @@ -537,6 +539,7 @@ export async function studioApp( | ||
| 537 | 539 | ${ownerFacts ? crewKeyCard(ctx, true) : null} |
| 538 | 540 | ${ownerFacts ? landOnApp(ctx, repo) : null} |
| 539 | 541 | ${pausedLine(ctx, repo)} |
| 542 | ${contributeSteps(ctx, repo)} | |
| 540 | 543 | ${ownerFacts |
| 541 | 544 | ? egressNote( |
| 542 | 545 | ctx.app.config.dataDir, |
src/web/studio/contribute_steps.ts added +44
| @@ -0,0 +1,44 @@ | ||
| 1 | /** | |
| 2 | * The way in for a reader who cannot write an app: on a public app's page, | |
| 3 | * the three steps to a first pull (get in, fork, send), each a row; a | |
| 4 | * signed-in reader gets "Fork and send a pull" directly. Built for the | |
| 5 | * options the guests decision lays out (docs/features/guests.md): an account | |
| 6 | * here, or a forge of their own. It widens nothing: every link is a page that | |
| 7 | * already exists and the mutations behind them check same-origin themselves. | |
| 8 | */ | |
| 9 | ||
| 10 | import { HELP_BUILD } from "../../core/contribute.ts"; | |
| 11 | import { canWrite, type Repo } from "../../core/repos.ts"; | |
| 12 | import type { Ctx } from "../../http/context.ts"; | |
| 13 | import { repoBase } from "../chrome.ts"; | |
| 14 | import { html, type Html } from "../html.ts"; | |
| 15 | import { btn, row } from "../ui.ts"; | |
| 16 | ||
| 17 | export const FORK_AND_SEND = "Fork and send a pull"; | |
| 18 | ||
| 19 | /** Whether the page offers the way in: a public app its viewer cannot already write. */ | |
| 20 | export function offersWayIn(ctx: Pick<Ctx, "app" | "auth">, repo: Repo): boolean { | |
| 21 | if (repo.visibility !== "public" || repo.archivedAt !== null) return false; | |
| 22 | const user = ctx.auth.user; | |
| 23 | return user === undefined || (user.id !== repo.ownerId && !canWrite(ctx.app.db, repo, user)); | |
| 24 | } | |
| 25 | ||
| 26 | /** The steps, or nothing for a viewer who builds here already. */ | |
| 27 | export function contributeSteps(ctx: Ctx, repo: Repo): Html { | |
| 28 | if (!offersWayIn(ctx, repo)) return html``; | |
| 29 | const base = repoBase(repo); | |
| 30 | if (ctx.auth.user !== undefined) | |
| 31 | return html`<section id="contribute" aria-label="${HELP_BUILD}"><ul class="list">${row({ | |
| 32 | title: HELP_BUILD, | |
| 33 | trail: btn(FORK_AND_SEND, { href: `${base}/make-it-yours`, variant: "primary", icon: "fork" }), | |
| 34 | })}</ul></section>`; | |
| 35 | return html`<section id="contribute" aria-labelledby="contribute-h"> | |
outdated
Principles 6 and 7: a card boxes a list. Use rows (a hairline where it ends), not card(). |
||
| 36 | <h2 id="contribute-h">${HELP_BUILD}</h2> | |
| 37 | <p class="muted small">Anyone can send a change; a person reviews it first.</p> | |
| 38 | <ul class="list"> | |
| 39 | ${row({ title: "Get in", href: "/signup", after: html`<a class="muted small" href="/forges/new">or use your own forge</a>` })} | |
| 40 | ${row({ title: "Fork it", href: `${base}/make-it-yours` })} | |
| 41 | ${row({ title: "Send a pull", href: `${base}/pulls/new` })} | |
| 42 | </ul> | |
outdated
Principle 8: 'you get your own copy to change' and 'a person reviews it before any of it runs' explain what the link text already says. Trim to the link. |
||
| 43 | </section>`; | |
| 44 | } |
test/guest-contribute.test.ts +132 −1
| @@ -23,7 +23,7 @@ import { startChecks, type ChecksHandle } from "../src/checks/service.ts"; | ||
| 23 | 23 | import { GUEST_REFUSALS, HELP_BUILD, HELP_BUILD_NEXT, isContributionFork, SIGNUP_YOU_GET } from "../src/core/contribute.ts"; |
| 24 | 24 | import { pullMayRun } from "../src/core/pull_trust.ts"; |
| 25 | 25 | import { getPull } from "../src/core/pulls.ts"; |
| 26 | import { canAdmin, canWrite, getRepoById, type Repo } from "../src/core/repos.ts"; | |
| 26 | import { canAdmin, canWrite, getRepo, getRepoById, moveArchived, type Repo } from "../src/core/repos.ts"; | |
| 27 | 27 | import { NOT_A_MEMBER, setMember } from "../src/core/roles.ts"; |
| 28 | 28 | import { createSession } from "../src/core/sessions.ts"; |
| 29 | 29 | import { createToken } from "../src/core/tokens.ts"; |
| @@ -35,9 +35,11 @@ import { queueAgentRun } from "../src/crew/dispatch.ts"; | ||
| 35 | 35 | import { sendToReview } from "../src/crew/review.ts"; |
| 36 | 36 | import { getIssueById } from "../src/core/issues.ts"; |
| 37 | 37 | import { serve, type Servers } from "../src/http/server.ts"; |
| 38 | import type { Ctx } from "../src/http/context.ts"; | |
| 38 | 39 | import { isGuestPull, guestWaits } from "../src/origins/guest.ts"; |
| 39 | 40 | import { setLimit } from "../src/settings/limits.ts"; |
| 40 | 41 | import { setSignup } from "../src/settings/signup.ts"; |
| 42 | import { FORK_AND_SEND, offersWayIn } from "../src/web/studio/contribute_steps.ts"; | |
| 41 | 43 | import { quietLogs } from "./helpers/globals.ts"; |
| 42 | 44 | import { mcpTool } from "./helpers/mcp.ts"; |
| 43 | 45 | import { promote } from "./helpers/members.ts"; |
| @@ -562,3 +564,132 @@ describe("a guest helps build a public repository", () => { | ||
| 562 | 564 | expect(merged.status).toBeLessThan(300); |
| 563 | 565 | }); |
| 564 | 566 | }); |
| 567 | ||
| 568 | describe("a stranger's first pull starts from the app's page", () => { | |
| 569 | let dataDir = ""; | |
| 570 | let app: App; | |
| 571 | let servers: Servers; | |
| 572 | ||
| 573 | beforeAll(async () => { | |
| 574 | dataDir = await mkdtemp(join(tmpdir(), "forge-first-pull-")); | |
| 575 | app = createApp({ dataDir, port: 0 }); | |
| 576 | servers = await serve(app); | |
| 577 | }, 60_000); | |
| 578 | ||
| 579 | afterAll(async () => { | |
| 580 | servers.stop(); | |
| 581 | app.close(); | |
| 582 | await rm(dataDir, { recursive: true, force: true }); | |
| 583 | }); | |
| 584 | ||
| 585 | test("signed out: get in, fork, send, each a link that works, to an open pull", async () => { | |
| 586 | const made = await createUser(app.db, { origin: "seed", trusted: true, username: "olive", email: "[email protected]", password: "correct-horse-battery" }); | |
| 587 | if (!made.ok) throw new Error(made.error); | |
| 588 | app.db.query("UPDATE user SET is_admin = 1 WHERE id = ?1").run(made.value.id); | |
| 589 | markVerified(app.db, made.value.id, new Date().toISOString()); | |
| 590 | const owner = getUserByName(app.db, "olive")!; | |
| 591 | const repo = await createRepo(app, owner, { name: "game", visibility: "public" }); | |
| 592 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 593 | const seeded = await commitToBranch(app, owner, repo.value, { message: "Start", files: [{ path: "README.md", content: "hi\n" }] }); | |
| 594 | if (!seeded.ok) throw new Error(JSON.stringify(seeded.error)); | |
| 595 | const enabled = enableApp(app, owner, getRepoById(app.db, repo.value.id)!); | |
| 596 | if (!enabled.ok) throw new Error(String(enabled.error)); | |
| 597 | const priv = await createRepo(app, owner, { name: "secret", visibility: "private" }); | |
| 598 | if (!priv.ok) throw new Error(String(priv.error)); | |
| 599 | ||
| 600 | // The page, signed out, names the three steps; every link answers. | |
| 601 | const res = await fetch(`${servers.url}/olive/game`); | |
outdated
Add cases for the owner, a writer and an archived app seeing no block; today offersWayIn's exclusions are untested. |
||
| 602 | const page = await res.text(); | |
| 603 | const block = page.slice(page.indexOf('id="contribute"')); | |
| 604 | expect(block).toContain("Get in"); | |
| 605 | expect(block).not.toContain(FORK_AND_SEND); | |
| 606 | const links = [...block.slice(0, block.indexOf("</section>")).matchAll(/href="([^"]+)"/g)].map((m) => m[1]!); | |
| 607 | expect(links).toEqual(["/signup", "/forges/new", "/olive/game/make-it-yours", "/olive/game/pulls/new"]); | |
| 608 | for (const href of links) { | |
| 609 | const hit = await fetch(`${servers.url}${href}`, { redirect: "manual" }); | |
| 610 | expect(hit.status).toBeLessThan(400); | |
| 611 | } | |
| 612 | // "Get in" leads to an account: with signup open, the page offers the form and the form's | |
| 613 | // first call answers with a challenge. (The passkey itself needs a browser.) | |
| 614 | setSignup(app.db, "open", made.value.id); | |
| 615 | const signupPage = await fetch(`${servers.url}/signup`); | |
| 616 | expect(signupPage.status).toBe(200); | |
| 617 | const opts = await fetch(`${servers.url}/signup/passkey/options`, { method: "POST", headers: { origin: servers.url, "content-type": "application/json" }, body: JSON.stringify({ username: "newcomer", email: "" }) }); | |
| 618 | expect(opts.status).toBe(200); | |
| 619 | expect(((await opts.json()) as { challenge?: string }).challenge).toBeTruthy(); | |
| 620 | const toSignIn = await fetch(`${servers.url}/olive/game/make-it-yours`, { redirect: "manual" }); | |
| 621 | expect(toSignIn.headers.get("location")).toContain("/signin?next="); | |
| 622 | ||
| 623 | // A private app offers nothing. | |
| 624 | expect(await (await fetch(`${servers.url}/olive/secret`)).text()).not.toContain('id="contribute"'); | |
| 625 | ||
| 626 | // Signed in as a new guest: the page says Fork and send a pull, and the walk reaches an open | |
| 627 | // pull. | |
| 628 | const guest = await createUser(app.db, { origin: "seed", trusted: true, username: "stranger", email: "[email protected]", password: "correct-horse-battery" }); | |
| 629 | if (!guest.ok) throw new Error(guest.error); | |
| 630 | markVerified(app.db, guest.value.id, new Date().toISOString()); | |
| 631 | app.db.query("UPDATE user SET created_at = ?1 WHERE id = ?2").run(LONG_AGO, guest.value.id); | |
| 632 | const cookie = { cookie: `forge_session=${createSession(app.db, guest.value.id).token}` }; | |
| 633 | const signedIn = await (await fetch(`${servers.url}/olive/game`, { headers: cookie })).text(); | |
| 634 | expect(signedIn).toContain(FORK_AND_SEND); | |
| 635 | expect(signedIn).not.toContain("Get in"); | |
| 636 | expect(await (await fetch(`${servers.url}/olive/game/make-it-yours`, { headers: cookie })).text()).toContain(HELP_BUILD); | |
| 637 | ||
| 638 | const form = { ...cookie, origin: servers.url, "content-type": "application/x-www-form-urlencoded" }; | |
| 639 | const forked = await fetch(`${servers.url}/olive/game/fork`, { method: "POST", redirect: "manual", headers: form, body: "" }); | |
| 640 | expect(forked.status).toBe(303); | |
| 641 | const forkPath = forked.headers.get("location")!; | |
| 642 | expect(forkPath).toContain("/stranger/"); | |
| 643 | const forkRepo_ = getRepo(app.db, "stranger", forkPath.split("/")[2]!)!; | |
| 644 | const committed = await commitToBranch(app, getUserByName(app.db, "stranger")!, forkRepo_, { branch: "change", message: "Change", files: [{ path: "a.md", content: "x\n" }] }); | |
| 645 | if (!committed.ok) throw new Error(JSON.stringify(committed.error)); | |
| 646 | ||
| 647 | // A cross-origin send is refused; the same-origin one opens the pull. | |
| 648 | const body = "title=My+change&body=&baseRepo=parent&base=main&head=change"; | |
| 649 | const cross = await fetch(`${servers.url}${forkPath}/pulls/new`, { method: "POST", redirect: "manual", headers: { ...form, origin: "https://evil.example" }, body }); | |
| 650 | expect(cross.status).toBeGreaterThanOrEqual(400); | |
| 651 | const sent = await fetch(`${servers.url}${forkPath}/pulls/new`, { method: "POST", redirect: "manual", headers: form, body }); | |
| 652 | expect(sent.status).toBe(303); | |
| 653 | expect(sent.headers.get("location")).toContain("/olive/game/pulls/1"); | |
| 654 | const pull = getPull(app.db, getRepoById(app.db, repo.value.id)!.id, 1)!; | |
| 655 | expect(pull.state).toBe("open"); | |
| 656 | expect(isGuestPull(app.db, pull)).toBe(true); | |
| 657 | }, 60_000); | |
| 658 | ||
| 659 | test("the owner, a writer and an archived app see no way in", async () => { | |
| 660 | const owner = getUserByName(app.db, "olive")!; | |
| 661 | const made = await createRepo(app, owner, { name: "tool", visibility: "public" }); | |
| 662 | if (!made.ok) throw new Error(String(made.error)); | |
| 663 | const seeded = await commitToBranch(app, owner, made.value, { message: "Start", files: [{ path: "README.md", content: "hi\n" }] }); | |
| 664 | if (!seeded.ok) throw new Error(JSON.stringify(seeded.error)); | |
| 665 | const enabled = enableApp(app, owner, getRepoById(app.db, made.value.id)!); | |
| 666 | if (!enabled.ok) throw new Error(String(enabled.error)); | |
| 667 | const tool = getRepoById(app.db, made.value.id)!; | |
| 668 | const person = await createUser(app.db, { origin: "seed", trusted: true, username: "reader", email: "[email protected]", password: "correct-horse-battery" }); | |
| 669 | if (!person.ok) throw new Error(person.error); | |
| 670 | const reader = getUserByName(app.db, "reader")!; | |
| 671 | const admin = await createUser(app.db, { origin: "seed", trusted: true, username: "writer", email: "[email protected]", password: "correct-horse-battery" }); | |
| 672 | if (!admin.ok) throw new Error(admin.error); | |
| 673 | app.db.query("UPDATE user SET is_admin = 1 WHERE id = ?1").run(admin.value.id); | |
| 674 | const writer = getUserByName(app.db, "writer")!; | |
| 675 | const as = (user: User | undefined) => ({ app, auth: { user } }) as Pick<Ctx, "app" | "auth">; | |
| 676 | ||
| 677 | expect(offersWayIn(as(undefined), tool)).toBe(true); | |
| 678 | expect(offersWayIn(as(reader), tool)).toBe(true); | |
| 679 | expect(offersWayIn(as(owner), tool)).toBe(false); | |
| 680 | expect(canWrite(app.db, tool, writer)).toBe(true); | |
| 681 | expect(offersWayIn(as(writer), tool)).toBe(false); | |
| 682 | ||
| 683 | // The pages agree: a writer's and the owner's view of the app carries no block. | |
| 684 | const cookie = (u: User) => ({ cookie: `forge_session=${createSession(app.db, u.id).token}` }); | |
| 685 | for (const u of [owner, writer]) { | |
| 686 | expect(await (await fetch(`${servers.url}/olive/tool`, { headers: cookie(u) })).text()).not.toContain('id="contribute"'); | |
| 687 | } | |
| 688 | expect(await (await fetch(`${servers.url}/olive/tool`, { headers: cookie(reader) })).text()).toContain('id="contribute"'); | |
| 689 | ||
| 690 | moveArchived(app.db, tool.id, true); | |
| 691 | const archived = getRepoById(app.db, tool.id)!; | |
| 692 | expect(offersWayIn(as(undefined), archived)).toBe(false); | |
| 693 | expect(offersWayIn(as(reader), archived)).toBe(false); | |
| 694 | }, 60_000); | |
| 695 | }); |
new above
offline
Principles 1 and 8: HELP_BUILD_NEXT beside a button that says the same thing explains itself twice. Drop the paragraph or the heading.