diff --git a/.changeset/type-check-coverage-root-manifest-sources.md b/.changeset/type-check-coverage-root-manifest-sources.md new file mode 100644 index 0000000000..6378049208 --- /dev/null +++ b/.changeset/type-check-coverage-root-manifest-sources.md @@ -0,0 +1,30 @@ +--- +"@objectstack/plugin-auth": patch +"@objectstack/plugin-security": patch +"@objectstack/service-i18n": patch +--- + +fix(tooling): put three more package-root plugin manifests inside a tsc program (#14386) + +`check:type-check-coverage`'s `isUncheckedSourceCandidate` skipped `depth === 0` +(the package root) unconditionally, so a package-root `.ts` file was invisible +to SOURCES_COVERED no matter what it contained — not reported, and not +tracked either. That is exactly why #13284's `driver-memory` / +`plugin-hono-server` manifests went unchecked for as long as they did: +`pnpm --filter typecheck` exited 0 with a file no tsc program read, and +the coverage gate called the package COVERED at the same time. + +This finds three more package-root manifest authoring sites the same hole +hid, all `objectstack.config.ts`: `plugin-auth`, `plugin-security` and +`service-i18n`. The gate now admits `depth === 0` only for a declared, +exact-name allowlist (`ROOT_SOURCE_FILES`, `objectstack.config.ts` its only +member) — not every root-level file, which stays the unresolved "104-file" +scope question this card explicitly declines to settle (comment +5504408509 on #14386) — and each of the three manifests now sits inside a +program its package's own `typecheck` script invokes: a widened `include` on +the existing sibling `noEmit` program for `plugin-auth` +(`tsconfig.examples.json`) and `plugin-security` (`tsconfig.scripts.json`), +and a new sibling `tsconfig.typecheck.json` for `service-i18n` (which had no +sibling to widen), following the `driver-memory` shape #13284 established. + +All three type-check clean at zero recorded debt — no ledger entry is added. diff --git a/packages/plugins/plugin-auth/tsconfig.examples.json b/packages/plugins/plugin-auth/tsconfig.examples.json index 7acd787df6..5f493fc408 100644 --- a/packages/plugins/plugin-auth/tsconfig.examples.json +++ b/packages/plugins/plugin-auth/tsconfig.examples.json @@ -1,4 +1,5 @@ -// The EXAMPLES-layer type-check program for @objectstack/plugin-auth (#10869). +// The EXAMPLES-layer type-check program for @objectstack/plugin-auth (#10869), +// WIDENED (#14386) to also carry this package's manifest authoring site. // // `packages/plugins/plugin-auth/examples/` held `basic-usage.ts` -- the file // `content/docs/permissions/authentication.mdx` publishes as "Basic Auth @@ -17,6 +18,20 @@ // would start shipping the example. This program emits nothing, so it can // neutralise `rootDir` without touching what ships. // +// #14386 ADDED `objectstack.config.ts` (the package-ROOT manifest, `defineStack` +// composing `./src/manifest`) to `include` below, joining `examples/**/*` in +// this same sibling rather than opening a second one: it is a package-root file +// for the identical reason `examples/` is -- `tsconfig.json`'s `include` is +// `src/**/*`, which cannot match a root-level file either -- and this sibling +// already neutralises `rootDir` to `.` for exactly that reason. Before this +// change no tsc program read the manifest: `check:type-check-coverage`'s +// `isUncheckedSourceCandidate` skips `depth === 0` by construction (the +// package-root line), so `pnpm --filter @objectstack/plugin-auth typecheck` +// exiting 0 carried no information about it, of the same shape #13284 found +// and fixed in `packages/drivers/driver-memory` and `packages/plugins/ +// plugin-hono-server`. See `scripts/check-type-check-coverage.mjs`'s +// `ROOT_SOURCE_FILES` for the declared allowlist this repairs against. +// // STRICTNESS IS INHERITED and deliberately not relaxed: `strict`, // `noUnusedLocals`, `noUnusedParameters`, `noImplicitReturns` and the rest come // from the root config through `tsconfig.json`. The directory type-checks clean @@ -27,10 +42,10 @@ "extends": "./tsconfig.json", "compilerOptions": { "noEmit": true, - // `.` rather than the inherited `src`, because the file this program checks - // is the one outside `src`. Safe precisely because nothing is emitted from - // here -- see the header. + // `.` rather than the inherited `src`, because the files this program + // checks (`examples/**/*`, `objectstack.config.ts`) sit outside `src`. + // Safe precisely because nothing is emitted from here -- see the header. "rootDir": "." }, - "include": ["examples/**/*"] + "include": ["examples/**/*", "objectstack.config.ts"] } diff --git a/packages/plugins/plugin-security/tsconfig.scripts.json b/packages/plugins/plugin-security/tsconfig.scripts.json index c8574c1d0a..9e730f898d 100644 --- a/packages/plugins/plugin-security/tsconfig.scripts.json +++ b/packages/plugins/plugin-security/tsconfig.scripts.json @@ -1,4 +1,5 @@ -// The SCRIPTS-layer type-check program for this package (#11351). +// The SCRIPTS-layer type-check program for this package (#11351), WIDENED +// (#14386) to also carry this package's manifest authoring site. // // `scripts/i18n-extract.config.ts` is the input to this package's i18n // extraction: it composes the package's own objects and translation bundles @@ -15,6 +16,20 @@ // `scripts/` would put the directory in front of the emit. This program // emits nothing. // +// #14386 ADDED `objectstack.config.ts` (the package-ROOT manifest, +// `defineStack` composing `./src/manifest`) to `include` below, joining +// `scripts/**/*` in this same sibling rather than opening a second one: it is +// a package-root file for the identical reason `scripts/` is -- `tsconfig +// .json`'s `include` is `src/**/*`, which cannot match a root-level file +// either. Before this change no tsc program read the manifest: +// `check:type-check-coverage`'s `isUncheckedSourceCandidate` skips +// `depth === 0` by construction (the package-root line), so `pnpm --filter +// @objectstack/plugin-security typecheck` exiting 0 carried no information +// about it, of the same shape #13284 found and fixed in `packages/drivers/ +// driver-memory` and `packages/plugins/plugin-hono-server`. See +// `scripts/check-type-check-coverage.mjs`'s `ROOT_SOURCE_FILES` for the +// declared allowlist this repairs against. +// // STRICTNESS IS INHERITED and deliberately not relaxed: `strict`, // `noUnusedLocals`, `noUnusedParameters`, `noImplicitReturns` and the rest // come from the root config through `tsconfig.json`. The directory @@ -26,16 +41,17 @@ // `rootDir` IS NOT OVERRIDDEN HERE, and that is the package-specific half of // this file. `tsconfig.json` already widens it to `../..` (= `packages/`) to // carry the `paths` redirect of a sibling package to source, so the inherited -// value ALREADY contains `scripts/` and this program needs no change. Setting -// it to `.` also measures 0 today, and is still the wrong value: it would -// re-narrow the root below the redirected source, so the first script here -// that reaches that sibling would report TS6059 about the CHECK rather than -// about the code. +// value ALREADY contains `scripts/` -- and, for the same reason, the +// package-root `objectstack.config.ts` added above -- and this program needs +// no change. Setting it to `.` also measures 0 today, and is still the wrong +// value: it would re-narrow the root below the redirected source, so the +// first script here that reaches that sibling would report TS6059 about the +// CHECK rather than about the code. // { "extends": "./tsconfig.json", "compilerOptions": { "noEmit": true }, - "include": ["scripts/**/*"] + "include": ["scripts/**/*", "objectstack.config.ts"] } diff --git a/packages/services/service-i18n/package.json b/packages/services/service-i18n/package.json index 783b9f479a..27d15a656d 100644 --- a/packages/services/service-i18n/package.json +++ b/packages/services/service-i18n/package.json @@ -20,7 +20,7 @@ }, "scripts": { "build": "tsup --config ../../../tsup.config.ts && node ../../../scripts/check-dts-emitted.mjs", - "typecheck": "tsc --noEmit", + "typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.typecheck.json", "test": "vitest run" }, "dependencies": { diff --git a/packages/services/service-i18n/tsconfig.typecheck.json b/packages/services/service-i18n/tsconfig.typecheck.json new file mode 100644 index 0000000000..82487d6972 --- /dev/null +++ b/packages/services/service-i18n/tsconfig.typecheck.json @@ -0,0 +1,48 @@ +// The MANIFEST-layer type-check program for @objectstack/service-i18n (#14386). +// +// `objectstack.config.ts` sits at the PACKAGE ROOT and declares this service's +// plugin manifest inline (a `defineStack({ manifest: { ... } })` call with no +// import from `src/`, unlike `plugin-auth` / `plugin-security`, so the manifest +// body itself is unread by any program too, not merely the composing site). +// Until this file existed no tsc program read a line of it: `tsconfig.json` +// selects `src`, and that entry cannot match a root-level file. The annotation +// was therefore decorative -- `pnpm --filter @objectstack/service-i18n +// typecheck` (a bare `tsc --noEmit`, this package's only program before this +// change) still exited 0 regardless of what the manifest object contained. +// `check:type-check-coverage`'s `isUncheckedSourceCandidate` skips +// `depth === 0` by construction (the package-root line), so the gate reported +// this package COVERED at the same time -- the same green-over-unread-source +// shape #13284 found and fixed in `packages/drivers/driver-memory` and +// `packages/plugins/plugin-hono-server`, and #14386 found two more instances +// of (`plugin-auth`, `plugin-security`) beside this one. See +// `scripts/check-type-check-coverage.mjs`'s `ROOT_SOURCE_FILES` for the +// declared allowlist this repairs against. +// +// A SIBLING rather than a wider `include` on `tsconfig.json`, the distinction +// #5475 drew for `packages/spec` and #10756 for `packages/objectql/scripts`, +// and the shape `packages/drivers/driver-memory/tsconfig.typecheck.json` +// (#13284) used for the identical case: that config EMITS (`rootDir: "src"`, +// `outDir: "dist"`), so widening it to reach the package root would put the +// manifest in front of the emit -- the measured failure mode there is TS6059 +// ("File ... is not under 'rootDir' ... 'rootDir' is expected to contain all +// source files"), and it fires under `--noEmit` too. This program emits +// nothing, so it can neutralise `rootDir` without touching what ships. `tsup` +// builds `src/index.ts` (via the root `tsup.config.ts`) and `files` publishes +// `dist` only, so nothing about the published package moves. +// +// STRICTNESS IS INHERITED and deliberately not relaxed: `strict`, +// `esModuleInterop`, `forceConsistentCasingInFileNames` and the rest come from +// the root config through `tsconfig.json`. The manifest type-checks clean +// under them -- it enters with ZERO recorded debt, and there is no ledger here +// to record any in. +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": true, + // `.` rather than the inherited `src`, because the file this program + // checks is the one outside `src`. Safe precisely because nothing is + // emitted from here -- see the header. + "rootDir": "." + }, + "include": ["objectstack.config.ts"] +} diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 30342a2941..635b34c0dd 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -1165,8 +1165,19 @@ const PHANTOM_PIN_DEBT = {}; // `packages/cli/test/helpers/serve-process.ts` // // This invariant governs the SECOND group only, which is why the observation -// half takes files at `depth > 0`. The line is drawn there on evidence rather -// than convenience, and the evidence cuts both ways, so both halves are here: +// half takes files at `depth > 0` -- EXCEPT for `ROOT_SOURCE_FILES`, the +// declared allowlist #14386 carved out of the first group for the 5 +// `objectstack.config.ts` instances above: those are manifest AUTHORING sites +// (of the same class #13284 put inside a tsc program), not build/test +// tooling, and #14386 found three of the five (`plugin-auth`, +// `plugin-security`, `service-i18n`) still outside every program with no +// ledger entry -- a green gate over unread source. The FOR/AGAINST split +// below is about the other 49 (`vitest.config.ts`, `tsup.config.ts`, ...), +// which #14386 explicitly declined to re-decide (comment 5504408509: "queue +// now" vs "not this card") -- see `ROOT_SOURCE_FILES`'s own docblock for why +// a declared name, not a wider predicate, is the line drawn there. The line +// is drawn there on evidence rather than convenience, and the evidence cuts +// both ways, so both halves are here: // // FOR -- the 54 are one repo-wide convention, not 42 independent decisions. // Every package's `include` is `src/**/*`; a tool config sits at the package @@ -1564,18 +1575,50 @@ function unreadFiles(rels, programs) { return rels.filter((rel) => !programs.some((c) => configCovers(c, rel))); } +/** + * Root-level (`depth === 0`) filenames admitted into SOURCES_COVERED despite + * the package-root line below (#14386). + * + * The census behind that line found 54 root configs against 11 files in a + * source directory, and most of the 54 are build/test tooling + * (`vitest.config.ts`, `tsup.config.ts`) rather than authored source -- + * deciding whether THAT population owes a program or a ledger entry is a + * scope decision about build tooling, not about authored source, and #14386 + * (comment 5504408509) is explicit that a dev repair must not settle it: "if + * whoever implements this believes the allowlist is the wrong shape, they + * stop and file a needs-user-decision card -- they do not widen the predicate + * to all of depth 0". + * + * `objectstack.config.ts` is admitted because it is not tooling -- it is a + * plugin manifest authoring site of exactly the class #13284 put inside a tsc + * program (`packages/drivers/driver-memory`, `packages/plugins/ + * plugin-hono-server`), and #14386 found three more package-root instances of + * it sitting outside every program with no ledger entry: a green gate over + * unread source. Declared as an exact filename set rather than matched by a + * pattern (e.g. any `*.config.ts`) so that growing it to cover a new filename + * stays a decision made here, on its own card, instead of happening as a side + * effect of some unrelated file landing at a package root. + */ +const ROOT_SOURCE_FILES = new Set(['objectstack.config.ts']); + /** * Is this file one SOURCES_COVERED asks about (#10756)? * * Three exclusions, each load-bearing: * - * `depth > 0` the package-root line. A `vitest.config.ts` beside the - * manifest is a tool's entry point, not a directory of source, - * and the census kept the two apart -- 54 root configs against - * 11 files in a source directory. Argued both ways on - * UNCHECKED_SOURCE_DEBT. + * `depth > 0` the package-root line, EXCEPT for `ROOT_SOURCE_FILES` + * above. A `vitest.config.ts` beside the manifest is a + * tool's entry point, not a directory of source, and the + * census kept the two apart -- 54 root configs against 11 + * files in a source directory. Argued both ways on + * UNCHECKED_SOURCE_DEBT for everything else at the root; a + * declared manifest authoring site is not "everything else" + * (#14386). * `.d.ts` a declaration file STATES types rather than being checked for - * them, so "no program reads it" is not the same finding. + * them, so "no program reads it" is not the same finding -- + * still enforced even for a `ROOT_SOURCE_FILES` name, so a + * hypothetical declaration-file entry there could never + * bypass this exclusion. * test files TESTS_COVERED's subject, decided per file over there. Counting * them here too would bill one hidden file to two ledgers. * @@ -1583,7 +1626,7 @@ function unreadFiles(rels, programs) { * @param {number} depth 0 at the package root */ function isUncheckedSourceCandidate(name, depth) { - if (depth === 0) return false; + if (depth === 0 && !ROOT_SOURCE_FILES.has(name)) return false; if (!SOURCE_FILE.test(name) || TEST_FILE.test(name)) return false; return !name.endsWith('.d.ts'); } @@ -1663,7 +1706,15 @@ function testCoverage(dir, scripts) { const uncheckedByDir = new Map(); if (scripts.typecheck !== undefined) { for (const rel of unreadFiles(sourceRels, accountedPrograms(configs, invoked))) { - const top = rel.slice(0, rel.indexOf('/')); + // `rel.indexOf('/')` is -1 for a package-ROOT file (#14386's + // `ROOT_SOURCE_FILES` admits those past `isUncheckedSourceCandidate` + // now) -- `.slice(0, -1)` on that would produce a garbage key (the + // basename minus its last character), not a missing one. `'.'` is the + // sensible root key: fed through `posix.join(dir, top)` below it + // collapses to the package's own directory, which is exactly what an + // unread root-level file is "inside" -- there is no narrower directory + // to name. + const top = rel.includes('/') ? rel.slice(0, rel.indexOf('/')) : '.'; uncheckedByDir.set(top, (uncheckedByDir.get(top) ?? 0) + 1); } } @@ -3733,6 +3784,28 @@ function selfTest() { state: okState, expect: [/packages\/a\/scripts: 2 non-test source file\(s\) here sit outside every tsc program/], }, + { + // #14386: every source-layer case above is a SUBDIRECTORY case + // (`packages/a/scripts`) -- that is exactly why the hole this card + // fixes survived. `testCoverage()`'s aggregation keys a root-level + // file (no `/` in its package-relative path) at `'.'`, which + // `posix.join(dir, '.')` collapses to the package's own directory -- + // so the dir this fixture feeds in (`'packages/a'`, with no + // subdirectory suffix) is what the real walk now actually produces + // for an unread `objectstack.config.ts`, and this pins that it + // renders sensibly rather than as the `rel.slice(0, -1)` garbage key + // the old `indexOf('/') === -1` arithmetic would have produced. + label: 'an unread ROOT-level source file in a COVERED package fails SOURCES_COVERED too, keyed at the package directory itself (#14386)', + packages: [ + pkg('a', { + scripts: { typecheck: 'tsc --noEmit' }, + uncheckedSources: [{ dir: 'packages/a', files: 1 }], + }), + ], + root: okRoot, + state: okState, + expect: [/packages\/a: 1 non-test source file\(s\) here sit outside every tsc program/], + }, { label: 'an UNCHECKED_SOURCE_DEBT entry covers it, but only with a reason', packages: [ @@ -4249,14 +4322,24 @@ function selfTest() { } // SOURCES_COVERED's file-level predicate (#10756). Each exclusion is a way - // this invariant can be silently wrong: lose `depth > 0` and 42 packages' - // tool configs flood the ledger, lose the test check and one hidden file is - // billed to two ledgers, lose `.d.ts` and the gate reports a finding about a - // file that states types rather than being checked for them. + // this invariant can be silently wrong: lose `depth > 0` (unqualified) and + // 42 packages' tool configs flood the ledger, lose the test check and one + // hidden file is billed to two ledgers, lose `.d.ts` and the gate reports a + // finding about a file that states types rather than being checked for + // them. const sourceCandidateCases = [ { label: 'a module in a subdirectory is the subject', name: 'dry-run-hash-compat.ts', depth: 1, expect: true }, { label: 'the same module at the package root is not', name: 'dry-run-hash-compat.ts', depth: 0, expect: false }, { label: 'a package-root tool config is out of scope by the depth rule', name: 'vitest.config.ts', depth: 0, expect: false }, + // #14386: `ROOT_SOURCE_FILES` is the depth-0 exception, and it is an + // EXACT-name allowlist, not "anything that looks like config at the + // root" -- the next two rows pin both sides of that line. + { label: 'a declared root source file IS in scope despite depth 0 (#14386)', name: 'objectstack.config.ts', depth: 0, expect: true }, + { label: 'a root-level BUILD tool config stays out of scope -- the 104-file question is not this predicate\'s to answer', name: 'tsup.config.ts', depth: 0, expect: false }, + // Defense in depth: even a name that WERE declared in `ROOT_SOURCE_FILES` + // must still lose to the `.d.ts` exclusion below -- a states-types file + // is never "unread source" regardless of depth. + { label: 'a declaration file at the root is still excluded, even hypothetically declared', name: 'globals.d.ts', depth: 0, expect: false }, { label: 'the SAME tool config inside a directory IS in scope', name: 'i18n-extract.config.ts', depth: 1, expect: true }, { label: 'a test file belongs to TESTS_COVERED, not here', name: 'engine.test.ts', depth: 1, expect: false }, { label: 'a spec file likewise', name: 'engine.spec.tsx', depth: 2, expect: false }, diff --git a/scripts/check-type-source-resolution.mjs b/scripts/check-type-source-resolution.mjs index 60d8a4dd5a..a4070192b1 100644 --- a/scripts/check-type-source-resolution.mjs +++ b/scripts/check-type-source-resolution.mjs @@ -498,6 +498,32 @@ const KNOWN_DIST_RESOLVED_TYPE_IMPORTS = { '@objectstack/service-cluster', '@objectstack/service-datasource', '@objectstack/spec', '@objectstack/types', ], + // #14386 re-baseline (the onboarding limb above): a NEW entry, reached ONLY + // through `tsconfig.typecheck.json` -- a program that card ADDED (this + // package's `typecheck` was a bare `tsc --noEmit` before it, with no sibling + // to move the set through). The bare `@objectstack/spec` specifier + // `objectstack.config.ts`'s `import { defineStack } from '@objectstack/spec'` + // resolves is not one `src/**/*` reached before: `src/` imports only + // SUBPATHS (`@objectstack/spec/contracts`, `@objectstack/spec/system`), + // never the bare package, so this program is genuinely the first to reach + // it. `paths` is deliberately NOT the tool, on the onboarding limb's own + // measured grounds (PR #12570): this program checks exactly one file whose + // manifest is inline literals, and pulling `@objectstack/spec/src` into it + // via `paths` would put that package's own source (and everything IT + // imports) inside a program whose `rootDir` is `.` (= this package's own + // directory) -- the same TS6059 storm `driver-memory/tsconfig.typecheck.json` + // (#13284) neutralises `rootDir` to avoid for its OWN single file, not for a + // transitively-pulled dependency tree. + // + // Numbers, `--list` before/after on the same checkout (measured at + // `bd8795ea1`, this entry excluded from the "after" run to isolate exactly + // what onboarding the program added): + // + // before 55 of 77 packages, 111 programs, 269 pairs, 22 clean + // after 56 of 77 packages, 112 programs, 270 pairs, 21 clean + // + // so +1 program, +1 pair, +1 package -- this entry and nothing else. + '@objectstack/service-i18n': ['@objectstack/spec'], // #11490 re-baseline: NEW entries — reached only through `tsconfig.scripts.json`. '@objectstack/service-messaging': ['@objectstack/spec'], '@objectstack/service-realtime': ['@objectstack/spec'],