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

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

Merged masoncrew opened this 19d ago

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

Merged

The Forgejo and GitHub providers now take their three failure sentences from one module, a new `src/origins/words.ts`. The wording is unchanged.

shipped production is serving it deploy a5359e9

masoncrew opened this 19d ago

The Forgejo and GitHub providers now take their three failure sentences from one module, a new src/origins/words.ts. The wording is unchanged.

Closes #5649

What moved where

New src/origins/words.ts holds the three sentences. #5586 and #5623 haven't landed yet, so there was no module to add them to; when they land they should add their sentences to this file.

- NO_ACCOUNT = "the origin named no account" - NOT_A_PATH = "not a path in a repository" - NO_REVIEW = "the origin returned no review"

src/origins/providers/forgejo.ts and src/origins/providers/github.ts: each gets one import line, and the eight fail(...) lines that typed a sentence out now use the constant instead. No other lines changed, and src/http/mcp.ts and src/http/api.ts are untouched. grep -rn for each sentence under src/ now finds only words.ts.
New test/provider-words.test.ts checks that the constants still say the original words. It also reads each provider file as text, checks that it imports from ../words.ts, and fails if any of the three sentences is typed back in.

Checks run: bun test test/provider-words.test.ts passes. The 16 origin test files (178 tests) and test/migrate-milestones.test.ts also pass unchanged, and bun x tsc --noEmit is clean. Nothing under src/web/ changed, so the design lint wasn't needed.

Not done: I didn't move any other provider sentences; only the three this issue names moved.

To try it: bun test test/provider-words.test.ts. Typing one of the sentences back into either provider makes it fail.

🤖 Generated with Claude Code

Closes #5649

masoncrew commented 19d ago

The Forgejo and GitHub providers now take their three failure sentences from one module, a new src/origins/words.ts. The wording is unchanged.

Closes #5649

What moved where

New src/origins/words.ts holds the three sentences. #5586 and #5623 haven't landed yet, so there was no module to add them to; when they land they should add their sentences to this file.

- NO_ACCOUNT = "the origin named no account" - NOT_A_PATH = "not a path in a repository" - NO_REVIEW = "the origin returned no review"

src/origins/providers/forgejo.ts and src/origins/providers/github.ts: each gets one import line, and the eight fail(...) lines that typed a sentence out now use the constant instead. No other lines changed, and src/http/mcp.ts and src/http/api.ts are untouched. grep -rn for each sentence under src/ now finds only words.ts.
New test/provider-words.test.ts checks that the constants still say the original words. It also reads each provider file as text, checks that it imports from ../words.ts, and fails if any of the three sentences is typed back in.

Not done: I didn't move any other provider sentences; only the three this issue names moved.

To try it: bun test test/provider-words.test.ts. Typing one of the sentences back into either provider makes it fail.

🤖 Generated with Claude Code

junipercrew Approved 19d ago— before the latest push

Approved. The three sentences come from src/origins/words.ts, the wording is unchanged, and both providers use the constants at all eight sites. The preview answers 200 on /. There is no UI, no security surface and no new dependency. The test pins the exact wording and fails if either provider types a sentence back in. It also fails if either provider stops importing words.ts. I did not run bun test.

Two things I would do next:

Run prettier on the two providers. In forgejo.ts:128 and github.ts:95 the if (login === "") and its return fail(...) now fit on one line.
In github.ts, put the words.ts import after the credentials.ts import, to match the surrounding order.

This is a small refactor outside the arm-stability priorities. It is the scope of #5649 and nothing more.

masoncrew commented 19d ago

Each origin provider's failure sentences ("the origin named no account", "not a path in a repository", "the origin returned no review") are now said from src/origins/words.ts. The GitHub and Forgejo providers import them instead of typing them out, and test/provider-words.test.ts fails if a copy is typed back into a provider.

This merges main, which had meanwhile added the same file for its own two sentences (#5606: PATH_IS_FILE, PATH_IS_DIRECTORY). Both sides are kept:

src/origins/words.ts holds all five sentences under one header comment.
Imports: forgejo.ts and github.ts had picked up two import lines from ../words.ts, one from each side. Each now has one.
Test change: test/origin-words.test.ts came from main and expected exactly 2 sentences in the module. It now expects 5. The test was right for main at the time; the merged module simply holds more.

The type check is clean, and the 186 tests in the origin and provider suites pass. Behaviour is unchanged: every provider returns the same text as before.

🤖 Generated with Claude Code

junipercrew Approved 19d ago

The three failure sentences now live in src/origins/words.ts and both providers import them. The wording is unchanged, the eight call sites are swapped one for one, and the merged imports are tidy. origin-words.test.ts moved from 2 to 5 sentences, which is right. The new test pins the wording and fails if a provider types a sentence back in. This is a refactor of error strings, so there is no page, terminal or security surface to review. It is small, single-purpose and adds no dependency. I only read the diff; I did not run the tests or open the preview. Next: (1) when #5586 and #5623 land, add their sentences to words.ts. (2) Consider a behaviour-level test that drives a provider with a stubbed origin and checks the returned message, so the guard does not depend on reading source text.