trevato/forge

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

More
Forks1g fMapg aRuns13g rEventsFlows

GET /review lists every ask as a row with its title, repository, age and why #4420

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/4407-get-review-lists-every-ask-as-a-row-with → main 3 commits fork 2 threads · 2 unresolved

  • Merged. scout/4407-get-review-lists-every-ask-as-a-row-with was merged into main as fa73a60.

Review

Sign in to review this change.
9 files changed +194 −4
README.md +5
@@ -444,6 +444,11 @@ thoughts, then the issues you can write to that are not shipped, oldest first
444444 only, shows a guest nothing, and names nothing of a placement in a repository
445445 you cannot read.
446446
447 Review: `/review` lists the asks waiting on you, one row each (title, repository,
448 age, why), newest first. It only reads; the buttons stay on `/asks`, which links
449 to it, and signed out it sends you to sign-in. The terminal has no page of its
450 own for it: the same asks are the dashboard's On you card.
451
447452 A client's forge: `Ctrl+K`, then **New forge** (any signed-in person's, as the
448453 page is), is `/forges/new` in the terminal — name, for whom, what it is for,
449454 apps to fork (`owner/name`, comma-separated), a fresh one-line app, model key
docs/design/audit.md +2 −1
@@ -33,7 +33,7 @@ their own vocabulary instead of composing the named one.
3333
3434 ## Finding 2 — the pages, and what each is for
3535
36 There are 61 top-level pages, one entry each: every static `GET` route in `src/http/server.ts`
36 There are 62 top-level pages, one entry each: every static `GET` route in `src/http/server.ts`
3737 that returns a page (`test/design-audit.test.ts` holds this list to the router,
3838 both ways). Each entry reads `route` — module · what it is for · what
3939 duplicates it · what breaks `docs/DESIGN.md`. "Breaks" counts colour literals
@@ -57,6 +57,7 @@ Machine routes are not pages and have no entry: `/audit.csv`,
5757 - `/boards/data` — `pages/boards.ts` · the boards' query/data surface · `/board` · none seen.
5858 - `/asks` — `studio/asks_page.ts` · what the crew is waiting on me for · `/` "needs me" list · colours 2.
5959 - `/asked` — `studio/asked_page.ts` · what I answered, as a record · `/asks` · none seen.
60 - `/review` — `studio/review_page.ts` · every ask waiting on me, one row each, newest first · `/asks` (same asks, with buttons) · none seen.
6061 - `/flow` — `pages/flow.ts` · the pipeline from ready issue to live deploy, and the bottleneck · `/system`, `/board`, `/` · none seen. A topbar button points here.
6162 - `/map` — `pages/map.ts` (1,806 lines) · the dependency map of work across the forge · `/board`, issue dependency lists, `/:owner/:name/map` · colours 3, sizes 12: the largest private vocabulary.
6263 - `/audit` — `pages/audit.ts` · every app's contract, who can do what · the app page's one-line policy · none seen. Names the audit of apps, not this document.
forge.json +1 −1
@@ -3,7 +3,7 @@
33 "api.anthropic.com"
44 ],
55 "app": {
6 "looks": ["/", "/forges", "/family", "/front", "/explore", "/board", "/boards", "/asked", "/search", "/new", "/audit", "/releases", "/apps", "/migrate", "/map", "/system", "/contribute", "/resilience", "/settings"],
6 "looks": ["/", "/forges", "/family", "/front", "/explore", "/board", "/boards", "/asked", "/review", "/search", "/new", "/audit", "/releases", "/apps", "/migrate", "/map", "/system", "/contribute", "/resilience", "/settings"],
77 "run": "bun run src/main.ts",
88 "install": "",
99 "health": "/healthz",
src/core/reserved.ts +1
@@ -108,6 +108,7 @@ export const SYSTEM_NAMES: readonly string[] = [
108108 "recover",
109109 "www",
110110 "asked",
111 "review",
111112 "claim",
112113 "first-admin",
113114 "forges",
src/http/server.ts +2
@@ -256,6 +256,7 @@ import { familyPage } from "../web/pages/family.ts";
256256 import { forgeLinkSubmit, forgesPage } from "../web/pages/forges.ts";
257257 import { newForgePage, newForgeSubmit } from "../web/pages/new-forge.ts";
258258 import { askedPage } from "../web/studio/asked_page.ts";
259 import { reviewPage } from "../web/studio/review_page.ts";
259260 import {
260261 asksBulkSubmit,
261262 asksBulkUndo,
@@ -737,6 +738,7 @@ export function buildRouter(): Router<Handler> {
737738 router.add("POST", "/asks/decide/:id/undo", asksDecideUndo);
738739 // What people asked of the crew, and what became of it (core/asked.ts).
739740 router.add("GET", "/asked", askedPage);
741 router.add("GET", "/review", reviewPage);
740742 // The big view, folded into the board: every old address lands on its wall mode.
741743 router.add("GET", "/big", bigRedirect);
742744 router.add("GET", "/search", searchPage);
src/web/studio/asks.ts +1 −1
@@ -851,7 +851,7 @@ export function repoAsks(
851851 }
852852
853853 /** What asks the viewer now: candidates, less what they set aside. */
854 function waitingOn(
854 export function waitingOn(
855855 db: Database,
856856 user: User | undefined,
857857 repos: readonly Repo[],
src/web/studio/asks_page.ts +1 −1
@@ -359,7 +359,7 @@ function renderAsks(ctx: Ctx, top: AsksTop | undefined): Response {
359359 const response = page({
360360 ctx,
361361 title: "Asks",
362 body: html`<main class="stack"><h1 class="sr-only">Asks</h1>${onYouRegion(ctx, user, undefined, section.body)}</main>`,
362 body: html`<main class="stack"><h1 class="sr-only">Asks</h1>${onYouRegion(ctx, user, undefined, section.body)}<p class="small muted"><a href="/review">Review: one row per ask</a></p></main>`,
363363 scriptJs: `${liveClientJs}\n${liveRegionsJs}\n${section.scriptJs}`,
364364 });
365365 notePhase(ctx.request, "render", performance.now() - rendering);
src/web/studio/review_page.ts added +55
@@ -0,0 +1,55 @@
1 /**
2 * `/review`: every ask waiting on the viewer as one row — the title, the
3 * repository, how long it has waited and why a person must decide — newest
4 * decision first. Read from the same candidates the asks page reads
5 * (`waitingOn`); no polling, and nothing here changes anything.
6 */
7
8 import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts";
9 import { listReposVisibleTo } from "../../core/repos.ts";
10 import type { Ctx } from "../../http/context.ts";
11 import { repoBase, signInFirst } from "../chrome.ts";
12 import { html } from "../html.ts";
13 import { page } from "../layout.ts";
14 import { empty, listCard, row, time } from "../ui.ts";
15 import { waitingOn } from "./asks.ts";
16
17 export function reviewPage(ctx: Ctx): Response {
18 notePhase(ctx.request, "session", takeSessionTime(ctx.request));
19 const user = ctx.auth.user;
20 if (!user) return signInFirst(ctx);
21 const asks = timePhase(ctx.request, "asks", () => {
22 const repos = listReposVisibleTo(ctx.app.db, user, { limit: 200, withoutArchived: true });
23 return waitingOn(ctx.app.db, user, repos).sort((a, b) => Date.parse(b.since) - Date.parse(a.since));
24 });
25 const rendering = performance.now();
26 const rows = asks.map((a) => {
27 const base = repoBase(a.repo);
28 const href =
29 a.pull !== undefined
30 ? `${base}/pulls/${a.pull.number}`
31 : a.issue !== undefined
32 ? `${base}/issues/${a.issue.number}`
33 : base;
34 return row({
35 href,
36 nav: true,
37 title: a.pull?.title ?? a.issue?.title ?? a.proposal?.title ?? `${a.repo.name} is waiting on you`,
38 titleAttr: `${a.repo.ownerName}/${a.repo.name}: ${a.why ?? a.consequence.why}`,
sentinelcrew outdated

Minor: .row-meta is nowrap with an ellipsis, so a long why is cut off. DESIGN pairs this with a title, so pass titleAttr with the full why so it can still be read.
39 meta: html`${a.repo.ownerName}/${a.repo.name} · ${time(a.since)} · ${a.why ?? a.consequence.why}`,
40 });
41 });
42 const body = html`<h1>Review</h1>
43 <p class="muted">${asks.length === 0 ? "All fine" : `${asks.length} ${asks.length === 1 ? "ask" : "asks"}, newest first`}</p>
44 ${rows.length === 0
sentinelcrew outdated

DESIGN §5: an empty state carries exactly one action. Add action: here (a link to /asks), and show the footer link only when there are rows.
45 ? empty({
46 icon: "issue",
47 title: "Nothing to review",
48 note: "When the crew needs a person, it shows up here.",
49 action: html`<a class="btn" href="/asks">All asks</a>`,
50 })
51 : html`${listCard(rows)}<p class="small muted"><a href="/asks">All asks, with their buttons</a></p>`}`;
52 const response = page({ ctx, title: "Review", body });
53 notePhase(ctx.request, "render", performance.now() - rendering);
54 return response;
55 }
test/review-page.test.ts added +126
@@ -0,0 +1,126 @@
1 /** `/review` lists each ask waiting on a person as one row with its why. */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtempSync, rmSync } from "node:fs";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp, type App } from "../src/app.ts";
7 import { addComment, openIssue } from "../src/actions/issues.ts";
8 import { addCrew, createAgentRow, setCrewRole } from "../src/core/agents.ts";
9 import { createRepo as createRepoRow } from "../src/core/repos.ts";
10 import { submitReview } from "../src/core/reviews.ts";
11 import { createUser, getUserByName } from "../src/core/users.ts";
12 import { BREAKER_NOTE } from "../src/crew/breaker.ts";
13 import { serve, type Servers } from "../src/http/server.ts";
14 import { id } from "../src/kernel/id.ts";
15 import { quietLogs } from "./helpers/globals.ts";
16
17 quietLogs();
18
19 const ZERO = "0".repeat(40);
20 const hoursAgo = (h: number) => new Date(Date.now() - h * 3_600_000).toISOString();
21
22 let root = "";
23 let app: App;
24 let servers: Servers;
25 let cookie = "";
26
27 beforeAll(async () => {
28 root = mkdtempSync(join(tmpdir(), "forge-review-"));
29 app = createApp({ dataDir: root, port: 0 });
30 servers = await serve(app);
31 const mk = async (username: string, agent: boolean) => {
32 const made = await createUser(app.db, {
33 username,
34 email: `${username}@forge.test`,
35 ...(agent
36 ? { agent: true, origin: "agent" as const }
37 : { password: "a-good-passphrase", origin: "seed" as const, trusted: true, adminIfFirst: true }),
38 });
39 if (!made.ok) throw new Error(made.error);
40 if (agent) createAgentRow(app.db, { userId: made.value.id, brief: `${username}.` });
41 return made.value;
42 };
43 const alice = await mk("alice", false);
44 const builder = await mk("builder", true);
45 const reviewer = await mk("reviewer", true);
46 const made = await createRepoRow(app.db, app.reposRoot, {
47 ownerId: alice.id,
48 name: "wait",
49 visibility: "public",
50 description: "wait",
51 });
52 if (!made.ok) throw new Error(made.error);
53 const repo = made.value;
54 for (const a of [builder, reviewer]) addCrew(app.db, repo.id, a.id, alice.id);
55 setCrewRole(app.db, repo.id, reviewer.id, "reviewer");
56 let seq = 0;
57 const pull = (author: string, title: string, age: number) => {
58 const issueId = id();
59 const number = app.db
60 .query<{ issue_seq: number }, [string]>("UPDATE repo SET issue_seq = issue_seq + 1 WHERE id = ?1 RETURNING issue_seq")
61 .get(repo.id)!.issue_seq;
62 app.db
63 .query(
64 `INSERT INTO issue (id, repo_id, number, kind, title, body, author_id, state, created_at)
65 VALUES (?1, ?2, ?3, 'pull', ?4, '', ?5, 'open', ?6)`,
66 )
67 .run(issueId, repo.id, number, title, author, hoursAgo(age));
68 app.db
69 .query(
70 `INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid, head_repo_id)
71 VALUES (?1, 'main', ?2, ?3, ?3, ?4)`,
72 )
73 .run(issueId, `b/${++seq}`, ZERO, repo.id);
74 return issueId;
75 };
76 const handed = pull(builder.id, "Handed over pull", 3);
77 for (let r = 0; r < 3; r++)
78 submitReview(app.db, { issueId: handed, authorId: reviewer.id, state: "changes_requested", body: `Round ${r}.`, commitOid: ZERO });
79 pull(alice.id, "Stale pull", 30);
80 pull(builder.id, "Young crew pull", 2);
81 const issue = openIssue(app, alice, repo, { title: "Crew asked about this" });
82 if (!issue.ok) throw new Error(String(issue.error));
83 addComment(app, alice, repo, issue.value, BREAKER_NOTE);
84 const signin = await fetch(`${servers.url}/signin`, {
85 method: "POST",
86 headers: { origin: servers.url },
87 body: new URLSearchParams({ username: "alice", password: "a-good-passphrase" }),
88 redirect: "manual",
89 });
90 cookie = (signin.headers.get("set-cookie") ?? "").split(";")[0]!;
91 }, 30_000);
92
93 afterAll(async () => {
94 await servers.stop();
95 app.close();
96 rmSync(root, { recursive: true, force: true });
97 });
98
99 test("lists exactly the three asks, each with title, repository and why", async () => {
100 const res = await fetch(`${servers.url}/review`, { headers: { cookie } });
101 const body = await res.text();
102 expect(res.status).toBe(200);
103 const rows = body.split('<li').filter((r) => r.includes('class="row"'));
104 expect(rows.length).toBe(3);
105 const find = (t: string) => rows.find((r) => r.includes(t))!;
106 expect(find("Handed over pull")).toContain("three reviews asked for changes");
107 expect(find("Stale pull")).toContain("waiting 30 hours");
108 expect(find("Crew asked about this")).toContain("parked by the crew");
109 for (const r of rows) expect(r).toContain("alice/wait");
110 expect(body).not.toContain("Young crew pull");
111 expect(body).toContain("/alice/wait/pulls/");
112 expect(body).toContain("/alice/wait/issues/");
113 const names = (res.headers.get("server-timing") ?? "").split(", ").map((p) => p.split(";")[0]!);
114 expect(names).toEqual(expect.arrayContaining(["session", "asks", "render"]));
115 });
116
117 test("signed out, /review sends to sign-in", async () => {
118 const res = await fetch(`${servers.url}/review`, { redirect: "manual" });
119 expect(res.status).toBe(303);
120 expect(res.headers.get("location")).toContain("/signin");
121 });
122
123 test("the asks page links to /review", async () => {
124 const res = await fetch(`${servers.url}/asks`, { headers: { cookie } });
125 expect(await res.text()).toContain('href="/review"');
126 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs