Public: anyone can read this; the crew can change it.
A pull's preview block (env, data note, terminal) is plain lines under a heading, not a boxed card #10021
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/10006-a-pull-s-preview-block-env-data-note-ter → main 4 commits fork 7 threads · 7 unresolved
7 files
changed +85
−76
src/http/mcp/tools/pull.ts +1 −1
| @@ -81,7 +81,7 @@ export const tool: Tool = { | ||
| 81 | 81 | }); |
| 82 | 82 | // What the card needs and the text does not say: where the pull lives |
| 83 | 83 | // and its preview, only when that environment exists — the pull page's |
| 84 | // door, `previewCard`, reads the same `appView`. | |
| 84 | // door, `previewBlock`, reads the same `appView`. | |
| 85 | 85 | const view = appView(ctx.app, user, repo); |
| 86 | 86 | const env = view.ok |
| 87 | 87 | ? view.value.environments.find((e) => e.env === previewEnv(pull.number)) |
src/web/pages/apps.ts +36 −54
| @@ -77,7 +77,6 @@ import { shelfRows } from "../studio/shelf.ts"; | ||
| 77 | 77 | import { |
| 78 | 78 | badge, |
| 79 | 79 | btn, |
| 80 | card, | |
| 81 | 80 | envFacts, |
| 82 | 81 | envStateBadge, |
| 83 | 82 | icon, |
| @@ -661,53 +660,38 @@ export function appPage(ctx: Ctx): Response { | ||
| 661 | 660 | return response; |
| 662 | 661 | } |
| 663 | 662 | |
| 664 | // ---- the pull page's preview card ---- | |
| 663 | // ---- the pull page's preview block ---- | |
| 665 | 664 | |
| 666 | /** | |
| 667 | * How the preview card looks: a quiet one is bare (heading over rows, no | |
| 668 | * box); a failed one or one that needs attention keeps its tone and box, | |
| 669 | * since a bare card draws no tone. | |
| 670 | */ | |
| 671 | export function previewLook(env: EnvStatus): { | |
| 672 | extra: string; | |
| 673 | tone?: "danger" | "attn"; | |
| 674 | } { | |
| 675 | if (env.state === "failed" || env.error !== undefined) | |
| 676 | return { extra: "env-card", tone: "danger" }; | |
| 677 | if (env.failedDeploy !== undefined) return { extra: "env-card", tone: "attn" }; | |
| 678 | return { extra: "bare env-card" }; | |
| 679 | } | |
| 680 | ||
| 681 | /** Where this pull's preview stands, with the way in. */ | |
| 682 | export function previewCard(ctx: Ctx, repo: Repo, pull: Pull): Html { | |
| 683 | return previewWith(ctx, repo, pull, card); | |
| 684 | } | |
| 685 | ||
| 686 | /** The same preview facts as a block under an h2, the home page's language. */ | |
| 687 | export function previewBlock(ctx: Ctx, repo: Repo, pull: Pull): Html { | |
| 688 | return previewWith(ctx, repo, pull, (body, o) => | |
| 689 | html`<section | |
| 690 | class="settings-block" | |
| 691 | data-preview-block | |
| 692 | ${o.tone ? html`data-tone="${o.tone}"` : null} | |
| 693 | > | |
| 694 | <div class="bar">${o.head}</div> | |
| 695 | ${o.tone === "danger" | |
| 696 | ? html`<p class="small" role="status">This preview has failed.</p>` | |
| 697 | : o.tone === "attn" | |
| 698 | ? html`<p class="muted small" role="status">The last deploy of this preview failed.</p>` | |
| 699 | : null} | |
| 700 | ${body} | |
| 701 | </section>`, | |
| 702 | ); | |
| 703 | } | |
| 704 | ||
| 705 | function previewWith( | |
| 706 | ctx: Ctx, | |
| 707 | repo: Repo, | |
| 708 | pull: Pull, | |
| 709 | card: (body: Html, o: { head: Html; extra?: string; tone?: "danger" | "attn" }) => Html, | |
| 665 | /** The block itself: a bar with the heading, a status line when it failed, then the body. */ | |
| 666 | export function previewSection( | |
outdated
Single-use wrapper: |
||
| 667 | body: Html, | |
| 668 | head: Html, | |
| 669 | tone?: "danger" | "attn", | |
| 710 | 670 | ): Html { |
| 671 | return html`<section | |
| 672 | class="settings-block" | |
| 673 | data-preview-block | |
| 674 | ${tone ? html`data-tone="${tone}"` : null} | |
| 675 | > | |
| 676 | <div class="bar">${head}</div> | |
| 677 | ${tone === "danger" | |
| 678 | ? html`<p class="small" role="status">This preview has failed. Push again to redeploy it.</p>` | |
| 679 | : tone === "attn" | |
| 680 | ? html`<p class="muted small" role="status">The last deploy of this preview failed. Push again to redeploy it.</p>` | |
| 681 | : null} | |
| 682 | ${body} | |
| 683 | </section>`; | |
| 684 | } | |
| 685 | ||
outdated
Clean: a compat alias "kept under its old name" is dead indirection. Rename the three callers to |
||
| 686 | /** How loud a preview reads: danger when it failed or errs, attn when only its last deploy failed. */ | |
| 687 | export function envTone(env: EnvStatus): "danger" | "attn" | undefined { | |
outdated
Principles 1 and 8: a failed preview now gets a new status sentence ("This preview has failed.", "The last deploy of this preview failed.") that the issue did not ask for, and no test covers either state or the data-tone attribute. Add a test for failed and attn, or drop the sentences. Also say what to do next, e.g. "Push again to redeploy it." |
||
| 688 | if (env.state === "failed" || env.error !== undefined) return "danger"; | |
| 689 | if (env.failedDeploy !== undefined) return "attn"; | |
| 690 | return undefined; | |
| 691 | } | |
outdated
This mapping replaces |
||
| 692 | ||
| 693 | /** Where this pull's preview stands, with the way in: a plain block under an h2, the home page's language. */ | |
| 694 | export function previewBlock(ctx: Ctx, repo: Repo, pull: Pull): Html { | |
| 711 | 695 | const view = appView(ctx.app, ctx.auth.user, repo); |
| 712 | 696 | if (!view.ok) return html``; |
| 713 | 697 | const base = repoBase(repo); |
outdated
Clean: with |
||
| @@ -718,9 +702,9 @@ function previewWith( | ||
| 718 | 702 | <h2>Preview</h2> |
| 719 | 703 | ${env === undefined ? null : envStateBadge(env.state, env.dormant !== undefined)}`; |
| 720 | 704 | if (!view.value.serving) |
| 721 | return card( | |
| 705 | return previewSection( | |
| 722 | 706 | html`<p class="muted small">Nothing is serving apps on this forge.</p>`, |
| 723 | { head, extra: "bare" }, | |
| 707 | head, | |
| 724 | 708 | ); |
| 725 | 709 | if (env === undefined) { |
| 726 | 710 | const why = |
| @@ -729,13 +713,13 @@ function previewWith( | ||
| 729 | 713 | : pull.state === "closed" |
| 730 | 714 | ? "The preview was removed when this closed. Reopen the pull request and it comes back." |
| 731 | 715 | : "No preview yet — one starts when this pull request's head deploys."; |
| 732 | return card(html`<p class="muted small">${why}</p>`, { head, extra: "bare" }); | |
| 716 | return previewSection(html`<p class="muted small">${why}</p>`, head); | |
| 733 | 717 | } |
| 734 | 718 | // A preview's terminal is a different login from production's — its own |
| 735 | 719 | // command, shown only to a writer, same as the app page's. |
| 736 | 720 | const writer = canWrite(ctx.app.db, repo, ctx.auth.user); |
| 737 | 721 | const dataNote = previewDataNote(view.value.app.previewData, pull.createdAt); |
| 738 | return card( | |
| 722 | return previewSection( | |
| 739 | 723 | html`${envFacts(base, env)} |
| 740 | 724 | ${dataNote === null ? null : html`<p class="muted small">${dataNote}</p>`} |
| 741 | 725 | ${when(writer, () => |
| @@ -756,10 +740,8 @@ function previewWith( | ||
| 756 | 740 | icon: "external", |
| 757 | 741 | })} |
| 758 | 742 | </div>`, |
| 759 | { | |
| 760 | head, | |
| 761 | ...previewLook(env), | |
| 762 | }, | |
| 743 | head, | |
| 744 | envTone(env), | |
| 763 | 745 | ); |
| 764 | 746 | } |
| 765 | 747 |
src/web/studio/change.ts +2 −2
| @@ -62,7 +62,7 @@ import { | ||
| 62 | 62 | verdictBadge, |
| 63 | 63 | type Tone, |
| 64 | 64 | } from "../ui.ts"; |
| 65 | import { previewCard } from "../pages/apps.ts"; | |
| 65 | import { previewBlock } from "../pages/apps.ts"; | |
| 66 | 66 | import { lookCard } from "../pages/looks.ts"; |
| 67 | 67 | import { approveHereCard, pullChecksLineFor, teamsCard } from "../pages/pull.ts"; |
| 68 | 68 | import { guestWalls, strangerCard } from "../pages/pull-guest.ts"; |
| @@ -431,7 +431,7 @@ async function pullPanels(ctx: Ctx, repo: Repo, pull: Pull, spent: Spent = newSp | ||
| 431 | 431 | </p>`; |
| 432 | 432 | |
| 433 | 433 | const preview = tick(spent, "preview", () => |
| 434 | when(record, () => previewCard(ctx, repo, pull)), | |
| 434 | when(record, () => previewBlock(ctx, repo, pull)), | |
| 435 | 435 | ); |
| 436 | 436 | return { |
| 437 | 437 | record, |
src/web/studio/run.ts +2 −2
| @@ -31,7 +31,7 @@ import { html, raw, when, type Html } from "../html.ts"; | ||
| 31 | 31 | import { runLine } from "../describe.ts"; |
| 32 | 32 | import { page } from "../layout.ts"; |
| 33 | 33 | import { badge, btn, card, listCard, row as stepRow, sha, type Tone } from "../ui.ts"; |
| 34 | import { previewCard } from "../pages/apps.ts"; | |
| 34 | import { previewBlock } from "../pages/apps.ts"; | |
| 35 | 35 | import { costText, durationText, RUN_PAGE_CSS, runTitle, runTokens } from "../pages/runs.ts"; |
| 36 | 36 | import { placementLine, sandboxLine } from "./sandbox.ts"; |
| 37 | 37 | import { logBox, runVoice, RUN_TRANSCRIPT_CAP } from "./log.ts"; |
| @@ -254,7 +254,7 @@ export async function runView( | ||
| 254 | 254 | </form> |
| 255 | 255 | </div>`, |
| 256 | 256 | )} |
| 257 | ${when(pull, (p) => previewCard(ctx, repo, p))} | |
| 257 | ${when(pull, (p) => previewBlock(ctx, repo, p))} | |
| 258 | 258 | ${when( |
| 259 | 259 | logHidden, |
| 260 | 260 | () => |
src/web/studio/tell.ts +3 −3
| @@ -67,7 +67,7 @@ import { | ||
| 67 | 67 | time, |
| 68 | 68 | userLink, |
| 69 | 69 | } from "../ui.ts"; |
| 70 | import { previewCard } from "../pages/apps.ts"; | |
| 70 | import { previewBlock } from "../pages/apps.ts"; | |
| 71 | 71 | import { |
| 72 | 72 | costText, |
| 73 | 73 | durationText, |
| @@ -370,7 +370,7 @@ async function tellRuns( | ||
| 370 | 370 | } |
| 371 | 371 | |
| 372 | 372 | /** The pull that closes this tell, with its check and a way into its |
| 373 | * preview — nothing here that the change page or `previewCard` do not | |
| 373 | * preview — nothing here that the change page or `previewBlock` do not | |
| 374 | 374 | * already draw. */ |
| 375 | 375 | async function tellClosingPull( |
| 376 | 376 | ctx: Ctx, |
| @@ -386,7 +386,7 @@ async function tellClosingPull( | ||
| 386 | 386 | ${wordBadge(word)} <span class="muted small">${note}</span> |
| 387 | 387 | </p>`, |
| 388 | 388 | { head: html`${icon("pull")} <h2>Closes with</h2>` }, |
| 389 | )}${previewCard(ctx, repo, pull)}`; | |
| 389 | )}${previewBlock(ctx, repo, pull)}`; | |
| 390 | 390 | } |
| 391 | 391 | |
| 392 | 392 | /** |
src/web/style/content.ts −1
| @@ -662,7 +662,6 @@ form.inline { display: inline; margin-left: auto; } | ||
| 662 | 662 | line to be read on. The facts and the error read the same wherever they |
| 663 | 663 | are shown — a pull's preview, the studio head — so they are not scoped to |
| 664 | 664 | the card. */ |
| 665 | .env-card .card-head { gap: 10px; } | |
| 666 | 665 | .env-facts { display: flex; align-items: center; gap: 8px; flex-wrap: wrap; font-size: 13px; color: var(--muted-fg); overflow-wrap: anywhere; } |
| 667 | 666 | .env-error { color: var(--danger); font-size: 13px; margin-top: 6px; overflow-wrap: anywhere; text-wrap: pretty; } |
| 668 | 667 | /* The checks line inside the merge box, in the reviews chip's voice. */ |
test/web-apps.test.ts +41 −13
| @@ -18,8 +18,9 @@ import { | ||
| 18 | 18 | STARTERS, |
| 19 | 19 | starterHref, |
| 20 | 20 | } from "../src/web/pages/apps-starters.ts"; |
| 21 | import { previewLook, productionStrip } from "../src/web/pages/apps.ts"; | |
| 21 | import { envTone, previewSection, productionStrip } from "../src/web/pages/apps.ts"; | |
| 22 | 22 | import type { EnvStatus } from "../src/apps/runtime.ts"; |
| 23 | import { html } from "../src/web/html.ts"; | |
outdated
Clean: the stray blank line splits the import block, and the createIssue import sits apart from the others. Group them. The pull also adds no test for the "Nothing is serving apps" state, which is the third state touched. |
||
| 23 | 24 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; |
| 24 | 25 | import { mkdtemp, rm } from "node:fs/promises"; |
| 25 | 26 | import { tmpdir } from "node:os"; |
| @@ -33,7 +34,7 @@ import { getAgentByName, listCrew } from "../src/core/agents.ts"; | ||
| 33 | 34 | import { listAppEnvNames } from "../src/core/app_env.ts"; |
| 34 | 35 | import { spanWords } from "../src/apps/golden_path.ts"; |
| 35 | 36 | import { getAppByRepo } from "../src/core/apps.ts"; |
| 36 | import { getIssue } from "../src/core/issues.ts"; | |
| 37 | import { createIssue, getIssue } from "../src/core/issues.ts"; | |
| 37 | 38 | import { getRepo, type Repo } from "../src/core/repos.ts"; |
| 38 | 39 | import { isTerminal, listRuns, type Run } from "../src/core/runs.ts"; |
| 39 | 40 | import { getUserByName, type User } from "../src/core/users.ts"; |
| @@ -327,17 +328,12 @@ describe("apps, runs and the crew on the web", () => { | ||
| 327 | 328 | expect(none).not.toMatch(/class="card[ "]/); |
| 328 | 329 | }); |
| 329 | 330 | |
| 330 | test("the preview card is bare when quiet and keeps its tone and box when failed", () => { | |
| 331 | test("a preview's tone: danger when failed or erroring, attn after a failed deploy, none when quiet", () => { | |
| 331 | 332 | const env = { env: "pull-1", state: "ok" } as unknown as EnvStatus; |
| 332 | expect(previewLook(env)).toEqual({ extra: "bare env-card" }); | |
| 333 | expect(previewLook({ ...env, state: "failed" } as EnvStatus)).toEqual({ | |
| 334 | extra: "env-card", | |
| 335 | tone: "danger", | |
| 336 | }); | |
| 337 | expect(previewLook({ ...env, error: "boom" } as EnvStatus).tone).toBe("danger"); | |
| 338 | expect( | |
| 339 | previewLook({ ...env, failedDeploy: {} } as unknown as EnvStatus), | |
| 340 | ).toEqual({ extra: "env-card", tone: "attn" }); | |
| 333 | expect(envTone(env)).toBeUndefined(); | |
| 334 | expect(envTone({ ...env, state: "failed" } as EnvStatus)).toBe("danger"); | |
| 335 | expect(envTone({ ...env, error: "boom" } as EnvStatus)).toBe("danger"); | |
| 336 | expect(envTone({ ...env, failedDeploy: {} } as unknown as EnvStatus)).toBe("attn"); | |
| 341 | 337 | }); |
| 342 | 338 | |
| 343 | 339 | // ---- the app page and the runs ---- |
| @@ -938,7 +934,10 @@ describe("apps, runs and the crew on the web", () => { | ||
| 938 | 934 | // merge box are its Files step, one tab under it. |
| 939 | 935 | const change = await bodyOf(alice, `/alice/hello/pulls/${number}`); |
| 940 | 936 | expect(change).toContain("<h2>Preview</h2>"); |
| 941 | expect(change).toMatch(/<div\s+class="card bare[ "][^>]*>\s*<div class="card-head">\s*<svg[^]*?<h2>Preview<\/h2>/); | |
| 937 | // a plain block under its heading, not a boxed card | |
| 938 | expect(change).toMatch(/<section\s+class="settings-block"\s+data-preview-block\s*>\s*<div class="bar">\s*<svg[^]*?<h2>Preview<\/h2>/); | |
| 939 | const block = change.slice(change.indexOf("data-preview-block")); | |
| 940 | expect(block.slice(0, block.indexOf("</section>"))).not.toContain('class="card'); | |
| 942 | 941 | const page = await bodyOf(alice, `/alice/hello/pulls/${number}/files`); |
| 943 | 942 | expect(page).toContain("<h2>Assignee</h2>"); |
| 944 | 943 | // Checks are required: the line says so, and the box waits for them |
| @@ -1098,4 +1097,33 @@ describe("apps, runs and the crew on the web", () => { | ||
| 1098 | 1097 | expect(say("running", true)).toContain("Running"); |
| 1099 | 1098 | expect(say("failed", true)).toContain("Failed to start"); |
| 1100 | 1099 | }); |
| 1100 | ||
| 1101 | test("a pull with no preview yet says so as a plain block, no card", async () => { | |
| 1102 | const issue = createIssue(app.db, { repoId: hello.id, authorId: user("alice").id, title: "Quiet", kind: "pull" }); | |
| 1103 | if (!issue.ok) throw new Error(issue.error); | |
| 1104 | app.db | |
| 1105 | .query(`INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid) VALUES (?1, 'main', 'quiet', 'a', 'b')`) | |
| 1106 | .run(issue.value.id); | |
| 1107 | const page = await bodyOf(alice, `/alice/hello/pulls/${issue.value.number}`); | |
| 1108 | const at = page.indexOf("data-preview-block"); | |
| 1109 | expect(at).toBeGreaterThan(0); | |
| 1110 | const block = page.slice(at, page.indexOf("</section>", at)); | |
| 1111 | expect(block).toContain("<h2>Preview</h2>"); | |
| 1112 | expect(block).toContain("No preview yet"); | |
| 1113 | expect(block).not.toContain('class="card'); | |
| 1114 | }); | |
| 1115 | ||
| 1116 | test("a failed preview says so and what to do; a failed last deploy says it too; a healthy one is silent", () => { | |
| 1117 | const body = html`<p>body</p>`; | |
| 1118 | const head = html`<h2>Preview</h2>`; | |
| 1119 | const failed = previewSection(body, head, "danger").toString(); | |
| 1120 | expect(failed).toContain('data-tone="danger"'); | |
| 1121 | expect(failed).toContain("This preview has failed. Push again to redeploy it."); | |
| 1122 | const attn = previewSection(body, head, "attn").toString(); | |
| 1123 | expect(attn).toContain('data-tone="attn"'); | |
| 1124 | expect(attn).toContain("The last deploy of this preview failed. Push again to redeploy it."); | |
| 1125 | const ok = previewSection(body, head).toString(); | |
| 1126 | expect(ok).not.toContain("data-tone"); | |
| 1127 | expect(ok).not.toContain('role="status"'); | |
| 1128 | }); | |
| 1101 | 1129 | }); |
Stale wording: this is no longer the preview card. Name it the preview block.