fix(service-knowledge): put the test layer in front of tsc, and repair the four defects it was hiding (#15049) (#15147)

packages/services/service-knowledge had no typecheck script at all, so no
tsc program anywhere read this package. Undivided (existing tsconfig.json)
measured 10 raw errors, matching the DEBT entry this PR deletes; the correct
split (new tsconfig.test.json) measured 4 -- not the ledger's own 3-code-tier
guess, because fixing the 3 TS2835 config-tier imports re-enabled an
excess-property check the resulting `any` cascade had been suppressing,
uncovering a 4th real error (a stale `roles` field, renamed to `positions`).
All 4 code-tier defects and the 3 TS2835 are repaired in the test files; both
readings are now 0.

DEBT entry deleted (not lowered). check:type-source-resolution gains an
onboarding-limb registry entry for the 3 workspace deps now reached only
through tsconfig.test.json (paths was measured and rejected: 0 -> 487
TS6059, all billed to other packages' source).


Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-sales

.changeset/service-knowledge-test-tsc-program.md

@@ -0,0 +1,86 @@
+---
+"@objectstack/service-knowledge": patch
+---
+
+fix(service-knowledge): put the test layer in front of tsc, and repair the four defects it was hiding (#15049)
+
+`packages/services/service-knowledge` had **no `typecheck` script at all** —
+its scripts were `build` and `test` — so no tsc program anywhere read this
+package. Turbo/CI typecheck lanes skipped it silently, because a zero-matching
+filter run exits 0. `tsup` transpiles with esbuild and `vitest` runs through
+esbuild type-**stripping**; neither type-checks. The package's own
+`tsconfig.json` does include the tests and always did, so the program that
+would have read them already existed and was simply never invoked — the same
+shape `@objectstack/service-cluster` (#14181) reached the ledger by.
+
+**Measured before any repair**, dependency closure built first: `tsc --noEmit`
+against the existing `tsconfig.json` (undivided, BUILD/NodeNext semantics)
+read **10** raw errors, matching the `DEBT` entry this PR deletes exactly. The
+new sibling `tsconfig.test.json` (module semantics only — `esnext` / `bundler`
+/ `lib: ES2022`, matching how vitest actually executes these files; strictness
+untouched) read **4** under the correct split — not the ledger's own 3-code-tier
+guess. Fixing the 3 TS2835 (three relative test imports missing `.js`, required
+by `moduleResolution: NodeNext`) removed the noise cascade (4 TS7006, every
+`(h) => h.documentId)` callback over a `KnowledgeService` search result that
+had degraded to `any`) and, in doing so, re-enabled a TypeScript excess-property
+check the cascade had been suppressing — uncovering a 4th real error the
+undivided reading had masked entirely.
+
+**The four code-tier defects, all in the test file, all in the test file's own
+typing — never in `src/`:**
+
+1. `roles: ['member']` in one `ExecutionContext` object literal (TS2353 once
+   the excess-property check could see it) — a field the spec renamed to
+   `positions` (`execution-context.zod.ts`: *"Position names held by the
+   user … Formerly `roles`"*), that no check had ever read against the
+   renamed type. Every other `executionContext` literal in this file already
+   used `positions`; this one was simply never checked before. Fixed by
+   renaming it — `ExecutionContext` itself is untouched and correct.
+2. `buildSetup`'s `vi.fn()` stub for `IDataEngine.find` typed its second
+   parameter as `{ context: { isSystem?: boolean } }`, omitting the `where`
+   field the real call site (`knowledge-service.ts`'s RLS re-check) actually
+   passes. `expect(opts.where).toEqual(...)` then read a property TypeScript
+   correctly said did not exist (TS2339). Fixed by widening the mock's
+   parameter type to match the call it stubs (`where` and `fields` added),
+   not by loosening the assertion.
+3 & 4. Two more `vi.fn()` mocks (`upsertSpy`/`deleteSpy`/`searchSpy` in
+   `makeAdapter`, and a `find` mock in the reindex test) had **no** parameter
+   type at all, so TypeScript inferred a zero-argument implementation and
+   `.mock.calls[N]` was typed as an array of **empty tuples**. Indexing past
+   that boundary (`.mock.calls[0][1]`) is a genuine tuple-length error
+   (TS2493), and casting the resulting `undefined` onward compounded into
+   TS2352. Fixed the reindex-test `find` mock by typing its parameters to
+   match the real `reindexSource` call site (`where`, `limit`, `context`);
+   the `makeAdapter` stubs were reached only through the 3 TS2835 (below) and
+   needed no change of their own once those were fixed.
+
+**The three TS2835 are repaired directly** (`.js` added to three relative
+specifiers in the test files), not routed around by excluding tests from
+`tsconfig.json` — which stays exactly as it is, per the family's own rule
+(AGENTS.md: never add such an exclusion). Because this package's build config
+already includes the tests, its `typecheck` script's own `tsc --noEmit tsconfig.json`
+step reads these same files under NodeNext regardless of the new sibling
+config, so they needed fixing either way — unlike `service-cluster`, whose test
+files already carried the extension and needed no import repair.
+
+Wired by the #14062 / #5286 route: `tsconfig.test.json` named by a new
+`typecheck` script through the shared `check:test-typecheck` gate. No
+`test-typecheck-debt.json` is added — its **absence is the zero**: the gate
+reads a missing ledger as no entries, under which any error in any file here
+is immediately red. After the repair, **both** readings (`tsconfig.json` and
+`tsconfig.test.json`) are 0.
+
+The package's `DEBT` entry in `scripts/check-type-check-coverage.mjs`
+(`errors: 10`) is **deleted**, not lowered — the graduation the ratchet's own
+invariant requires. `scripts/check-type-source-resolution.mjs` gains a
+registry entry for the three workspace deps (`core`, `objectql`, `spec`) now
+reached only through the new `tsconfig.test.json` program (the #11490
+onboarding-limb re-baseline, same route `service-cluster` took): `paths` was
+measured and rejected — redirecting those three deps to source takes this
+package's test layer from 0 errors to 487, all TS6059, all in another
+package's source.
+
+No runtime code changes: `src/**` (excluding tests) is byte-identical, so no
+shipped behaviour moves. The `patch` level reflects the published
+`package.json` gaining `typecheck` / `check:test-typecheck` scripts and a
+`tsx` devDependency.

packages/services/service-knowledge/package.json

@@ -20,7 +20,9 @@
   },
   "scripts": {
     "build": "tsup --config ../../../tsup.config.ts && node ../../../scripts/check-dts-emitted.mjs",
-    "test": "vitest run"
+    "test": "vitest run",
+    "typecheck": "tsc --noEmit && pnpm check:test-typecheck",
+    "check:test-typecheck": "tsx ../../../scripts/check-test-typecheck.mts --self-test && tsx ../../../scripts/check-test-typecheck.mts --package packages/services/service-knowledge --project tsconfig.test.json"
   },
   "dependencies": {
     "@objectstack/core": "workspace:*",
@@ -29,6 +31,7 @@
   "devDependencies": {
     "@objectstack/objectql": "workspace:*",
     "@types/node": "^26.2.0",
+    "tsx": "^4.23.12",
     "typescript": "^6.0.3",
     "vitest": "^4.1.10"
   },

packages/services/service-knowledge/src/__tests__/event-sync-data-events.test.ts

@@ -12,8 +12,8 @@
 
 import { describe, it, expect, vi } from 'vitest';
 import type { RealtimeEventHandler, RealtimeEventPayload } from '@objectstack/spec/contracts';
-import { KnowledgeServicePlugin } from '../knowledge-service-plugin';
-import type { KnowledgeService } from '../knowledge-service';
+import { KnowledgeServicePlugin } from '../knowledge-service-plugin.js';
+import type { KnowledgeService } from '../knowledge-service.js';
 
 function makeCtx() {
   let readyHook: (() => Promise<void>) | undefined;

packages/services/service-knowledge/src/__tests__/knowledge-service.test.ts

@@ -1,7 +1,7 @@
 // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
 
 import { describe, it, expect, vi } from 'vitest';
-import { KnowledgeService, documentIdFor, recordToDocument } from '../knowledge-service';
+import { KnowledgeService, documentIdFor, recordToDocument } from '../knowledge-service.js';
 import type {
   IDataEngine,
   IKnowledgeAdapter,
@@ -87,7 +87,10 @@ describe('KnowledgeService — adapter & source registry', () => {
 describe('KnowledgeService — permission-aware search', () => {
   function buildSetup(hits: KnowledgeHit[]) {
     const adapter = makeAdapter('memory', hits);
-    const findSpy = vi.fn(async (_obj: string, opts: { context: { isSystem?: boolean } }) => {
+    const findSpy = vi.fn(async (
+      _obj: string,
+      opts: { where?: Record<string, unknown>; fields?: string[]; context: { isSystem?: boolean } },
+    ) => {
       if (opts.context?.isSystem) return [{ id: 'rec_1' }, { id: 'rec_2' }];
       return [{ id: 'rec_1' }];
     });
@@ -140,7 +143,7 @@ describe('KnowledgeService — permission-aware search', () => {
     ];
     const { svc, findSpy } = buildSetup(hits);
     const out = await svc.search('q', {
-      executionContext: { userId: 'u1', roles: ['member'], permissions: [], isSystem: false },
+      executionContext: { userId: 'u1', positions: ['member'], permissions: [], isSystem: false },
     });
     expect(out.map((h) => h.documentId)).toEqual(['d1']);
     expect(findSpy).toHaveBeenCalledOnce();
@@ -256,7 +259,10 @@ describe('KnowledgeService — event sync', () => {
 
 describe('KnowledgeService — reindex', () => {
   it('object source: walks IDataEngine with isSystem context and pushes docs', async () => {
-    const find = vi.fn(async () => [
+    const find = vi.fn(async (
+      _obj: string,
+      _opts: { where?: unknown; limit?: number; context: { isSystem?: boolean } },
+    ) => [
       { id: 'r1', title: 'T1', notes: 'N1', status: 'open' },
       { id: 'r2', title: 'T2', notes: 'N2', status: 'done' },
     ]);
@@ -268,7 +274,7 @@ describe('KnowledgeService — reindex', () => {
     expect(res.ok).toBe(true);
     expect(res.indexed).toBe(2);
     expect(res.discovered).toBe(2);
-    expect((find.mock.calls[0][1] as { context: { isSystem?: boolean } }).context.isSystem).toBe(true);
+    expect(find.mock.calls[0][1].context.isSystem).toBe(true);
     expect(adapter.upsertSpy).toHaveBeenCalledOnce();
   });
 

packages/services/service-knowledge/tsconfig.test.json

@@ -0,0 +1,98 @@
+// The TEST-layer type-check program (#15049 — the `packages/services/**`
+// instance of the class #14062 settled for `packages/plugins/**`, itself
+// adopting the mechanism #5286 set for `packages/spec`, #5449 generalised,
+// #12542 carried to `packages/rest`, #13176 to `packages/plugins/
+// plugin-security`, and #14181 / PR #15032 to `packages/services/
+// service-cluster` — the worked example this file is copied from).
+// `tsconfig.json` beside this one stays exactly as it is: it is the BUILD
+// config. This sibling puts the test layer in front of tsc under the module
+// semantics vitest really executes it with, and `package.json`'s `typecheck`
+// script NAMES it (via `check:test-typecheck --project`), because a config no
+// script invokes is exactly the phantom this whole change is about.
+//
+// ⚠️ WHY `service-knowledge`, LIKE `service-cluster` AND UNLIKE `plugin-auth` /
+// `plugin-sharing` / `core`: this package's `tsconfig.json` does NOT exclude
+// tests (`include: ["src"]`, no `**/*.test.ts` exclusion) and never did, so the
+// program that would have read them already existed and was simply never
+// invoked -- no `typecheck` script named it. `tsup` type-strips, `vitest`
+// type-strips; neither ran tsc.
+//
+// What differs from the build config, and what deliberately does NOT:
+//   - MODULE SEMANTICS ONLY, plus `lib`. The tests are written and executed as
+//     ESM by vitest (esbuild/vite). Matching that is FIDELITY, not laxity: it is
+//     the same subtraction `packages/spec`, `packages/rest`, the
+//     `packages/plugins/**` family and `service-cluster` each made.
+//   - ⛔ STRICTNESS IS UNTOUCHED. `strict`, `noUnusedLocals`,
+//     `noUnusedParameters`, `noImplicitReturns`, `noFallthroughCasesInSwitch`,
+//     `rootDir`, `paths` and `types` are all INHERITED from `tsconfig.json`
+//     (and through it the root config), and none of them is re-declared here.
+//     ⚠️ A child that declared its own `paths` would REPLACE the parent map
+//     rather than merge into it, silently sending a source-resolved specifier
+//     back to `dist/` — a BUILD ARTIFACT — so this file declares none.
+//     Nothing here may loosen a type rule; if a test does not compile, that is
+//     the finding.
+//   - `lib: ["ES2022"]`, for the same reason `packages/rest` states: the root
+//     config's `lib` is ES2020 and vitest runs on a Node that has es2022
+//     builtins, so the gap is reported as TS2550 about the CHECK. No `DOM`:
+//     nothing in this layer touches a browser global.
+//
+// MEASURED, workspace closure built first (`tsc --noEmit --pretty false
+// --listFiles -p <config>`, and the same command without `--listFiles`), on a
+// checkout at merge-base 2cc4610304, BEFORE any repair:
+//
+//   files in this program (test config)     409
+//   files in tsconfig.json (build config)   441
+//   own `src/**/*.test.ts` in the program     4
+//   errors under BUILD semantics (tsconfig.json, undivided)   10
+//   errors under THIS config (the split)                       4
+//
+// The 10 undivided matched the DEBT entry this PR deletes exactly: 3 TS2835
+// (config-tier -- three relative test imports missing `.js`, required by
+// `moduleResolution: NodeNext`) + 4 TS7006 (noise -- `KnowledgeService`
+// resolving to `any` through the unresolved imports cascades into every
+// `(h) => h.documentId)` callback over its return value) + 3 code-tier the
+// ledger's note itemised (TS2339/TS2352/TS2493).
+//
+// The split did NOT confirm the ledger's 3-code-tier guess -- it found 4, the
+// same "a tier split read off an unrepaired config is a guess about what is
+// UNDER it" lesson this file's sibling ledger note states for `metadata` and
+// `service-storage`: stripping the config-tier noise re-enabled an EXCESS
+// PROPERTY CHECK that the `any`-typed parameter had been suppressing, and it
+// caught a real one -- `roles: ['member']` in one `ExecutionContext` literal,
+// stale since the field was renamed to `positions` (`execution-context.zod.ts`
+// docs it: "Formerly `roles`"). The other 3 were exactly the ledger's guess:
+// two `vi.fn()` mocks stubbing `IDataEngine.find` typed with fewer parameters
+// than the real call site they stand in for, so `.mock.calls[N]` indexed past
+// a TS-inferred EMPTY tuple (TS2493, plus the `as` cast off it reading TS2352)
+// and a third mock's param type omitted the `where` field the real call
+// passes (TS2339). All 4 are fixed in the test file, matching each mock's type
+// to the call site it stubs -- never widening the mock, never touching
+// `ExecutionContext` (which is correct; the test's stale field name was not).
+//
+// The three TS2835 are ALSO repaired directly (`.js` added to the three
+// relative specifiers), because `tsconfig.json` genuinely DOES include the
+// tests -- same as `service-cluster` -- so the package's `typecheck` script's
+// bare `tsc --noEmit` step reads these same files under NodeNext and needs
+// them resolvable regardless of this sibling config; unlike `service-cluster`,
+// this package's test files had NOT already been written with the extension.
+// After both repairs, BOTH readings are 0 -- see the PR body / changeset for
+// the fix-by-fix account.
+//
+// There is NO `test-typecheck-debt.json` beside this config, and its ABSENCE is
+// the zero: `check:test-typecheck` reads a missing ledger as `{ entries: {} }`,
+// under which ANY error in ANY file here is red immediately, with no entry to be
+// added to. If this package ever acquires residue that cannot be fixed in the
+// PR that causes it, THAT is when a ledger and a `gen:test-typecheck-debt`
+// script are owed — and adding one is maintainer-only (#5286), exactly as the
+// gate says when it refuses.
+{
+  "extends": "./tsconfig.json",
+  "compilerOptions": {
+    "noEmit": true,
+    "module": "esnext",
+    "moduleResolution": "bundler",
+    "lib": ["ES2022"]
+  },
+  "include": ["src/**/*"],
+  "exclude": ["node_modules", "dist"]
+}

pnpm-lock.yaml

@@ -679,6 +679,27 @@ const ROOT_PROGRAM_COUPLED_SCRIPT = 'scripts/check-test-typecheck.mts';
 // ran even though that config DOES include the tests. Repaired by the #5286
 // route -- a `tsconfig.test.json` over the test layer, named by a new `typecheck`
 // script -- so the entry is deleted rather than lowered.
+//
+// `@objectstack/service-knowledge` GRADUATED from this ledger too (#15049,
+// PR #15032's sibling for `packages/services/**`; entry: 10 raw, repaired to
+// 0 under BOTH the build config and the new `tsconfig.test.json` split). This
+// one is worth a line because the split did NOT confirm this entry's own
+// 3-code-tier guess -- it found 4, the same "a tier split read off an
+// unrepaired config is a guess about what is UNDER it" lesson the paragraph
+// above states for `metadata` and `service-storage`. Fixing the 3 TS2835 (the
+// config-tier third, and the noise: the unresolved imports made
+// `KnowledgeService` `any`, which suppressed the TypeScript excess-property
+// check on an `ExecutionContext` literal) uncovered a 4th real error the
+// undivided reading had masked: `roles: ['member']`, a field the spec renamed
+// to `positions` (`execution-context.zod.ts`: "Formerly `roles`") that no
+// check had ever read with the renamed type. The other 3 code-tier errors
+// were exactly this entry's guess: two `vi.fn()` mocks stubbing
+// `IDataEngine.find` typed with fewer parameters than the call site they
+// stand in for, so `.mock.calls[N]` indexed past a TS-inferred EMPTY tuple
+// (TS2493/TS2352), and a third mock's parameter type omitted the `where`
+// field the real call passes (TS2339). All 4 are fixed in the test file,
+// matching each mock's type to the call site it stubs; `ExecutionContext`
+// itself was not touched (it was correct -- the test's field name was stale).
 const DEBT = {
   '@objectstack/cloud-connection': {
     errors: 13,
@@ -706,12 +727,6 @@ const DEBT = {
       + 'by acquiring a second file, then 5 -> 3 by graduating the first -- so re-read what the pile is '
       + 'made of before sizing it, never just the number.',
   },
-  '@objectstack/service-knowledge': {
-    errors: 10,
-    note: 'code-tier 3 (TS2339/TS2352/TS2493); config-tier 3 (TS2835); noise 4 (TS7006). Re-measured 10 at '
-      + '5ab08428, up from 8; code-tier is unchanged at 3, so the +2 is config-tier/noise. 8 of the 10 are '
-      + 'in __tests__/knowledge-service.test.ts.',
-  },
   '@objectstack/service-storage': {
     errors: 51,
     note: 'code-tier 8 (TS2339 x4, TS2347 x4); config-tier 26 (TS2835 x23, TS2550 x3); noise 17 '