Public: anyone can read this; the crew can change it.
`here` names a person's or an organization's page, and an unknown name, instead of saying "forge" #6281
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6249-here-names-a-person-s-or-an-organization → main 3 commits fork 2 threads · 2 unresolved
3 files
changed +107
−4
src/actions/here.ts +46
| @@ -26,6 +26,7 @@ import { | ||
| 26 | 26 | getRepo, |
| 27 | 27 | type Repo, |
| 28 | 28 | } from "../core/repos.ts"; |
| 29 | import { getUserByName, hiddenFrom } from "../core/users.ts"; | |
| 29 | 30 | import { SYSTEM_NAMES } from "../core/reserved.ts"; |
| 30 | 31 | import { scopeAllows, type TokenScope } from "../core/tokens.ts"; |
| 31 | 32 | import type { User } from "../core/users.ts"; |
| @@ -54,6 +55,8 @@ export interface Action { | ||
| 54 | 55 | export interface Manifest { |
| 55 | 56 | readonly here: |
| 56 | 57 | | { readonly kind: "forge" | "explore" | "search" } |
| 58 | | { readonly kind: "person"; readonly username: string } | |
| 59 | | { readonly kind: "org"; readonly org: string } | |
| 57 | 60 | | { |
| 58 | 61 | readonly kind: "repository"; |
| 59 | 62 | readonly name: string; |
| @@ -111,6 +114,49 @@ export function manifestFor( | ||
| 111 | 114 | name === undefined && (owner === "explore" || owner === "search") |
| 112 | 115 | ? owner |
| 113 | 116 | : undefined; |
| 117 | // One segment, or `/:owner/-/…`, that is not a page the forge keeps for | |
| 118 | // itself names an account: a person, an organization, or nobody. | |
| 119 | if ( | |
| 120 | owner !== undefined && | |
| 121 | page === undefined && | |
| 122 | (name === undefined || name === "-") && | |
| 123 | !SYSTEM.has(owner.toLowerCase()) | |
| 124 | ) { | |
| 125 | const account = getUserByName(app.db, owner); | |
| 126 | if (account === undefined || hiddenFrom(account, viewer)) | |
| 127 | return err("not_found"); | |
| 128 | const you = youFor(viewer, scope, undefined, app); | |
| 129 | const rest = forgeActions(app, viewer, scope); | |
| 130 | if (account.kind === "org") | |
| 131 | return ok({ | |
| 132 | here: { kind: "org", org: account.username }, | |
| 133 | you, | |
| 134 | actions: [ | |
| 135 | { | |
| 136 | do: "list the organization's teams", | |
| 137 | http: `GET /api/v1/orgs/${account.username}/teams`, | |
| 138 | }, | |
| 139 | { | |
| 140 | do: "list its public repositories", | |
| 141 | http: `GET /api/v1/users/${account.username}/repos`, | |
| 142 | }, | |
| 143 | ...rest, | |
| 144 | ], | |
| 145 | ...common, | |
| 146 | }); | |
| 147 | return ok({ | |
| 148 | here: { kind: "person", username: account.username }, | |
| 149 | you, | |
| 150 | actions: [ | |
| 151 | { | |
| 152 | do: "read their public repositories", | |
| 153 | http: `GET /api/v1/users/${account.username}/repos`, | |
| 154 | }, | |
| 155 | ...rest, | |
| 156 | ], | |
| 157 | ...common, | |
| 158 | }); | |
| 159 | } | |
| 114 | 160 | // Two segments under a name the forge keeps for itself are its own page, |
| 115 | 161 | // never a repository (`core/reserved.ts`). |
| 116 | 162 | // `/:owner/-/…` is an account's or an org's own page (its teams), never |
test/here-manifest.test.ts +57 −3
| @@ -236,7 +236,7 @@ describe("the manifest: what you can do here, as who you are", () => { | ||
| 236 | 236 | } |
| 237 | 237 | const q = (await manifest("/search?q=a%20b%26c")).body; |
| 238 | 238 | expect(q.actions[0].http).toBe("GET /api/v1/search?q=a%20b%26c"); |
| 239 | const other = (await manifest("/elsewhere")).body; | |
| 239 | const other = (await manifest("/settings")).body; | |
| 240 | 240 | expect(other.here).toEqual({ kind: "forge" }); |
| 241 | 241 | expect(other.actions[0].do).toBe("explore apps and repositories"); |
| 242 | 242 | }); |
| @@ -307,10 +307,64 @@ describe("the manifest: what you can do here, as who you are", () => { | ||
| 307 | 307 | expect(carol.actions.find((a: any) => a.do === "offer a change").verify).toBeUndefined(); |
| 308 | 308 | }); |
| 309 | 309 | |
| 310 | test("an org's own pages are the forge's, not a missing repository", async () => { | |
| 310 | test("an org's own pages are the org's, not a missing repository", async () => { | |
| 311 | 311 | const res = await here("/acme/-/teams/Writers", "alice"); |
| 312 | 312 | expect(res.status).toBe(200); |
| 313 | expect(((await res.json()) as any).here.kind).toBe("forge"); | |
| 313 | expect(((await res.json()) as any).here).toEqual({ kind: "org", org: "acme" }); | |
| 314 | }); | |
| 315 | ||
| 316 | test("a suspended person's page is not_found like an unknown name, except to an admin", async () => { | |
| 317 | const made = await createUser(app.db, { | |
| 318 | origin: "seed", | |
| 319 | trusted: true, | |
| 320 | username: "gone", | |
| 321 | email: "[email protected]", | |
| 322 | password: "a-good-passphrase", | |
| 323 | }); | |
| 324 | if (!made.ok) throw new Error(made.error); | |
outdated
The unknown-name case is covered here, but the hidden-account case next to it is not. Suspend or remove a user, then assert |
||
| 325 | app.db.run("UPDATE user SET suspended_at = ?1 WHERE id = ?2", [ | |
| 326 | new Date().toISOString(), | |
| 327 | made.value.id, | |
| 328 | ]); | |
| 329 | const unknown = await (await here("/nope")).json(); | |
| 330 | for (const who of [undefined, "carol"]) { | |
| 331 | const res = await here("/gone", who); | |
| 332 | expect(res.status).toBe(404); | |
| 333 | expect(await res.json()).toEqual(unknown); | |
| 334 | } | |
| 335 | const { body } = await manifest("/gone", "alice"); | |
| 336 | expect(body.here).toEqual({ kind: "person", username: "gone" }); | |
| 337 | }); | |
| 338 | ||
| 339 | test("one segment is a person, an org, nobody, or one of the forge's own names", async () => { | |
| 340 | const person = (await manifest("/wes")).body; | |
| 341 | expect(person.here).toEqual({ kind: "person", username: "wes" }); | |
| 342 | expect(person.actions[0].http).toBe("GET /api/v1/users/wes/repos"); | |
| 343 | const org = (await manifest("/ACME", "carol")).body; | |
| 344 | expect(org.here).toEqual({ kind: "org", org: "acme" }); | |
| 345 | expect(org.actions[0].http).toBe("GET /api/v1/orgs/acme/teams"); | |
| 346 | const nobody = await here("/nope"); | |
| 347 | expect(nobody.status).toBe(404); | |
| 348 | expect(await nobody.json()).toEqual(await (await here("/acme/nothing")).json()); | |
| 349 | expect((await manifest("/settings")).body.here.kind).toBe("forge"); | |
| 350 | expect((await manifest("/explore")).body.here.kind).toBe("explore"); | |
| 351 | expect((await manifest("/search?q=x")).body.here.kind).toBe("search"); | |
| 352 | const mcp = (await ( | |
| 353 | await fetch(`${base}/mcp`, { | |
| 354 | method: "POST", | |
| 355 | headers: { | |
| 356 | "content-type": "application/json", | |
| 357 | authorization: `Bearer ${tokens["carol"]}`, | |
| 358 | }, | |
| 359 | body: JSON.stringify({ | |
| 360 | jsonrpc: "2.0", | |
| 361 | id: 9, | |
| 362 | method: "tools/call", | |
| 363 | params: { name: "here", arguments: { url: `${base}/wes` } }, | |
| 364 | }), | |
| 365 | }) | |
| 366 | ).json()) as any; | |
| 367 | expect(JSON.parse(mcp.result.content[0].text).here.kind).toBe("person"); | |
| 314 | 368 | }); |
| 315 | 369 | |
| 316 | 370 | test("a pull page says whether the pull's code runs, or waits for a writer's approval", async () => { |
test/openapi.test.ts +4 −1
| @@ -13,7 +13,10 @@ test("every http route the manifest names is described", () => { | ||
| 13 | 13 | const named = [...src.matchAll(/http: [`"]([^`"]+)[`"]/g)].flatMap((m) => |
| 14 | 14 | m[1]!.split(", then ").map((one) => { |
| 15 | 15 | const [method, path] = one.replace(" {headRepo}", "").replace(/\?.*/, "").split(" "); |
| 16 | return [method!.toLowerCase(), path!.replaceAll("${r}", "/api/v1/repos/{owner}/{name}")] as const; | |
| 16 | return [method!.toLowerCase(), path! | |
| 17 | .replaceAll("${r}", "/api/v1/repos/{owner}/{name}") | |
| 18 | .replace("/orgs/${account.username}/", "/orgs/{org}/") | |
| 19 | .replace("/users/${account.username}/", "/users/{username}/")] as const; | |
| 17 | 20 | }), |
| 18 | 21 | ); |
| 19 | 22 | expect(named.length).toBeGreaterThan(5); |
new above
offline
This gate is the only thing that keeps a suspended or removed account from showing up as
{kind:"person"}. No test fails if it is deleted. Add a test: suspend a user, then assert anonymous and non-admin get 404 (same body as an unknown name) and an admin gets the person.