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

Every origin provider says its failures in one set of words #5691

Merged opened this

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

Merged

mason/forge:mason/5649-every-origin-provider-says-its-failures → main 2 commits fork 1 thread · 1 unresolved

  • Merged. mason/5649-every-origin-provider-says-its-failures was merged into main as 1039747.

Review

Sign in to review this change.
5 files changed +69 −13
src/origins/providers/forgejo.ts +11 −5
@@ -43,6 +43,13 @@ import type {
4343 RemoteReviewState,
4444 RemoteTreeEntry,
4545 } 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";
4653 import {
4754 ApiClient,
4855 COMMIT_PAGE,
@@ -74,7 +81,6 @@ import {
7481 toCommit,
7582 toTreeEntry,
7683 } from "./http.ts";
77 import { PATH_IS_DIRECTORY, PATH_IS_FILE } from "../words.ts";
7884
7985 /** What we ask for before the instance tells us its own ceiling. */
8086 const DEFAULT_LIMIT = 50;
@@ -126,7 +132,7 @@ export class ForgejoProvider implements Provider {
126132 if (!result.ok) return result;
127133 const login = asString(field(result.value.data, "login"));
128134 if (login === "")
129 return fail("invalid_response", "the origin named no account");
135 return fail("invalid_response", NO_ACCOUNT);
130136 this.uid = asId(field(result.value.data, "id")) || undefined;
131137 return succeed({
132138 login,
@@ -241,7 +247,7 @@ export class ForgejoProvider implements Provider {
241247 const named = splitFullName(fullName);
242248 if (!named) return fail("not_found", `not a repository name: ${fullName}`);
243249 if (!isSafeRemoteRef(ref) || !isSafeRemotePath(path))
244 return fail("not_found", "not a path in a repository");
250 return fail("not_found", NOT_A_PATH);
245251 const result = await this.api.call({
246252 path:
247253 `/repos/${seg(named.owner)}/${seg(named.name)}/contents` +
@@ -265,7 +271,7 @@ export class ForgejoProvider implements Provider {
265271 const named = splitFullName(fullName);
266272 if (!named) return fail("not_found", `not a repository name: ${fullName}`);
267273 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);
269275 const result = await this.api.call({
270276 path: `/repos/${seg(named.owner)}/${seg(named.name)}/contents/${pathSegments(path)}`,
271277 query: { ref },
@@ -595,7 +601,7 @@ export class ForgejoProvider implements Provider {
595601 if (!result.ok) return result;
596602 const state = toReviewState(result.value.data);
597603 return state === undefined
598 ? fail("invalid_response", "the origin returned no review")
604 ? fail("invalid_response", NO_REVIEW)
599605 : succeed(state);
600606 }
601607
src/origins/providers/github.ts +11 −5
@@ -31,6 +31,13 @@ import type {
3131 RemoteReviewState,
3232 RemoteTreeEntry,
3333 } 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";
3441 import { basicHeader, gitAuthEnv } from "../credentials.ts";
3542 import {
3643 ApiClient,
@@ -62,7 +69,6 @@ import {
6269 toCommit,
6370 toTreeEntry,
6471 } from "./http.ts";
65 import { PATH_IS_DIRECTORY, PATH_IS_FILE } from "../words.ts";
6672
6773 const PER_PAGE = 100;
6874 /** A page walk we drive ourselves (labels, reviews) stops here regardless. */
@@ -93,7 +99,7 @@ export class GitHubProvider implements Provider {
9399 if (!result.ok) return result;
94100 const login = asString(field(result.value.data, "login"));
95101 if (login === "")
96 return fail("invalid_response", "the origin named no account");
102 return fail("invalid_response", NO_ACCOUNT);
97103 return succeed({
98104 login,
99105 displayName: asString(field(result.value.data, "name")) || login,
@@ -185,7 +191,7 @@ export class GitHubProvider implements Provider {
185191 const named = splitFullName(fullName);
186192 if (!named) return fail("not_found", `not a repository name: ${fullName}`);
187193 if (!isSafeRemoteRef(ref) || !isSafeRemotePath(path))
188 return fail("not_found", "not a path in a repository");
194 return fail("not_found", NOT_A_PATH);
189195 const result = await this.api.call({
190196 path:
191197 `/repos/${seg(named.owner)}/${seg(named.name)}/contents` +
@@ -209,7 +215,7 @@ export class GitHubProvider implements Provider {
209215 const named = splitFullName(fullName);
210216 if (!named) return fail("not_found", `not a repository name: ${fullName}`);
211217 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);
213219 const result = await this.api.call({
214220 path: `/repos/${seg(named.owner)}/${seg(named.name)}/contents/${pathSegments(path)}`,
215221 query: { ref },
@@ -565,7 +571,7 @@ export class GitHubProvider implements Provider {
565571 if (!result.ok) return result;
566572 const state = toReviewState(result.value.data);
567573 return state === undefined
568 ? fail("invalid_response", "the origin returned no review")
574 ? fail("invalid_response", NO_REVIEW)
569575 : succeed(state);
570576 }
571577
src/origins/words.ts +12 −2
@@ -1,8 +1,18 @@
11 /**
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.
45 */
56
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
616 /** A directory listing was asked of a path that is a file. */
717 export const PATH_IS_FILE = "that path is a file, not a directory";
818
test/origin-words.test.ts +1 −1
@@ -15,7 +15,7 @@ const PROVIDERS = [
1515 const sentences = Object.entries(words) as [string, string][];
1616
1717 test("each sentence is non-empty", () => {
18 expect(sentences.length).toBe(2);
18 expect(sentences.length).toBe(5);
1919 for (const [, s] of sentences) expect(s.trim().length).toBeGreaterThan(0);
2020 });
2121
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 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs