From b3ec947f80e105da022ff77ad55746bbc9b6f3d7 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:21:25 -0700 Subject: [PATCH 1/2] Add tests for node: denylist and undeclared ./client exports Covers the two gaps CL-7124 found: a node:* import should be denied like any other server-only specifier, and a package.json declaring a ./client export with no ENTRIES ruling should fail loudly instead of being silently skipped. --- .../checks/test/browser-safe-subpaths.test.ts | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/scripts/checks/test/browser-safe-subpaths.test.ts b/scripts/checks/test/browser-safe-subpaths.test.ts index 471b206de..d6dd0231a 100644 --- a/scripts/checks/test/browser-safe-subpaths.test.ts +++ b/scripts/checks/test/browser-safe-subpaths.test.ts @@ -266,6 +266,66 @@ test("a comment mentioning import does not swallow a later type-only import", () ]); }); +test("a node: import is denylisted regardless of subpath", () => { + const packages: PackageManifest[] = [ + { + name: "@corbits/pkg-a", + exports: { "./client": "packages/pkg-a/src/client.ts" }, + }, + ]; + const files = new Map([ + ["packages/pkg-a/src/client.ts", `import { join } from "node:path";`], + ]); + + const report = auditBrowserSafeSubpaths( + [{ package: "@corbits/pkg-a", subpath: "./client" }], + packages, + files, + ); + + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("node:path"); +}); + +test("a declared ./client export with no ENTRIES ruling is a violation naming the package", () => { + const packages: PackageManifest[] = [ + { + name: "@corbits/pkg-a", + exports: { "./client": "packages/pkg-a/src/client.ts" }, + }, + ]; + const files = new Map([ + ["packages/pkg-a/src/client.ts", `export const x = 1;`], + ]); + + const report = auditBrowserSafeSubpaths([], packages, files); + + expect(report.violations).toHaveLength(1); + expect(report.violations[0]).toContain("@corbits/pkg-a"); + expect(report.violations[0]).toContain("./client"); + expect(report.violations[0]).toContain("ENTRIES"); +}); + +test("a declared ./client export with a matching ENTRIES ruling is not a violation", () => { + const packages: PackageManifest[] = [ + { + name: "@corbits/pkg-a", + exports: { "./client": "packages/pkg-a/src/client.ts" }, + }, + ]; + const files = new Map([ + ["packages/pkg-a/src/client.ts", `export const x = 1;`], + ]); + + const report = auditBrowserSafeSubpaths( + [{ package: "@corbits/pkg-a", subpath: "./client" }], + packages, + files, + ); + + expect(report.violations).toEqual([]); +}); + test("a block comment cannot hide a real value import", () => { const parsed = parseImportSpecifiers( ['/* import x from "commented-out"; */', 'import y from "real";'].join( From 74743e5d0528ae207f178e3cc73e85554b4dc665 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 28 Aug 2026 05:21:35 -0700 Subject: [PATCH 2/2] browser-safe-subpaths: walk bench/preferences, deny node:, catch forgotten clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit grep -l '"./client"' packages/*/package.json turned up seven packages declaring a browser-safe client, but ENTRIES only walked five — @corbits/bench/client and @corbits/preferences/client were never audited. The denylist also had no node:* pattern, so a Node-only import would pass silently. Add the two missing entries, deny node:*, and make the check itself fail when a package.json declares the conventional ./client export with no matching ENTRIES ruling, so a future client can't be forgotten the same way. Fixes CL-7124. --- scripts/checks/browser-safe-subpaths.ts | 41 ++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/scripts/checks/browser-safe-subpaths.ts b/scripts/checks/browser-safe-subpaths.ts index 95c4e557d..d4ae2e5df 100644 --- a/scripts/checks/browser-safe-subpaths.ts +++ b/scripts/checks/browser-safe-subpaths.ts @@ -17,7 +17,11 @@ // always is. // // New browser-safe subpaths need an explicit ruling in ENTRIES below — -// this check only walks what it's told to. +// this check only walks what it's told to. A package.json declaring the +// conventional `./client` subpath with no matching ENTRIES ruling is +// itself a violation, so a new client can't be silently forgotten; a +// non-conventional browser-safe subpath (like api-query's `./envelope`) +// still needs to be added to ENTRIES by hand. import { Glob } from "bun"; import path from "node:path"; import { @@ -50,6 +54,8 @@ export const ENTRIES: readonly BrowserSafeEntry[] = [ { package: "@corbits/inbox", subpath: "./client" }, { package: "@corbits/insights", subpath: "./client" }, { package: "@corbits/agent-directory", subpath: "./client" }, + { package: "@corbits/bench", subpath: "./client" }, + { package: "@corbits/preferences", subpath: "./client" }, { package: "@corbits/api-query", subpath: "." }, // The envelope alone (UnauthenticatedError, ApiQueryError, toAPIQuery) has // no React/JSX dependency, so packages without `jsx` configured (e.g. @@ -77,6 +83,7 @@ const DENYLIST_PATTERNS: readonly RegExp[] = [ /^hono(\/|$)/, /^postgres(\/|$)/, /^drizzle-orm(\/|$)/, + /^node:/, ]; function isDenylisted(specifier: string): boolean { @@ -164,6 +171,36 @@ function splitPackageSpecifier(specifier: string): { return { packageName, rest }; } +/** + * `./client` is this repo's naming convention for a browser-safe subpath + * (routines, inbox, insights, agent-directory, presence, bench, + * preferences all use it) — a package that declares one is making the same + * "nothing server-only reaches here" promise ENTRIES exists to check, so a + * declared `./client` with no ENTRIES ruling is itself a violation, not a + * silent skip. + */ +const CONVENTIONAL_SUBPATH = "./client"; + +function findUnruledClientExports( + entries: readonly BrowserSafeEntry[], + packages: readonly PackageManifest[], +): string[] { + const ruled = new Set( + entries.map((entry) => `${entry.package}${entry.subpath}`), + ); + const violations: string[] = []; + for (const pkg of packages) { + if (pkg.exports[CONVENTIONAL_SUBPATH] === undefined) continue; + if (ruled.has(`${pkg.name}${CONVENTIONAL_SUBPATH}`)) continue; + violations.push( + `${pkg.name} declares a "${CONVENTIONAL_SUBPATH}" export with no ` + + `ENTRIES ruling in scripts/checks/browser-safe-subpaths.ts — add ` + + `{ package: "${pkg.name}", subpath: "${CONVENTIONAL_SUBPATH}" } to ENTRIES.`, + ); + } + return violations; +} + /** * Walks the transitive import graph of every declared entry and reports * a violation for each denylisted import reached, and for any import @@ -179,6 +216,8 @@ export function auditBrowserSafeSubpaths( ): CheckReport { const report = emptyReport(); + report.violations.push(...findUnruledClientExports(entries, packages)); + for (const entry of entries) { const label = entryLabel(entry); const manifest = packages.find((pkg) => pkg.name === entry.package);