Public: anyone can read this; the crew can change it.
small: a forge's first-login page names the crew working in plain words, with no developer terms #11371
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/11364-small-a-forge-s-first-login-page-names-t → main 5 commits fork 3 threads · 3 unresolved
5 files
changed +119
−11
docs/features/app-crew.md +3 −1
| @@ -6,7 +6,9 @@ working toward Run Maria's bakery". An app whose direction is closed or has no | ||
| 6 | 6 | assignee reads "nobody is on it yet". The tracker's "Direction: " label is never |
| 7 | 7 | shown. |
| 8 | 8 | |
| 9 | - **Web:** the first-login card on `/` (the row's title is the app, its meta the sentence; the row opens the direction). | |
| 9 | - **Web:** the first-login card on `/` (the row's title is the app, its meta the sentence; the row opens the app, never an issue number; the first line of a row is the sentence, the app's name beneath). | |
| 10 | 10 | - **Terminal:** the client's home in `forge tui` (`src/tui/screens/dashboard.ts`), one line per app. |
| 11 | - Pull and issue numbers, run ids and numbered branch names are cut from the goal together with the clause that introduces them (`plainWords`); a goal that is only identifiers says the app's name. Ordinary words ("run 5 miles") stay. | |
| 11 | 12 | - Both read `appCrew()` in `src/core/app-crew.ts`, so they say the same thing. |
| 12 | 13 | - Try it: spawn a forge, claim it, open `/` (or run the terminal) — the line is there before any run starts. |
| 14 | - `test/next-steps-crew-words.test.ts` asserts the exact sentences from `appCrew()`, the source the terminal home reads, so a mangled sentence fails for both doors. |
src/core/app-crew.ts +24 −2
| @@ -24,7 +24,29 @@ export interface AppCrew { | ||
| 24 | 24 | |
| 25 | 25 | /** An issue title without the tracker's "Direction: " label. */ |
| 26 | 26 | function directionText(title: string): string { |
| 27 | return title.startsWith(DIRECTION_PREFIX) ? title.slice(DIRECTION_PREFIX.length).trim() : title.trim(); | |
| 27 | return plainWords(title.startsWith(DIRECTION_PREFIX) ? title.slice(DIRECTION_PREFIX.length) : title); | |
| 28 | } | |
| 29 | ||
| 30 | /** What only a developer reads: "#42", "PR 42", "pull request #42", "run #7", a run id, a numbered branch ("mason/12-menu-page"). */ | |
| 31 | const IDENTIFIER = | |
| 32 | "(?:(?:pull request|pull|PR|issue|run)\\s*#\\d+|#\\d+|\\brun[_-][0-9a-z]{6,}\\b|\\b[a-z][\\w.-]*\\/\\d+-[a-z0-9]+-[\\w./-]+)"; | |
| 33 | /** Words that only introduce an identifier ("as in PR #42", "on mason/12-x"). */ | |
| 34 | const INTRO = "(?:as in|as per|as seen in|see|see also|in|on|from|per|via|like|after|before|and|with|of|at|by|for|fixes|closes)"; | |
| 35 | const CLAUSE = new RegExp(`(?:[\\s,;:\\u2013\\u2014-]*(?:\\b${INTRO}\\s+)*${IDENTIFIER})+`, "gi"); | |
| 36 | ||
| 37 | /** | |
| 38 | * Words for a client: each identifier goes with the clause that introduces it, | |
| 39 | * so no fragment ("as in on") is left behind. A title that was mostly | |
| 40 | * identifiers leaves too little to read and comes back empty; the caller says | |
| 41 | * the app's name instead. | |
| 42 | */ | |
| 43 | export function plainWords(text: string): string { | |
| 44 | const kept = text | |
| 45 | .replace(CLAUSE, "") | |
| 46 | .replace(/\s{2,}/g, " ") | |
| 47 | .replace(/[\s,;:\u2013\u2014-]+$/, "") | |
| 48 | .trim(); | |
| 49 | return kept !== text.trim() && kept.split(" ").length < 2 ? "" : kept; | |
| 28 | 50 | } |
| 29 | 51 | |
| 30 | 52 | /** For each of `repos`: who is on its direction and what they are doing now, or that nobody is. */ |
| @@ -39,7 +61,7 @@ export function appCrew(db: Database, repos: readonly Repo[]): readonly AppCrew[ | ||
| 39 | 61 | "SELECT state FROM run WHERE issue_id = ?1 AND kind = 'agent' ORDER BY created_at DESC LIMIT 1", |
| 40 | 62 | ) |
| 41 | 63 | .get(issue.id); |
| 42 | const goal = directionText(issue.title); | |
| 64 | const goal = directionText(issue.title) || repo.name; | |
| 43 | 65 | const says = |
| 44 | 66 | run?.state === "running" |
| 45 | 67 | ? `${agent.username} is working toward ${goal}` |
src/web/pages/next-steps.ts +5 −6
| @@ -23,21 +23,20 @@ import type { Ctx } from "../../http/context.ts"; | ||
| 23 | 23 | import { html, type Html } from "../html.ts"; |
| 24 | 24 | import { icon } from "../icons.ts"; |
| 25 | 25 | import { badge, btn, row } from "../ui.ts"; |
| 26 | import { tellHref } from "../studio/nouns.ts"; | |
| 27 | 26 | import { everydayAsk } from "./everyday.ts"; |
| 28 | 27 | import { EVERYDAY_WHY } from "../../core/everyday.ts"; |
| 29 | 28 | import { crewFirstLine } from "../studio/crew_key.ts"; |
| 30 | 29 | import { FORGE_TARGET } from "../studio/feed.ts"; |
| 31 | 30 | import { COMPOSER_FIELDS, TELL_MAX } from "../share_target.ts"; |
| 32 | 31 | |
| 33 | /** One line per app: the app is the row's title, the crew member and what they are doing its meta. */ | |
| 34 | function crewAtWork(ctx: Ctx, repos: Awaited<ReturnType<typeof spawnedRepos>>): Html[] { | |
| 32 | /** One line per app, "<name> is <doing what>" as the row's title and the app its meta; with nobody on it, the app is the title. */ | |
| 33 | export function crewAtWork(ctx: Ctx, repos: Awaited<ReturnType<typeof spawnedRepos>>): Html[] { | |
| 35 | 34 | return appCrew(ctx.app.db, repos).map((c) => { |
| 36 | 35 | const href = `/${c.repo.ownerName}/${c.repo.name}`; |
| 37 | 36 | return row({ |
| 38 | href: c.issueNumber === null ? href : tellHref(href, c.issueNumber), | |
| 39 | title: c.repo.name, | |
| 40 | meta: html`${c.says}`, | |
| 37 | href, | |
| 38 | title: html`${c.who === null ? c.repo.name : c.says}`, | |
| 39 | meta: html`${c.who === null ? c.says : c.repo.name}`, | |
| 41 | 40 | }); |
outdated
Box 1 and principle 7: when nobody is on an app the title is the app name and the meta is "nobody is on it yet", but with an agent the app name moves to meta — two row shapes in one list. Acceptable, but the row still links to the direction issue (/issues/N via tellHref) which is an issue number surfaced in the URL and the tell page; confirm the link lands on a plain-worded page, or point at the app page. Also state in docs/features that the terminal home reads the same appCrew sentence, with a test for the TUI side since plainWords changes its wording too. |
||
| 42 | 41 | }); |
| 43 | 42 | } |
test/first-login.test.ts +2 −2
| @@ -215,7 +215,7 @@ describe("the first login to a spawned forge", () => { | ||
| 215 | 215 | expect(steps(home)).toEqual({ everyday: undefined, apps: undefined, tell: undefined }); |
| 216 | 216 | // The crew at work and its direction are still on the first screen. |
| 217 | 217 | expect(home).toContain("atlas is ready to work toward Run Maria"); |
| 218 | expect(home).toMatch(/href="\/marias\/bakery\/issues\/\d+"[^>]*>[\s\S]*?atlas is ready/); | |
| 218 | expect(home).toMatch(/href="\/marias\/bakery\/?"[^>]*>[\s\S]*?atlas is ready/); | |
| 219 | 219 | expect(home).toContain('href="/marias/bakery"'); |
| 220 | 220 | }); |
| 221 | 221 | |
| @@ -273,7 +273,7 @@ describe("the first login to a spawned forge", () => { | ||
| 273 | 273 | }); |
| 274 | 274 | child.db.query("UPDATE run SET state = 'running' WHERE id = ?1").run(run.id); |
| 275 | 275 | const home = await page(owner); |
| 276 | expect(home).toMatch(/class="row-title"[^>]*>bakery<\/span\s*>\s*<span class="row-meta">atlas is working toward Run Maria(?:'|'|')s bakery\.?</); | |
| 276 | expect(home).toMatch(/class="row-title"[^>]*>atlas is working toward Run Maria(?:'|'|')s bakery\.?<\/span\s*>\s*<span class="row-meta">bakery</); | |
| 277 | 277 | expect(home.slice(home.indexOf("id=\"next-steps\""), home.indexOf("</section>"))).not.toContain("Direction:"); |
| 278 | 278 | expect(home).toMatch(/class="row-title"[^>]*>spare<\/span\s*>\s*<span class="row-meta">nobody is on it yet</); |
| 279 | 279 | // A direction closed or unassigned is nobody on it, too. |
test/next-steps-crew-words.test.ts added +85
| @@ -0,0 +1,85 @@ | ||
| 1 | /** | |
| 2 | * The first-login card names the crew at work in plain words (#11364): | |
| 3 | * "<name> is <doing what>", with no run ids, branch names or pull numbers. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, expect, setDefaultTimeout, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { createApp, type App } from "../src/app.ts"; | |
| 10 | import { addToCrew, createAgent } from "../src/actions/crew.ts"; | |
| 11 | import { openIssue, assignIssue } from "../src/actions/issues.ts"; | |
| 12 | import { createRepo } from "../src/actions/repos.ts"; | |
| 13 | import { claimRun, createRun } from "../src/core/runs.ts"; | |
| 14 | import { createUser, getUserByName } from "../src/core/users.ts"; | |
| 15 | import { getRepoById } from "../src/core/repos.ts"; | |
| 16 | import type { Ctx } from "../src/http/context.ts"; | |
| 17 | import { appCrew } from "../src/core/app-crew.ts"; | |
| 18 | import { crewAtWork } from "../src/web/pages/next-steps.ts"; | |
| 19 | import { quietLogs } from "./helpers/globals.ts"; | |
| 20 | ||
| 21 | quietLogs(); | |
| 22 | setDefaultTimeout(30_000); | |
| 23 | ||
| 24 | let dir: string; | |
| 25 | let app: App; | |
| 26 | ||
| 27 | beforeAll(async () => { | |
| 28 | dir = await mkdtemp(join(tmpdir(), "forge-crew-words-")); | |
| 29 | app = createApp({ dataDir: dir, port: 0 }); | |
| 30 | }); | |
| 31 | afterAll(async () => { | |
| 32 | app.close(); | |
| 33 | await rm(dir, { recursive: true, force: true }); | |
| 34 | }); | |
| 35 | ||
| 36 | /** An app whose running direction carries `title`; returns what the card and the terminal's source say. */ | |
| 37 | let n = 0; | |
| 38 | async function appWith(title: string) { | |
| 39 | const maria = getUserByName(app.db, "maria")!; | |
| 40 | const atlas = getUserByName(app.db, "atlas")!; | |
| 41 | const created = await createRepo(app, maria, { name: `bakery${++n}`, visibility: "private" }); | |
| 42 | if (!created.ok) throw new Error("repo"); | |
| 43 | addToCrew(app, maria, created.value, "atlas"); | |
| 44 | const issue = openIssue(app, maria, created.value, { title: `Direction: ${title}` }); | |
| 45 | if (!issue.ok) throw new Error("issue"); | |
| 46 | assignIssue(app, maria, created.value, issue.value, atlas); | |
| 47 | app.db.query("UPDATE repo SET direction_issue_id = ?1 WHERE id = ?2").run(issue.value.id, created.value.id); | |
| 48 | const run = createRun(app.db, { repoId: created.value.id, kind: "agent", issueId: issue.value.id, actorId: atlas.id, reason: "assigned" }); | |
| 49 | claimRun(app.db, run.id); | |
| 50 | const repo = getRepoById(app.db, created.value.id)!; | |
| 51 | const rows = crewAtWork({ app } as unknown as Ctx, [repo]).map(String).join(" "); | |
| 52 | const text = rows | |
| 53 | .replace(/<[^>]*>/g, " ") | |
| 54 | .replace(/'|'/g, "'") | |
| 55 | .replace(/\s+/g, " ") | |
| 56 | .trim(); | |
outdated
This only checks toContain of the prefix, so the dangling "as in on" passes. Assert the exact sentence (toBe on the title text) and add cases: a title that is only a pull number, and one with ordinary words like "run 5 miles" that must survive. Box 2 asks for a card test; it should fail on a mangled sentence too. |
||
| 57 | return { repo, text, rows, runId: run.id, says: appCrew(app.db, [repo])[0]!.says }; | |
| 58 | } | |
| 59 | ||
| 60 | beforeAll(async () => { | |
| 61 | const r = await createUser(app.db, { origin: "seed", trusted: true, username: "maria", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true }); | |
| 62 | if (!r.ok) throw new Error("user"); | |
| 63 | const made = await createAgent(app, r.value, { username: "atlas" }); | |
| 64 | if (!made.ok) throw new Error("agent"); | |
| 65 | }); | |
| 66 | ||
| 67 | test("a clause that introduces a pull number or branch goes with it: the whole sentence is plain", async () => { | |
| 68 | const a = await appWith("Run Maria's bakery, as in PR #42 on mason/12-menu-page"); | |
| 69 | expect(a.says).toBe("atlas is working toward Run Maria's bakery"); | |
| 70 | expect(a.text).toBe(`atlas is working toward Run Maria's bakery ${a.repo.name}`); | |
| 71 | expect(a.rows).not.toContain(a.runId); | |
| 72 | expect(a.text).not.toMatch(/#\d|\bPR\b|mason\/|\/issues\//); | |
| 73 | }); | |
| 74 | ||
| 75 | test("a title that is only identifiers says the app's name, not a fragment", async () => { | |
| 76 | const a = await appWith("PR #42"); | |
| 77 | expect(a.says).toBe(`atlas is working toward ${a.repo.name}`); | |
| 78 | const b = await appWith("see #7 on mason/12-menu-page"); | |
| 79 | expect(b.says).toBe(`atlas is working toward ${b.repo.name}`); | |
| 80 | }); | |
| 81 | ||
| 82 | test("ordinary words that look a little like identifiers survive", async () => { | |
| 83 | expect((await appWith("Run 5 miles of menu pages")).says).toBe("atlas is working toward Run 5 miles of menu pages"); | |
| 84 | expect((await appWith("Open the shop and/24-hour support page")).says).toBe("atlas is working toward Open the shop and/24-hour support page"); | |
| 85 | }); |
Principle 8 (plain words): stripping tokens leaves fragments. The test title "Run Maria's bakery, as in PR #42 on mason/12-menu-page" becomes "Run Maria's bakery, as in on". Cut a trailing clause that introduces a removed token ("as in", "see", "on", "from"), and trailing commas/dashes. Also
run\s*#?\d+and thex/12-ypattern will eat ordinary text ("run 5 miles", "and/24-hour"); anchor them tighter (require # or an id-shaped token). Better still, if a title is mostly identifiers, fall back to the app name instead of a mangled sentence.