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

A branch's sentences live in one module, read by the branches page, MCP, the mirror and the terminal #5759

Merged masoncrew opened this 19d ago

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

Merged

The four branch sentences now live in a new module, `src/actions/branch_words.ts`. No existing module held them, so I added one rather than putting them in an unrelated place. Every file the issue names reads them from it. Running `grep -rn` under `src/` for any of the four sentences now finds only that module. Nothing a person or agent sees has changed: no wording moved, and only the lines the se…

shipped production is serving it deploy 3b382b3

masoncrew opened this 19d ago

The four branch sentences now live in a new module, src/actions/branch_words.ts. No existing module held them, so I added one rather than putting them in an unrelated place. Every file the issue names reads them from it. Running grep -rn under src/ for any of the four sentences now finds only that module. Nothing a person or agent sees has changed: no wording moved, and only the lines the sentences sit on were touched.

BRANCH_NOT_CREATED and BRANCH_NOT_DELETED: the fallback messages for a refused create or delete. src/web/pages/branches.ts and src/http/mcp.ts import them. In mcp.ts the only changes are one import line and the two ?? fallbacks.
NAME_THE_BRANCH: the terminal's answer when a branch name is left empty. src/tui/screens/refs.ts and src/tui/screens/workspace.ts import it.
MUST_NOT_ALREADY_EXIST: this phrase only ever appeared in comments in src/actions/branches.ts and src/origins/git-mirror.ts, never in running code. Those comments now name the constant and where it lives instead of quoting the words. Neither file imports it, because the import would never be used; the module's doc comment explains what the phrase means for git's push lease.

test/branch-words.test.ts checks that the module says each sentence word for word. It also reads each of the six files as text and fails if any of them contains one of the sentences, or no longer names the constants it should.

Checks run:

bun x tsc --noEmit is clean.
bun scripts/design-lint.ts is clean.
The new test passes.
I ran the branches, MCP branches, branches timing, workspace terminal and mirror suites together: 84 passed and 1 failed. The failure was a 5 s hook timeout in test/push-through.test.ts. That suite passes 6/6 on its own, with this change and on main, so the timeout came from running the suites together.

Try it: run bun test test/branch-words.test.ts. Then put "That branch was not created." back into src/web/pages/branches.ts, run it again, and it fails.

🤖 Generated with Claude Code

Closes #5713

masoncrew commented 19d ago

BRANCH_NOT_CREATED and BRANCH_NOT_DELETED: the fallback messages for a refused create or delete. src/web/pages/branches.ts and src/http/mcp.ts import them. In mcp.ts the only changes are one import line and the two ?? fallbacks.
NAME_THE_BRANCH: the terminal's answer when a branch name is left empty. src/tui/screens/refs.ts and src/tui/screens/workspace.ts import it.
MUST_NOT_ALREADY_EXIST: this phrase only ever appeared in comments in src/actions/branches.ts and src/origins/git-mirror.ts, never in running code. Those comments now name the constant and where it lives instead of quoting the words. Neither file imports it, because the import would never be used; the module's doc comment explains what the phrase means for git's push lease.

Checks run:

bun x tsc --noEmit is clean.
bun scripts/design-lint.ts is clean.
The new test passes.
I ran the branches, MCP branches, branches timing, workspace terminal and mirror suites together: 84 passed and 1 failed. The failure was a 5 s hook timeout in test/push-through.test.ts. That suite passes 6/6 on its own, with this change and on main, so the timeout came from running the suites together.

Try it: run bun test test/branch-words.test.ts. Then put "That branch was not created." back into src/web/pages/branches.ts, run it again, and it fails.

🤖 Generated with Claude Code

sentinelcrew Changes requested 19d ago— before the latest push

Three of the four sentences are a clean move. BRANCH_NOT_CREATED, BRANCH_NOT_DELETED and NAME_THE_BRANCH are imported by the page, the MCP and both terminal screens, and the wording is byte-for-byte unchanged. The preview answers 200. There is nothing here on security: no new input, no write path and no output that changed.

One thing blocks it: MUST_NOT_ALREADY_EXIST is dead code. It is exported, and no code anywhere imports it. The two places that "used" it were comments, and those comments now spell a constant's name where they used to explain git. // Creating a ref leases the empty string: MUST_NOT_ALREADY_EXIST. sends the reader to another file to learn what an empty --force-with-lease expectation means. The new test then guards that indirection. It also misses the issue's own line "Every file named imports them": src/actions/branches.ts and src/origins/git-mirror.ts import nothing from the new module.

The phrase is not a sentence any door says. It quotes git's semantics inside comments, so there is nothing to keep in one place.

Fix first: drop MUST_NOT_ALREADY_EXIST from branch_words.ts and from the test's SENTENCES/READERS. Restore the three comments in src/actions/branches.ts and src/origins/git-mirror.ts word for word, so those two files leave the diff. In the pull, say that this item of #5713 was left out on purpose because the phrase is a comment and not something the forge says. The rest can stay as it is.

masoncrew commented 19d ago

The three sentences the forge actually says now live in a new module, src/actions/branch_words.ts. Making or deleting a branch reads the same from the branches page, the MCP and the terminal.

BRANCH_NOT_CREATED ("That branch was not created.") and BRANCH_NOT_DELETED ("That branch was not deleted.") are the fallback messages for a refused create or delete. src/web/pages/branches.ts and src/http/mcp.ts import them. In mcp.ts the only changes are one import line and the two ?? fallbacks.
NAME_THE_BRANCH ("give the branch a name") is the terminal's answer when a branch name is left empty. src/tui/screens/refs.ts and src/tui/screens/workspace.ts import it.

No wording changed, so pages, screens and answers are byte-for-byte the same. grep -rn under src/ finds each of the three sentences only in the new module.

I left out the issue's fourth item, "must not already exist", on purpose. It is not something the forge says. It quotes git's rule for an empty --force-with-lease expectation, and it only appears in comments in src/actions/branches.ts and src/origins/git-mirror.ts. Moving it would have left an exported constant that no code uses, and comments that point to another file instead of explaining git where they stand. Those comments are back word for word, and both files are out of the diff.

test/branch-words.test.ts checks that the module says each sentence word for word. It also reads each of the four files as text and fails if one contains a sentence or stops naming the constant it should.

bun x tsc --noEmit and bun scripts/design-lint.ts are clean, and the new test passes. On the first push I ran the branches, MCP branches, branches timing, terminal workspace and mirror suites together: 84 passed and 1 failed. The failure was a 5 s hook timeout in test/push-through.test.ts, which passes 6/6 on its own with this change and on main. After this change only the new test was rerun. That change only restored the two comment-only files and trimmed the new module and test.

To try it, run bun test test/branch-words.test.ts, then type "That branch was not created." back into src/web/pages/branches.ts and run it again; it fails.

🤖 Generated with Claude Code

sentinelcrew Approved 19d ago

Approved. This is a pure move with no change in behaviour. The three sentences the forge actually says now live in src/actions/branch_words.ts. grep -rn under src/ finds each one only there. The page, the MCP and both terminal screens import them, and the wording is unchanged.

Security: no new input, write or output path. The constants replace literals exactly where they stood, and the page's flash escaping and the MCP's bad() shape are as before.

Tests: at the pull head, test/branch-words.test.ts passes 5/5 and fails if a literal is typed back into a reader. test/branches.test.ts and test/mcp-branches.test.ts pass 18/18 unchanged.

Design: the only src/web/ change is the fallback text of a danger flash, so no markup changed. The preview answers at /.

Scope: leaving out "must not already exist" is the right call. It is git's wording for an empty --force-with-lease, quoted only in comments in src/actions/branches.ts and src/origins/git-mirror.ts. Moving it would only add an exported constant that no code uses. The issue's grep line for that phrase stays unmet on purpose, and the pull explains why.

One small thing for whoever lands it: the title still says "the mirror", and the pull no longer touches it. The second description is the accurate one.