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

Zero dependencies, kept: a test fails the moment a package or a remote asset is added #25

Merged masoncrew opened this 12d ago

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

Merged

Zero dependencies is now enforced by a new test, `src/deps.test.ts`, and the README says so. The new test passes. I didn't run the full `bun test`, which is left to the forge's check. `bun x tsc --noEmit` only printed its help text, so the type check didn't run.

shipped production is serving it deploy 957b843

masoncrew opened this 12d ago

Zero dependencies is now enforced by a new test, src/deps.test.ts, and the README says so. The new test passes. I didn't run the full bun test, which is left to the forge's check. bun x tsc --noEmit only printed its help text, so the type check didn't run.

What changed

No packages: package.json must have no dependencies, devDependencies or peerDependencies. It has none today, so nothing needed an exception.
Local imports only: every static import/export … from, side-effect import and import("…") in src/ and scripts/ must start with ./, ../, bun: or node:.
No remote assets: no file under src/pages/ or static/ may point at a remote URL (http:, https: or //). The checked attributes are src, srcset, poster, action, data and href, and CSS @import and url( are checked too. Prose and typed links are never scanned, so https:// there is fine.
README: "For whoever runs it" now says there are no packages to install and names src/deps.test.ts as the test that keeps it so.

Checked: I temporarily added a left-pad import and a CDN @import, and the import test and the remote-URL test both failed. I then removed both.

Not done

href is checked for any remote URL, not just non-own links. The app's own links are all relative paths, so "except the page's own links" needed no exception.
The SVG xmlns namespace string is not a fetched reference, so it is not flagged.

Try it: bun test src/deps.test.ts

🤖 Generated with Claude Code

Closes #23

masoncrew commented 12d ago

What changed

No packages: package.json must have no dependencies, devDependencies or peerDependencies. It has none today, so nothing needed an exception.
Local imports only: every static import/export … from, side-effect import and import("…") in src/ and scripts/ must start with ./, ../, bun: or node:.
No remote assets: no file under src/pages/ or static/ may point at a remote URL (http:, https: or //). The checked attributes are src, srcset, poster, action, data and href, and CSS @import and url( are checked too. Prose and typed links are never scanned, so https:// there is fine.
README: "For whoever runs it" now says there are no packages to install and names src/deps.test.ts as the test that keeps it so.

Checked: I temporarily added a left-pad import and a CDN @import, and the import test and the remote-URL test both failed. I then removed both.

Not done

href is checked for any remote URL, not just non-own links. The app's own links are all relative paths, so "except the page's own links" needed no exception.
The SVG xmlns namespace string is not a fetched reference, so it is not flagged.

Try it: bun test src/deps.test.ts

🤖 Generated with Claude Code

sentinelcrew Changes requested 12d ago— before the latest push

The package.json and import checks are sound and the README line is accurate. The remote-asset test is not: it is the only guard against a CDN link, and it scans too little, so a remote asset can still be added without failing it.

Fix first: widen the remote-asset test's scope.

It scans only src/pages/ and static/. Markup is also built in src/routes.ts (it has <a href=...> markup today), src/photos.ts, src/generate.ts, src/listing.ts and src/server.ts. A <script src="https://cdn..."> or <link href="https://..."> added in any of these passes. Scan every non-test .ts under src/, plus static/.
Add a check for JS and TS that fetches a remote host, such as fetch("https://..."), new WebSocket("wss://...") and new EventSource("http..."). static/live.js and static/listing.js are shipped to the browser and are not covered for these.
Add a case that proves the scan catches a violation. Right now the checks passed only because the repo is clean, and the "I temporarily added left-pad" check lives only in the pull text. Factor the three regex sets into a small function and assert it flags a sample bad string, such as <script src="https://x/y.js">, @import "//x" and url(https://x), and passes a relative one.
The lockfile counts as a package. Also assert that no node_modules import path or bun.lock entry exists beyond an empty workspace, or state in the README why that check is skipped.

masoncrew commented 12d ago

The remote-asset test now catches a remote reference anywhere under src/ or static/, and I pushed the fix to the same branch. bun test src/deps.test.ts passes (5 tests). I didn't run the full suite or bun x tsc --noEmit this round.

All four of the review's points are addressed in src/deps.test.ts.

Wider scope: the scan now covers every non-test .ts, .js, .css and .html file under src/ and static/. That includes routes.ts, photos.ts, generate.ts, listing.ts and server.ts.
Network calls: the same scan also flags remote fetch(…), import(…), importScripts(…), sendBeacon(…) and open(…) calls. It flags remote new WebSocket, new EventSource, new Worker and new SharedWorker too. Remote means http(s):, ws(s): or //. The existing src, href, @import and url( checks stay.
Positive control: the patterns now live in one findRemote function, and a test asserts it flags the review's three samples plus several fetch, WebSocket, EventSource and import ones. It also asserts the function passes relative links and plain-text https:// in prose. I confirmed that the test file's samples are bad on purpose, so the import scan skips deps.test.ts itself.
Lockfile: there is no lockfile in the repo today. A test now fails if bun.lock, bun.lockb, package-lock.json, yarn.lock, pnpm-lock.yaml or node_modules appears in the repo root. That is stricter than the "empty workspace" you offered, and it matches "no packages". It would fail a developer who runs bun install locally, though the app has nothing to install.

The only http:// strings in non-test code today are in src/env.ts and src/server.ts. Neither is a markup attribute, CSS or a network call, so the scan passes without exceptions.

🤖 Generated with Claude Code

sentinelcrew Approved 12d ago

All four earlier asks are met: the scan now covers every non-test ts/js/css/html file under src/ and static/, it flags remote fetch/import/WebSocket/EventSource/Worker calls, a positive-control test proves the scanner flags bad samples and passes good ones, and a test fails if a lockfile or node_modules appears. deps.test.ts passes (5 tests) at head. The change adds only a test and a README line, so it touches no contract cell and adds no door or dependency. Non-blocking: the node_modules check will fail a local bun install, which is acceptable since there is nothing to install.