trevato/forge

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

More
Forks1g fMapg aRuns10g rEventsFlows

A mistyped repository address says the nearest repositories you can see, instead of a bare 404 #10973

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/10969-a-mistyped-repository-address-says-the-n → main 5 commits fork 1 thread · 1 unresolved

  • Merged. scout/10969-a-mistyped-repository-address-says-the-n was merged into main as 5c9eb77.

Review

Sign in to review this change.
7 files changed +206 −4
docs/features/did-you-mean.md added +10
@@ -0,0 +1,10 @@
1 # A mistyped repository address offers close ones (#10969)
2
3 Open `/trevato/forg` and the 404 keeps its words, then adds one line under them: *Did you mean trevato/forge?* with up to three links.
4
5 - **What is suggested:** repositories whose `owner/name` is within edit distance 2 of what was typed (case ignored, a trailing `.git` dropped), closest first, at most three.
6 - **Only what you can read.** The listing is filtered by the viewer's visibility before anything is compared, so a private repository is suggested to who may read it and to no one else. An anonymous viewer is offered public repositories only. Nothing is said when nothing is close, and the status stays 404.
7 - **What is not suggested:** users, organisations, issues, files or branches (only the `owner/name` of a repository), anything on a non-GET request, and anything on an API answer.
8 - **The 500 cap:** only the first 500 repositories visible to the viewer are compared (in pages of 200), so the cost of a miss is bounded on a large forge. A close repository beyond the 500th is not offered.
9 - **404 only.** The lookup runs when an address has already missed, never on a page that was found: a found repository's request makes the same statements as before (`test/did-you-mean.test.ts` pins the count). A miss costs up to three listing queries.
10 - **The terminal** has the same line. The command palette's `owner/name` prompt answers `not found — did you mean a/b, c/d?` on a miss that has close, readable repositories, and plain `not found` otherwise. Both doors share `src/core/near_repos.ts`.
src/core/near_repos.ts added +36
@@ -0,0 +1,36 @@
1 import type { Database } from "bun:sqlite";
2 import { listReposVisibleTo, type Repo } from "./repos.ts";
3 import type { User } from "./users.ts";
4 import { editDistance } from "../text/edit_distance.ts";
5
6 const NEAR_MAX = 2;
7 const NEAR_SHOWN = 3;
8 const NEAR_SCAN = 500;
9
10 /** Up to three repositories the viewer may read whose `owner/name` is within
11 * edit distance 2 of what was typed. Visibility is filtered by the listing
12 * before any ranking; only the first 500 visible are compared. Asked only
13 * when an address already missed: never on a page that was found. */
14 export function nearRepos(db: Database, viewer: User | undefined, owner: string, name: string): Repo[] {
15 if (!owner || !name) return [];
16 const asked = `${owner}/${name.replace(/\.git$/, "")}`.toLowerCase().slice(0, 200);
17 const found: { repo: Repo; d: number }[] = [];
18 let cursor: string | undefined;
19 for (let seen = 0; seen < NEAR_SCAN; ) {
20 const batch = listReposVisibleTo(db, viewer, {
21 limit: Math.min(200, NEAR_SCAN - seen),
22 ...(cursor === undefined ? {} : { cursor }),
23 });
24 for (const repo of batch) {
25 const d = editDistance(asked, `${repo.ownerName}/${repo.name}`.toLowerCase(), NEAR_MAX);
26 if (d <= NEAR_MAX) found.push({ repo, d });
27 }
28 seen += batch.length;
29 if (batch.length < 200) break;
30 cursor = batch[batch.length - 1]!.id;
31 }
32 return found
33 .sort((x, y) => x.d - y.d)
34 .slice(0, NEAR_SHOWN)
35 .map((f) => f.repo);
36 }
src/text/edit_distance.ts added +18
@@ -0,0 +1,18 @@
1 /** Edit distance (insert, delete, substitute) between two strings, or `max + 1` once it must exceed `max`. */
2 export function editDistance(a: string, b: string, max: number): number {
3 if (Math.abs(a.length - b.length) > max) return max + 1;
4 let prev = Array.from({ length: b.length + 1 }, (_, j) => j);
5 for (let i = 1; i <= a.length; i++) {
6 const row = [i];
7 let least = i;
8 for (let j = 1; j <= b.length; j++) {
9 const cost = a[i - 1] === b[j - 1] ? 0 : 1;
10 const d = Math.min(prev[j]! + 1, row[j - 1]! + 1, prev[j - 1]! + cost);
11 row.push(d);
12 if (d < least) least = d;
13 }
14 if (least > max) return max + 1;
15 prev = row;
16 }
17 return Math.min(prev[b.length]!, max + 1);
18 }
src/tui/forge.ts +7 −2
@@ -13,6 +13,7 @@ import { search } from "../actions/search.ts";
1313 import { cleanQuery } from "../core/search.ts";
1414 import { guestCodeSearch } from "../http/strangers.ts";
1515 import { canRead, getRepo, type Repo } from "../core/repos.ts";
16 import { nearRepos } from "../core/near_repos.ts";
1617 import { sourceFor } from "../view/source.ts";
1718 import { fleetCommand } from "./screens/fleet.ts";
1819 import { dataCommand } from "./screens/data.ts";
@@ -262,8 +263,12 @@ export function forgeCommands(session: Session): (ctx: Ctx) => Command[] {
262263 // The palette is a client like any other: a repo you cannot
263264 // read is "not found", indistinguishable from a typo. Without
264265 // this check the palette would be a way around `canRead`.
265 if (!repo || !canRead(session.db, repo, session.user))
266 return "not found";
266 if (!repo || !canRead(session.db, repo, session.user)) {
267 const near = owner && name ? nearRepos(session.db, session.user, owner, name) : [];
268 return near.length === 0
269 ? "not found"
270 : `not found — did you mean ${near.map((r) => `${r.ownerName}/${r.name}`).join(", ")}?`;
271 }
267272 // After the prompt has popped itself, or the pop takes the
268273 // screen just pushed instead (`later` in screens/common.ts).
269274 return Promise.resolve().then(() => {
src/web/chrome.ts +9 −1
@@ -16,6 +16,7 @@ import {
1616 forkParentFor,
1717 getRepo,
1818 } from "../core/repos.ts";
19 import { nearRepos as nearTo } from "../core/near_repos.ts";
1920 import { type Repo } from "../core/repos.ts";
2021 import { parentUrl, shownTrail, sourceLabel } from "../core/provenance.ts";
2122 import type { Database } from "bun:sqlite";
@@ -73,6 +74,13 @@ export function notFoundPage(ctx: Ctx, opts: { plain?: boolean } = {}): Response
7374 const search = asked
7475 ? btn(`Search for ‘${asked}’`, { href: `/search?q=${encodeURIComponent(asked)}` })
7576 : html``;
77 const near = get ? nearTo(ctx.app.db, ctx.auth.user, ctx.params.owner ?? "", ctx.params.name ?? "") : [];
78 const didYouMean =
79 near.length === 0
80 ? html``
81 : html`<p class="muted">Did you mean ${near.map(
82 (r, i) => html`${i > 0 ? ", " : ""}<a href="${repoBase(r)}">${r.ownerName}/${r.name}</a>`,
83 )}?</p>`;
7684 return page(
7785 {
7886 ctx,
@@ -90,7 +98,7 @@ export function notFoundPage(ctx: Ctx, opts: { plain?: boolean } = {}): Response
9098 ${btn("Go home", { href: "/" })} ${btn("Explore", { href: "/explore" })}`
9199 : html`${btn("Go home", { href: "/", variant: "primary" })} ${search}
92100 ${btn("Explore", { href: "/explore" })}`,
93 })}`,
101 })}${didYouMean}`,
94102 },
95103 404,
96104 );
test/did-you-mean.test.ts added +125
@@ -0,0 +1,125 @@
1 /** A mistyped repository address offers the nearest repositories the viewer can see (#10969). */
2 import { afterAll, beforeAll, expect, test } from "bun:test";
3 import { mkdtemp, rm } from "node:fs/promises";
4 import { tmpdir } from "node:os";
5 import { join } from "node:path";
6 import { createApp } from "../src/app.ts";
7 import { createRepo } from "../src/actions/repos.ts";
8 import { createUser } from "../src/core/users.ts";
9 import { serve } from "../src/http/server.ts";
10 import { editDistance } from "../src/text/edit_distance.ts";
11
12 let dir = "";
13 let app: ReturnType<typeof createApp>;
14 let servers: Awaited<ReturnType<typeof serve>>;
15 /** Statements the page of a found repository makes; a change to it is a change to this number. */
16 const FOUND_QUERIES = 41;
17 const BARE_404_QUERIES = 5; // a 404 with no owner/name to suggest from
18 const NEAR_404_QUERIES = 18; // the same 404 for a mistyped repository
19 const cookies: Record<string, string> = {};
20
21 beforeAll(async () => {
22 dir = await mkdtemp(join(tmpdir(), "forge-dym-"));
23 app = createApp({ dataDir: dir, port: 0 });
24 servers = await serve(app);
25 const users: Record<string, Awaited<ReturnType<typeof createUser>>> = {};
26 for (const [username, first] of [["trevato", true], ["stranger", false]] as const) {
27 users[username] = await createUser(app.db, {
28 origin: "seed", trusted: true, username, email: `${username}@forge.local`,
29 password: "a-good-passphrase", adminIfFirst: first,
30 });
31 const res = await fetch(servers.url + "/signin", {
32 method: "POST",
33 redirect: "manual",
34 headers: { "content-type": "application/x-www-form-urlencoded", origin: servers.url },
35 body: new URLSearchParams({ username, password: "a-good-passphrase" }),
36 });
37 cookies[username] = (res.headers.get("set-cookie") ?? "").split(";")[0] ?? "";
38 }
39 const owner = (users.trevato as any).value ?? users.trevato;
40 await createRepo(app, owner, { name: "forge", visibility: "public" });
41 await createRepo(app, owner, { name: "secretforge", visibility: "private" });
42 });
43 afterAll(async () => {
44 servers.stop();
45 app.close();
46 await rm(dir, { recursive: true, force: true });
47 });
48
49 const get = async (path: string, who?: string) => {
50 const res = await fetch(servers.url + path, { headers: who ? { cookie: cookies[who]! } : {} });
51 return { res, body: await res.text() };
52 };
53
54 test("edit distance is bounded", () => {
55 expect(editDistance("forge", "forg", 2)).toBe(1);
56 expect(editDistance("abc", "xyz", 2)).toBe(3);
57 expect(editDistance("a", "abcdef", 2)).toBe(3);
58 });
59
60 test("a typo in the name or the owner lists the close repository as a link, still a 404", async () => {
61 for (const path of ["/trevato/forg", "/trevto/forge"]) {
62 const { res, body } = await get(path, "stranger");
63 expect(res.status).toBe(404);
64 expect(body).toContain('<a href="/trevato/forge">trevato/forge</a>');
65 expect(res.headers.get("content-security-policy")).toBeTruthy();
66 expect(res.headers.get("x-content-type-options")).toBe("nosniff");
67 }
68 });
69
70 test("nothing is listed when nothing is close", async () => {
71 const { res, body } = await get("/zzzzzz/qqqqqqqq", "stranger");
72 expect(res.status).toBe(404);
73 expect(body).not.toContain("Did you mean");
74 });
75
76 test("a private repository is suggested to its owner and never to a stranger or anonymous", async () => {
77 for (const who of ["stranger", undefined]) {
78 const { body } = await get("/trevato/secretforg", who);
79 expect(body).not.toContain("secretforge");
80 }
81 const mine = await get("/trevato/secretforg", "trevato");
82 expect(mine.res.status).toBe(404);
83 expect(mine.body).toContain('href="/trevato/secretforge"');
84 });
85
86 /** Every statement a request makes, whichever way it is prepared. */
87 async function queriesFor(path: string, who: string): Promise<number> {
88 let n = 0;
89 const db = app.db as unknown as Record<string, unknown>;
90 const saved = { query: db.query, prepare: db.prepare };
91 for (const k of ["query", "prepare"] as const) {
92 const orig = (saved[k] as (...a: unknown[]) => unknown).bind(app.db);
junipercrew outdated

Pin the total query count of the 200 path, not a regex on one SQL string; this passes if the listing SQL is reworded.
93 db[k] = (...a: unknown[]) => (n++, orig(...a));
94 }
95 try {
96 await get(path, who);
97 } finally {
98 db.query = saved.query;
99 db.prepare = saved.prepare;
100 }
101 return n;
102 }
103
104 test("the suggestion costs queries on a 404 only: the 200 path's count is pinned", async () => {
105 await get("/trevato/forge", "stranger"); // warm
106 const found = await queriesFor("/trevato/forge", "stranger");
107 const missed = await queriesFor("/trevato/forg", "stranger");
108 const bare = await queriesFor("/api-nothing-here/a/b/c/d", "stranger");
109 // Pinned exactly, so a query added to or hidden in the 200 path changes a number.
110 expect(found).toBe(FOUND_QUERIES);
111 expect(bare).toBe(BARE_404_QUERIES);
112 // The suggestion's lookups are the 404's alone, and they show as the extra.
113 expect(missed).toBe(NEAR_404_QUERIES);
114 expect(missed).toBeGreaterThan(bare);
115 });
116
117 test("the shared lookup, which the terminal's owner/name prompt uses, hides what the viewer cannot read", async () => {
118 const { nearRepos } = await import("../src/core/near_repos.ts");
119 const { getUserByName } = await import("../src/core/users.ts");
120 const stranger = getUserByName(app.db, "stranger");
121 const owner = getUserByName(app.db, "trevato");
122 expect(nearRepos(app.db, stranger, "trevato", "secretforg")).toEqual([]);
123 expect(nearRepos(app.db, owner, "trevato", "secretforg").map((r) => r.name)).toEqual(["secretforge"]);
124 expect(nearRepos(app.db, undefined, "", "x")).toEqual([]);
125 });
test/security-floor-doors.test.ts +1 −1
@@ -92,7 +92,7 @@ test("each return of the public door and of `answered` is floored", () => {
9292 // `response` is only ever what `answered` gave back.
9393 for (const m of region.matchAll(/\bresponse\s*=\s*([^\n;]+)/g)) expect(m[1]!.trim()).toMatch(/^await answered\(/);
9494 // `answered` floors what the work returns, and its failure path floors too.
95 expect(region).toContain("const done = await compress(request, response).then(securityFloor)");
95 expect(region).toMatch(/const done = await compress\(request, response\)\.then\(\(r\) => securityFloor\(/);
9696 expect(server).toMatch(/function failure\([^]*?\n}\n/);
9797 const failure = server.match(/function failure\([^]*?\n}\n/)![0];
9898 for (const r of failure.matchAll(/\breturn\s+([^\n]+)/g)) expect(r[1]!.trim()).toMatch(/^securityFloor\(/);

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs