Uh oh!
There was an error while loading. Please reload this page.
fix(service-knowledge): put the test layer in front of tsc, and repair the four defects it was hiding - #15147
Merged
Conversation
…r the four defects it was hiding (#15049) 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). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 0 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
This was referenced Sep 4, 2026
os-sales
marked this pull request as ready for review
September 4, 2026 04:14
os-sales
enabled auto-merge
September 4, 2026 04:14
This was referenced Sep 4, 2026
Uh oh!
There was an error while loading. Please reload this page.
os-warren pushed a commit
that referenced
this pull request
Sep 4, 2026
Resolves the third and last conflict of the #14062 family. Both siblings have landed on main (#15147 service-knowledge, #15152 service-automation) and all three touch the same two gate scripts. Resolution is "keep every entry" -- the three cards do not disagree: * scripts/check-type-check-coverage.mjs: all three graduation paragraphs kept above `DEBT` (service-storage first, so the service-knowledge paragraph's "the paragraph above ... for `metadata` and `service-storage`" back-reference still resolves), and all three DEBT entries deleted. The ledger summary line is computed at runtime from DEBT/TEST_DEBT, so no hand-edited total exists there to reconcile. * scripts/check-type-source-resolution.mjs: auto-merged; all three new registry entries present, service-cluster / service-i18n untouched. * pnpm-lock.yaml: not hand-merged. `pnpm install` over the merged manifest set leaves the auto-merged file byte-identical (hash 000bb7d...) and `pnpm install --frozen-lockfile` exits 0. The diff against main is unchanged from the reviewed one: same 20 files, same +307/-133. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#15049
Clause-②: no
The gap, and which exemplar this is copied from
packages/services/service-knowledgehad notypecheckscript at all —its scripts were
buildandtest— so no tsc program anywhere read thispackage. Turbo/CI typecheck lanes skipped it silently (a zero-matching filter
run exits 0);
tsuptranspiles with esbuild andvitestruns through esbuildtype-stripping; neither type-checks. Errors were carried instead as a
DEBT entry of 10 in
scripts/check-type-check-coverage.mjs.Copied from
@objectstack/service-cluster(#14181 / PR #15032), theworked example this card names — not from
plugin-auth/plugin-sharing/core, whosetsconfig.jsonexcludes tests, because AGENTS.md forbidsadding such an exclusion.
service-knowledge'stsconfig.jsondoes notexclude tests (
include: ["src"], no**/*.test.tsexclusion) and neverdid — the same shape as
service-clusterandplugin-webhooks— so theprogram that would have read them already existed and was simply never
invoked.
tsconfig.jsonis therefore untouched; the siblingtsconfig.test.jsonis the family's uniform instrument over the same files.Measured error count, before and after
Dependency closure built first (
pnpm --filter '@objectstack/service-knowledge^...' build),measured at merge-base
2cc4610304:tsc --noEmit -p tsconfig.json, undivided)tsc --noEmit -p tsconfig.test.json, the split)409 files in the test program (441 in the build program — the gap is dist
declaration-variant granularity for the same three workspace deps under
NodeNext vs bundler resolution,
.d.ts/.d.ctschunk files vs.d.mts, nota difference in which of this package's own
src/test files are read),covering all 4 of the package's
src/__tests__/*.test.ts.The undivided 10 matched the DEBT entry this PR deletes exactly — 3 TS2835
(config-tier: three relative test imports missing
.js, required bymoduleResolution: NodeNext) + 4 TS7006 (noise: the unresolved imports makeKnowledgeServiceresolve toany, cascading into everyout.map((h) => h.documentId)callback over its search results) + 3 code-tierthe ledger's note itemised (TS2339/TS2352/TS2493).
The split did NOT confirm the ledger's 3-code-tier guess — it read 4. This
is the same "a tier split read off an unrepaired config is a guess about what
is UNDER it" lesson
scripts/check-type-check-coverage.mjsalready states for@objectstack/metadataand@objectstack/service-storage: fixing the 3TS2835 removed the
anycascade, and doing so re-enabled a TypeScriptexcess-property check the cascade had been silently suppressing —
uncovering a 4th real error the undivided reading had masked completely.
The four code-tier defects — all in the test file's own typing,
src/untouchedexecutionContext: { userId: 'u1', roles: ['member'], permissions: [], isSystem: false }—
roleswas renamed topositions(execution-context.zod.ts:"Position names held by the user … Formerly
roles"). Every otherexecutionContextliteral in this file already usedpositions; this onesimply had never been read by a type checker before, so nothing ever
caught it. TS2353 (excess property) once the excess-property check could
see it. Fixed by renaming the field —
ExecutionContextitself is correctand untouched.
buildSetup'svi.fn()stand-in forIDataEngine.findtyped its second parameter as{ context: { isSystem?: boolean } }, omitting thewherefield the realcall site (
knowledge-service.ts's RLS re-check) actually passes.expect(opts.where).toEqual(...)then read a property TypeScript correctlysaid did not exist (TS2339). Fixed by widening the mock's parameter type to
match the call it stubs —
whereandfieldsadded — never by looseningthe assertion.
find = vi.fn(async () => [...])in the reindex test had zero declared parameters, so TypeScript inferred a
0-arity implementation and typed
.mock.callsas an array of emptytuples.
find.mock.calls[0][1]then indexed past a fixed-length-0 tuple— a genuine TS2493 — and the
as { context: {...} }cast off the resultingundefinedcompounded into TS2352. Fixed by typing the mock's parametersto match the real
reindexSourcecall site (where,limit,context);the assertion no longer needs its
ascast at all.Why these and not a widened
ExecutionContextor a looserIDataEnginestub: the spec contract is correct in both cases (the field really was
renamed;
findreally is called withwhere/fields/limit) — the defectwas in the test's typing of its own doubles, so the test is where it is
fixed. This is the same contract-first call PR #15032 made for
service-cluster's TS2322 (fix the test, not the contract it stubs).The three TS2835 — repaired directly, not routed around
.jsadded to the three relative specifiers(
../knowledge-service-plugin→../knowledge-service-plugin.js,../knowledge-service→../knowledge-service.js×2). Unlikeservice-cluster, whose test files already carried the extension (undividedand split agreed at 1 both times), this package's did not — and because
tsconfig.jsongenuinely includes the tests and stays untouched (excludingthem is the one thing AGENTS.md forbids), the package's own
typecheckscript's bare
tsc --noEmit tsconfig.jsonstep reads these same files underNodeNext regardless of the new sibling config. They needed fixing either way;
the fix is exactly what TS2835's own message names (
Did you mean '../knowledge-service.js'?), not a config change.tsconfig.test.jsonModule semantics only —
module: esnext,moduleResolution: bundler,lib: ["ES2022"]— matching how vitest actually executes these files.Strictness is inherited and untouched, and it declares no
paths(a child'spathswould REPLACE the parent map rather than merge).package.jsongainstypecheck/check:test-typecheck, both scripts checked against therequired spellings
check-type-check-coverage.mjsandcheck-test-typecheck.mts --self-test/--projectname.No
test-typecheck-debt.jsonis created — its absence is the zero: thegate reads a missing ledger as no entries, so any error in any file here
would be red immediately with nothing to add an entry to.
Ledger consequences
DEBTentry deleted, not lowered.check-type-check-coverage.mjscarried
'@objectstack/service-knowledge': { errors: 10, note: '...' }.That gate's own invariant — covered packages must not also sit in the
ledger — makes this a structural requirement, not a courtesy. Deleted, with
the graduation and its real composition recorded in the file's prose (the
same convention
service-cluster's graduation used).test-typecheck-debt.json— see above.check:type-source-resolution— an onboarding re-baseline, please review it as oneWent red on my diff, same shape PR #15032 hit for
service-cluster:onboarding
tsconfig.test.jsonmoves the package's tsc PROGRAM SET, and thegate's doc-block opens an explicit onboarding limb for exactly this.
Term 1 — provenance, all three deps annotated
via tsconfig.test.jsonbythe gate's own failure text; the BUILD program (
tsconfig.json) carries zerodist-resolved workspace type imports both before and after this PR (confirmed
by direct measurement, not inferred), so the exposure is only reachable
through the onboarded program.
Term 2 — numbers,
--listbefore/after on the same checkout (before atthe
service-clustermerge,2cc4610304; after with this card applied):+1 package, +1 program, +3 pairs — this entry (
@objectstack/core,@objectstack/objectql,@objectstack/spec) and nothing else.Term 3 — why the entry and not
paths, measured both ways. Redirectingthe three deps to source takes this package's test layer from 0 errors to
487, all
TS6059(not under rootDir) and every one in anotherpackage's source (
packages/spec/src/**,packages/core/src/**,packages/objectql/src/**) — billed to a package that cannot pay them down.Same shape as
service-cluster's own 0 → 435 and #12570's +5 forrest, at alarger scale here because three workspace deps are pulled rather than two.
Gates
Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandson the rebased head
2969cef816, plus the roster-silent families this diff'spaths sit inside (
check:error-code-casing,check:filter-alias-parity,check:i18n-stale-fill,check:published-readme-exports,check:published-list-mirrors,check:swallow-census-controls,check:authz-resolver,check:console-injection) — silent verdicts readagainst a roster, not against these paths, so run rather than trusted.
Exit codes captured redirect-then-read, never across a pipe.
73 gates run: 70 exit 0, 2 exit 3 (NOT MEASURED), 1 exit 1 that is also NOT
MEASURED in substance (below) , 0 red.
NOT MEASURED, quoting each gate's own verdict:
check:dual-build-cjs-loads— exit 3: "PREREQUISITE NOT MET — thisgate reads built output, and some package has no dist/." Zero mentions of
service-knowledgein its output.check:type-check-debt— exit 3: "check-type-check-coverage:PREREQUISITE NOT MET … --re-measure cannot run: 18 workspace
dependenc(ies) of the ledgered packages have no built type entry point on
disk."
service-knowledgegraduated out of this ledger in this PR, so itis not among the 18.
check:published-readme-exports— exit 1 (a genuine finding, not thisgate's own NOT-MEASURED code), but its 84 findings are every OTHER
unbuilt workspace package's README pointing at a missing
dist/. Itnamed
service-knowledgetwice before I built this package's own dist(
pnpm --filter @objectstack/service-knowledge build); re-run after thatbuild: 0
service-knowledgementions, 84 remain for packages thiscard does not touch. None of the three above is evidence about this
diff — building the other ~60 unbuilt packages is CI's
pnpm buildstep,not a scoped card's local obligation.
Package-level, on the final head:
pnpm --filter @objectstack/service-knowledge typecheckexit 0, and
pnpm --filter @objectstack/service-knowledge test— 4 files / 40tests passed.
Declined, with reasons
ExecutionContextuntouched — the test'srolesfield was stale, thecontract's renamed
positionsfield is correct.pathsrules added — measured at +487 TS6059 in other packages'source (above).
@ts-expect-error/@ts-ignoreadded;strictuntouched.packages/specnot touched (single-owner lane) — only read, to confirmthe
ExecutionContextfield rename.content/docs/releasesedit.packages/services/*packages with the same gap — not touched.service-automationandservice-storagestill lack atypecheckscript(dispatched separately, tooling(services):
service-automationhas notypecheckscript — its test layer is compiled by no tsc program (#14062 family, sibling of #14181) #15048 / tooling(services):service-storagehas notypecheckscript — its test layer is compiled by no tsc program (#14062 family, sibling of #14181) #15050). The family-per-card pattern isdeliberate.
Changeset
.changeset/service-knowledge-test-tsc-program.md,patchon@objectstack/service-knowledge. No runtime code changed —src/**excluding tests is byte-identical, verified — so no shipped behaviour moves;
the level reflects the published
package.jsongainingtypecheck/check:test-typecheckscripts and atsxdevDependency.🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code