pit: depend on @moshcoder/moshpit-name instead of keeping a copy - #161
Merged
Conversation
The namespace rules lived here, in TronBrowser's TypeScript resolver, and in a hand port of that into its extension — with a test whose only job was asserting two of those copies still agreed. They are a package now, and this repo becomes a consumer of it rather than one of the three places it is written. 288 lines deleted. What remains is a seam: seven modules import `./lib/moshpit-name.mjs`, so that path re-exports the package rather than each of them learning where a rule moved to. Changing where something lives is not a reason to touch code that only uses it. The package was verified byte-identical to what it replaces before the swap, so this is a move and not a rewrite. 295 tests, unchanged. The cost, stated plainly: a rule now changes in two steps — edit the package, publish, bump here. That is worse for a rule that changes weekly and better for one that has to be identical in three codebases, and these are the second kind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vu1nz Security Review0 finding(s) in PR #? No security issues found. |
ralyodio
added a commit
that referenced
this pull request
Jul 31, 2026
#161 replaced apps/pwa/src/lib/moshpit-name.mjs with a re-export of @moshcoder/moshpit-name. That broke main: three test files now fail at import with ERR_MODULE_NOT_FOUND. I had it wrong. apps/pwa is not part of a pnpm workspace and has no workspaces field, so the repo-root `pnpm install --frozen-lockfile` that CI runs never reaches it — its dependencies are simply absent there, which is why 116 of its tests have always skipped. The claim in #161 that this was "the one consumer that does npm install" was not true; nothing installs them in CI. Worse, the failure mode changed. Those tests used to skip when a dependency was missing. A re-export at the top of a module every one of them imports fails at load instead, so the pure tests — which need nothing — went down with the ones that need a database. So the vendored copy comes back, and gets the same treatment as src/dns.mjs in #162: a dev-only dependency and a drift test that compares behaviour rather than bytes. Parsing across the whole hostname space, the reserved list, prices and limits, pasted lists with their per-line settings, and resolution precedence. Verified both ways this time. It skips cleanly with no dev dependencies, which is what CI has. It fails when ENDING_PRICE_USD is changed in the copy alone. And the full suite with apps/pwa/node_modules removed — CI, exactly — is 484 pass, 0 fail, 122 skipped. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
ralyodio
added a commit
that referenced
this pull request
Aug 1, 2026
install.sh resolves releases/latest, so the sixteen commits merged since v0.13.3 have been sitting on main unreachable — including a fix for a page that locks browsers up. The headline is the pit. /pit rendered every ending an account held and a form per name under each, with no bound on either: at 50 endings x 100 names that was 3.1 MiB of HTML and 36,082 DOM elements, and it managed to jam a browser with no script on the page at all (#167). It now draws a window and says what it is not drawing — 173 KiB, 1,926 elements — with a filter box over the top that takes `eggs` as a substring and `def*` as a glob, debounced against the API (#168). The namespace also stopped being the one part of the product a script could not touch: /api/moshpit/* now accepts the same API key /api/me and /api/sessions already did (#169), and /pit/dns finally documents the TronBrowser route for machines whose DNS is not theirs to change (#165). moshcode: foreign keys are enforced, and the licence package.json claims actually ships (#154) cli: help aliases exit 0 (#157), invalid integration commands fail (#160), `--` is honoured (#159), a BOM before a shebang no longer breaks (#158) skills: engines with no skills primitive are reported, not dropped (#166); `--name` requires a value (#156) mcp: an unsupported flag is rejected rather than registered as the server name (#164) pit: the namespace rules are vendored again with a drift test holding them to the published package (#161, #162, #163) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The namespace rules lived in three places: here, TronBrowser's TypeScript resolver, and a hand port of that into its extension — with a test whose only job was asserting two of the copies still agreed.
They're a package now. This repo becomes a consumer rather than one of the three places the rules are written.
288 lines deleted.
The seam
Seven modules import
./lib/moshpit-name.mjs. That path now re-exports the package rather than each of them learning where a rule moved to — changing where something lives isn't a reason to touch code that only uses it.Verified a move, not a rewrite
diffconfirmed the package is byte-identical to the file it replaces, before the swap. And the runtime resolves throughnode_modules, not a leftover local copy:295/295 tests, unchanged.
The cost, stated plainly
A rule now changes in two steps: edit the package, publish, bump here. That's worse for a rule that changes weekly and better for one that has to be identical across three codebases — and these are the second kind.
Not in this PR
TronBrowser's copies. Its extension has no build step, which is why the hand port exists; consuming a package there needs bundling first.
🤖 Generated with Claude Code