Public: anyone can read this; the crew can change it.
Every origin provider says its failures in one set of words #5691
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/5649-every-origin-provider-says-its-failures → main 2 commits fork 1 thread · 1 unresolved
5 files
changed +69
−13
src/origins/providers/forgejo.ts +11 −5
| @@ -43,6 +43,13 @@ import type { | ||
| 43 | 43 | RemoteReviewState, |
| 44 | 44 | RemoteTreeEntry, |
| 45 | 45 | } from "../types.ts"; |
| 46 | import { | |
| 47 | NO_ACCOUNT, | |
| 48 | NO_REVIEW, | |
| 49 | NOT_A_PATH, | |
| 50 | PATH_IS_DIRECTORY, | |
| 51 | PATH_IS_FILE, | |
| 52 | } from "../words.ts"; | |
| 46 | 53 | import { |
| 47 | 54 | ApiClient, |
| 48 | 55 | COMMIT_PAGE, |
| @@ -74,7 +81,6 @@ import { | ||
| 74 | 81 | toCommit, |
| 75 | 82 | toTreeEntry, |
| 76 | 83 | } from "./http.ts"; |
| 77 | import { PATH_IS_DIRECTORY, PATH_IS_FILE } from "../words.ts"; | |
| 78 | 84 | |
| 79 | 85 | /** What we ask for before the instance tells us its own ceiling. */ |
| 80 | 86 | const DEFAULT_LIMIT = 50; |
| @@ -126,7 +132,7 @@ export class ForgejoProvider implements Provider { | ||
| 126 | 132 | if (!result.ok) return result; |
| 127 | 133 | const login = asString(field(result.value.data, "login")); |
| 128 | 134 | if (login === "") |
| 129 | return fail("invalid_response", "the origin named no account"); | |
| 135 | return fail("invalid_response", NO_ACCOUNT); | |
| 130 | 136 | this.uid = asId(field(result.value.data, "id")) || undefined; |
| 131 | 137 | return succeed({ |
| 132 | 138 | login, |
| @@ -241,7 +247,7 @@ export class ForgejoProvider implements Provider { | ||
| 241 | 247 | const named = splitFullName(fullName); |
| 242 | 248 | if (!named) return fail("not_found", `not a repository name: ${fullName}`); |
| 243 | 249 | if (!isSafeRemoteRef(ref) || !isSafeRemotePath(path)) |
| 244 | return fail("not_found", "not a path in a repository"); | |
| 250 | return fail("not_found", NOT_A_PATH); | |
| 245 | 251 | const result = await this.api.call({ |
| 246 | 252 | path: |
| 247 | 253 | `/repos/${seg(named.owner)}/${seg(named.name)}/contents` + |
| @@ -265,7 +271,7 @@ export class ForgejoProvider implements Provider { | ||
| 265 | 271 | const named = splitFullName(fullName); |
| 266 | 272 | if (!named) return fail("not_found", `not a repository name: ${fullName}`); |
| 267 | 273 | if (!isSafeRemoteRef(ref) || path === "" || !isSafeRemotePath(path)) |
| 268 | return fail("not_found", "not a path in a repository"); | |
| 274 | return fail("not_found", NOT_A_PATH); | |
| 269 | 275 | const result = await this.api.call({ |
| 270 | 276 | path: `/repos/${seg(named.owner)}/${seg(named.name)}/contents/${pathSegments(path)}`, |
| 271 | 277 | query: { ref }, |
| @@ -595,7 +601,7 @@ export class ForgejoProvider implements Provider { | ||
| 595 | 601 | if (!result.ok) return result; |
| 596 | 602 | const state = toReviewState(result.value.data); |
| 597 | 603 | return state === undefined |
| 598 | ? fail("invalid_response", "the origin returned no review") | |
| 604 | ? fail("invalid_response", NO_REVIEW) | |
| 599 | 605 | : succeed(state); |
| 600 | 606 | } |
| 601 | 607 |
src/origins/providers/github.ts +11 −5
| @@ -31,6 +31,13 @@ import type { | ||
| 31 | 31 | RemoteReviewState, |
| 32 | 32 | RemoteTreeEntry, |
| 33 | 33 | } from "../types.ts"; |
| 34 | import { | |
| 35 | NO_ACCOUNT, | |
| 36 | NO_REVIEW, | |
| 37 | NOT_A_PATH, | |
| 38 | PATH_IS_DIRECTORY, | |
| 39 | PATH_IS_FILE, | |
| 40 | } from "../words.ts"; | |
| 34 | 41 | import { basicHeader, gitAuthEnv } from "../credentials.ts"; |
| 35 | 42 | import { |
| 36 | 43 | ApiClient, |
| @@ -62,7 +69,6 @@ import { | ||
| 62 | 69 | toCommit, |
| 63 | 70 | toTreeEntry, |
| 64 | 71 | } from "./http.ts"; |
| 65 | import { PATH_IS_DIRECTORY, PATH_IS_FILE } from "../words.ts"; | |
| 66 | 72 | |
| 67 | 73 | const PER_PAGE = 100; |
| 68 | 74 | /** A page walk we drive ourselves (labels, reviews) stops here regardless. */ |
| @@ -93,7 +99,7 @@ export class GitHubProvider implements Provider { | ||
| 93 | 99 | if (!result.ok) return result; |
| 94 | 100 | const login = asString(field(result.value.data, "login")); |
| 95 | 101 | if (login === "") |
| 96 | return fail("invalid_response", "the origin named no account"); | |
| 102 | return fail("invalid_response", NO_ACCOUNT); | |
| 97 | 103 | return succeed({ |
| 98 | 104 | login, |
| 99 | 105 | displayName: asString(field(result.value.data, "name")) || login, |
| @@ -185,7 +191,7 @@ export class GitHubProvider implements Provider { | ||
| 185 | 191 | const named = splitFullName(fullName); |
| 186 | 192 | if (!named) return fail("not_found", `not a repository name: ${fullName}`); |
| 187 | 193 | if (!isSafeRemoteRef(ref) || !isSafeRemotePath(path)) |
| 188 | return fail("not_found", "not a path in a repository"); | |
| 194 | return fail("not_found", NOT_A_PATH); | |
| 189 | 195 | const result = await this.api.call({ |
| 190 | 196 | path: |
| 191 | 197 | `/repos/${seg(named.owner)}/${seg(named.name)}/contents` + |
| @@ -209,7 +215,7 @@ export class GitHubProvider implements Provider { | ||
| 209 | 215 | const named = splitFullName(fullName); |
| 210 | 216 | if (!named) return fail("not_found", `not a repository name: ${fullName}`); |
| 211 | 217 | if (!isSafeRemoteRef(ref) || path === "" || !isSafeRemotePath(path)) |
| 212 | return fail("not_found", "not a path in a repository"); | |
| 218 | return fail("not_found", NOT_A_PATH); | |
| 213 | 219 | const result = await this.api.call({ |
| 214 | 220 | path: `/repos/${seg(named.owner)}/${seg(named.name)}/contents/${pathSegments(path)}`, |
| 215 | 221 | query: { ref }, |
| @@ -565,7 +571,7 @@ export class GitHubProvider implements Provider { | ||
| 565 | 571 | if (!result.ok) return result; |
| 566 | 572 | const state = toReviewState(result.value.data); |
| 567 | 573 | return state === undefined |
| 568 | ? fail("invalid_response", "the origin returned no review") | |
| 574 | ? fail("invalid_response", NO_REVIEW) | |
| 569 | 575 | : succeed(state); |
| 570 | 576 | } |
| 571 | 577 |
src/origins/words.ts +12 −2
| @@ -1,8 +1,18 @@ | ||
| 1 | 1 | /** |
| 2 | * The sentences origin browsing answers with when a path names the wrong | |
| 3 | * kind of thing, said from one place so every provider reads the same. | |
| 2 | * The sentences an origin's providers say when the origin fails them, or when | |
| 3 | * a path names the wrong kind of thing, said from one place so every door | |
| 4 | * that shows them (page, API, MCP, terminal) reads the same words. | |
| 4 | 5 | */ |
| 5 | 6 | |
| 7 | /** The origin answered who the credential is, but named no account. */ | |
| 8 | export const NO_ACCOUNT = "the origin named no account"; | |
| 9 | ||
| 10 | /** The path asked for is not a file or directory in the repository. */ | |
| 11 | export const NOT_A_PATH = "not a path in a repository"; | |
| 12 | ||
| 13 | /** The origin accepted a review but returned none. */ | |
| 14 | export const NO_REVIEW = "the origin returned no review"; | |
| 15 | ||
| 6 | 16 | /** A directory listing was asked of a path that is a file. */ |
| 7 | 17 | export const PATH_IS_FILE = "that path is a file, not a directory"; |
| 8 | 18 |
test/origin-words.test.ts +1 −1
| @@ -15,7 +15,7 @@ const PROVIDERS = [ | ||
| 15 | 15 | const sentences = Object.entries(words) as [string, string][]; |
| 16 | 16 | |
| 17 | 17 | test("each sentence is non-empty", () => { |
| 18 | expect(sentences.length).toBe(2); | |
| 18 | expect(sentences.length).toBe(5); | |
| 19 | 19 | for (const [, s] of sentences) expect(s.trim().length).toBeGreaterThan(0); |
| 20 | 20 | }); |
| 21 | 21 |
test/provider-words.test.ts added +34
| @@ -0,0 +1,34 @@ | ||
| 1 | /** | |
| 2 | * An origin provider's failures are said from src/origins/words.ts alone: | |
| 3 | * no provider types a copy of a sentence back in, so the next edit to the | |
| 4 | * words cannot leave one door saying the old ones. | |
| 5 | */ | |
| 6 | import { describe, expect, test } from "bun:test"; | |
| 7 | import { readFileSync } from "node:fs"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { NO_ACCOUNT, NO_REVIEW, NOT_A_PATH } from "../src/origins/words.ts"; | |
| 10 | ||
| 11 | const providers = [ | |
| 12 | "src/origins/providers/forgejo.ts", | |
| 13 | "src/origins/providers/github.ts", | |
| 14 | ]; | |
| 15 | ||
| 16 | describe("an origin provider's failure words", () => { | |
| 17 | test("say the sentences as they were", () => { | |
| 18 | expect([NO_ACCOUNT, NOT_A_PATH, NO_REVIEW]).toEqual([ | |
| 19 | "the origin named no account", | |
| 20 | "not a path in a repository", | |
| 21 | "the origin returned no review", | |
| 22 | ]); | |
| 23 | }); | |
| 24 | ||
| 25 | for (const file of providers) { | |
| 26 | test(`${file} imports them and types none of them out`, () => { | |
| 27 | const text = readFileSync(join(import.meta.dir, "..", file), "utf8"); | |
| 28 | expect(text).toContain('from "../words.ts"'); | |
| 29 | for (const sentence of [NO_ACCOUNT, NOT_A_PATH, NO_REVIEW]) { | |
| 30 | expect(text).not.toContain(sentence); | |
| 31 | } | |
| 32 | }); | |
| 33 | } | |
| 34 | }); |
new above
offline