Uh oh!
There was an error while loading. Please reload this page.
fix(service-storage): put the test layer in front of tsc, and repair what it was hiding - #15157
Merged
Merged
Conversation
…what it was hiding (#15050) `packages/services/service-storage` had no `typecheck` script at all, so no tsc program anywhere read its test layer; its 51 errors were carried as a DEBT entry. Gives it the #14062/#14181 checked-test-zone shape: a sibling `tsconfig.test.json` plus `tsconfig.scripts.json` (the ninth `i18n-extract` instance of #11351), both named by a new `typecheck` script. Fixes the config-tier pile (missing `.js` extensions, `Array.prototype.at` lib gap) and the genuine code-tier defects it was hiding (a test helper's spread losing its index signature; a fake `ctx: any`'s generic calls). Both readings now agree at 0/0. DEBT entry deleted, not lowered; no test-typecheck-debt.json needed. `check:type-source-resolution` repaired via the documented onboarding-limb registry entry, not `paths` (measured both ways: `paths` -> 306 errors, all in other packages' source). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Contributor
📓 Docs Drift Check
What this run could not see
Coarse fallback — 6 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
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
github-merge-queueBot
removed this pull request from the merge queue due to a conflict with the base branch
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
…he two landed siblings The merge of origin/main @ 460134a brought in the service-knowledge (#15049) and service-automation (#15048) onboardings, which moved every ABSOLUTE this entry's doc-block states and none of its DELTAS. Re-measured all four provenance rows on the merged tree by varying only what the `typecheck` script NAMES, restoring package.json from HEAD between rows (restore proven byte-identical: hash-object == HEAD blob 40773ae, `git diff HEAD` empty): no `typecheck` script (origin/main) absent 121 programs / 302 pairs names `tsconfig.json` only absent 121 programs / 302 pairs names `tsconfig.test.json` only PRESENT 122 programs / 309 pairs names all three (this card) PRESENT 123 programs / 309 pairs origin/main itself measures 60 of 78 / 121 / 302 / 18 clean in a detached worktree at 460134a, matching rows 1-2. Deltas unchanged: +1 package, +2 programs, +7 pairs. `check-type-check-coverage.mjs` needed no equivalent edit: its ledger summary is computed at runtime from DEBT/TEST_DEBT and now prints 75/79 covered, 4 in DEBT, 53 frozen raw errors. 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#15050
packages/services/service-storagehad notypecheckscript at all (its scripts werebuildandtest), so no tsc program anywhere read this package's test layer. Turbo/CI typecheck lanes skipped it silently, because a zero-matching filter run exits 0.tsuptranspiles with esbuild andvitestruns through esbuild type-stripping; neither type-checks.Clause-②: no — this is the checked-test-zone mechanism (#14062/#14181) applied one package over; it touches no spec/API contract surface, only this package's own test files, tsc configs and the two shared ledger scripts the ratchet already routes graduations through.
The route, and which exemplar it was copied from
Copied from
plugin-webhooks/service-cluster(#14181, PR #15032 — read before starting), not fromplugin-auth/plugin-sharing/core, because it is the structural match. The deciding property is what the BUILD config does with tests:tsconfig.jsonexcludes tests?plugin-auth,plugin-sharing,coreplugin-webhooks,service-cluster,service-storageservice-storage'stsconfig.jsonincludes the tests and always did, so the program 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 (module semantics only —esnext/bundler/lib: ES2022; strictness inherited, nopaths).A ninth file also needed the same treatment:
scripts/i18n-extract.config.tsis the ninth instance #11351 onboarded for the other 8 packages that carry one — deliberately left out of that ledger before now, becauseSOURCES_COVEREDonly asks its question of a package that declarestypecheck, and this one didn't. Giving it atypecheckscript makes that question start firing here, sotsconfig.scripts.json(copied frompackages/objectql's minimal shape) completes the family in the same PR rather than leaving a new gap the momenttypecheckexists.Measured error count, both ways, before and after
Dependency closure built first (
pnpm --filter '@objectstack/service-storage^...' build;@objectstack/objectql's DTS worker had been truncated by an unrelated 10-minute foreground kill on the first attempt — see Findings — and was rebuilt before measuring):tsc --noEmit -p tsconfig.json, already includes tests)tsc --noEmit -p tsconfig.test.json)1102 files in the BUILD program / 1068 in the test program, covering all 35 of the package's
src/**/*.test.ts. (Re-measured after the merge below with--listFiles: still 1102 / 1068 / 35, both programs still exit 0.)Both readings agree at 0/0 — the same headline result
service-clusterreported, reached by a longer road. Unlike that package, this one's BUILD reading was not already clean, so the 51→0 needed real repair on both legs, not just the test-only split:TS2339× 4,TS2347× 4) — genuine test-file defects, fixed in the tests (below).TS2835× 23 relative-import extensions,TS2550× 3Array.prototype.at) — repaired, not routed around.TS7006× 15,TS6196× 1,TS6133× 1).The composition matches the DEBT entry's own note (
code-tier 8 (TS2339 x4, TS2347 x4); config-tier 26 (TS2835 x23, TS2550 x3); noise 17 (TS7006 x15, TS6196, TS6133)) exactly — a positive control that the 51-measurement is reading the same thing the ledger already recorded.The config-tier repair, and why it's a repair rather than a workaround
The 23
TS2835are relative imports missing their.jsextension, required under BUILD's NodeNext resolution but optional under the split's bundler resolution. The fix is to add the extension (./local-storage-adapter→./local-storage-adapter.js), across 12 test files — this resolves correctly under both module modes, so it isn't a config-only accommodation, it's a real fix. Doing so also cleared all 15TS7006as a downstream cascade — the same shape@objectstack/corereported at 98 → 4 (#14916): an unresolved relative import makes every symbol it namesany, and once the import resolves, the "implicitly any" errors disappear with it. Measured directly: 41 BUILD errors before the extension fix, 3 after (allTS2550).The remaining 3
TS2550(Array.prototype.atneedinglibes2022) are rewritten to indexed access (arr[arr.length - 1]) instorage-adapter-list.conformance.test.tsrather than widening the shared BUILDtsconfig.json'slib— the narrower fix, and it leaves the BUILD config untouched.The 8 code-tier errors, and why each fix is correct rather than convenient
TS2339× 4,file-reference-lifecycle.test.ts— thedriveInserttest helper buildsrowby spreading a value typedRecordof string keys to unknown values, plusid. TypeScript's object-spread inference drops the source's index signature when it has no known properties, so an un-annotatedrowinfers as bare{ id: string }and every per-key read offrowis an error — even though the runtime shape genuinely carries the caller's fields. Reproduced in isolation (a 5-line repro) before touching the file. Fixed with an explicit annotation onrow— the sameRecordshape intersected with{ id: string }— which states the true type rather than widening anything.TS2347× 4,storage-service-plugin.test.ts— a fakectx: any'sgetServicemethod (declared generic, one type parameter) becomesany-typed through the outer annotation, so calling it WITH an explicit type argument —ctx.getService('storage')spelled with the type argument inline, IStorageService — is invalid (TS can't verify a generic call throughany). One call site in this same file had already hit this and left a comment documenting the fix: "Plain calls with casts, not the type-argument form — the fake ctx's getService is untyped, and each type-argument call adds a frozen-debt TS2347..." — the other three call sites just hadn't been converted yet. Converted all four to the cast form,ctx.getService('storage') asthe target type, the pattern the file itself had already established.TS6196× 1 — a genuinely deadStorageFileInfoimport, invisible until a tsc program finally read the file. Removed.TS6133× 1 — an unused destructured param in a.map()callback. Renamed to_r(the underscore-ignore convention already used elsewhere in this package's tests).Ledger consequences
DEBT['@objectstack/service-storage']deleted, not lowered —errors: 51is gone; the graduation note is recorded incheck-type-check-coverage.mjs's own prose, per its convention. Post-merge figures: the gate now reads 53 raw errors across 4 packages, wheremainat460134af8reads 104 across 5. (The pre-merge body said 66/6 from 117/7; both siblings of this family have since landed and taken their own entries out, so those two pairs are stale and are replaced here.)test-typecheck-debt.jsoncreated — residue is 0, so none is owed; opening one is maintainer-only (@ts-expect-error退役 pin 在packages/spec里是幽灵检查:tsconfig 把**/*.test.ts排除出唯一的tsc --noEmit#5286).UNCHECKED_SOURCE_DEBT's "ninth config" paragraph updated to record thatscripts/i18n-extract.config.tsjoined the other eight, measuring 0 errors under its newtsconfig.scripts.json, same as all eight siblings.check:type-source-resolution— an onboarding re-baseline, please review it as oneThis gate went red on my diff. Onboarding
tsconfig.test.json+tsconfig.scripts.jsonmoves the package's tsc PROGRAM SET, which the gate judges per program — its own doc-block opens an explicit onboarding limb for exactly this, on the same three terms #14181/PR #15032 used forservice-cluster.Term 1 — provenance, measured four ways on one checkout by varying only what
typechecknames (--list). Re-measured on the post-merge tree (the two landed siblings moved every absolute here and none of the deltas), restoringpackage.jsonfromHEADbetween rows and proving the restore byte-identical:typechecknamestsconfig.jsononlytsconfig.test.jsononlyRows 1–2 match
origin/main@460134af8measured on its own in a detached worktree (60 of 78 packages, 121 programs, 302 pairs, 18 clean).Row 2 is load-bearing: BUILD carries zero dist-resolved workspace type imports, so the exposure is only reachable through the onboarded programs, not merely first seen there.
Term 2 — numbers: +1 package (60→61 of 78), +2 programs, +7 pairs — this entry and nothing else. The deltas are unchanged from the reviewed reading; only the absolutes moved (the pre-merge body said 58→59).
Term 3 — why the registry entry and not
paths, measured both ways on the pre-merge checkout (temporarypathsblock added totsconfig.test.json,tsc --noEmit -prun, then removed — never committed): redirecting all 7 deps to source takes this package's test layer from 0 errors to 306 (305 ×TS6059"not under rootDir" + 1 ×TS6133), everyTS6059in another package's source (packages/types/src/**,packages/spec/src/**,packages/objectql/src/**,packages/observability/src/**,packages/drivers/driver-sql/src/**) — billed to a package that cannot pay it down. Same shape as #12570 (+5 forrest) and #8021 (247TS6059), reproduced at larger scale on a package whose whole point here is reaching zero. This 306 was NOT re-measured after the merge — and it is not being claimed as unchangeable:mainmovedpackages/spec/src/**and other trees the TS6059 land in, so the exact number could differ today. What the merge cannot flip is the direction the term rests on (apathsredirect bills hundreds of other packages'TS6059to this one), and the committed tree contains nopathseither way.Gates
All figures in this section are the post-merge re-run, on head
2e8cbeed5over merge base460134af8— not carried over from the pre-merge round.origin/mainwas merged into this branch (merge commita58676160), never rebased; the conflict was incheck-type-check-coverage.mjsonly, and it resolved as "keep every entry" — all three graduation paragraphs kept, all three DEBT entries gone.Derived with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandsagainst the post-merge diff (20 paths vs merge base460134af8).66 command(s) — 36 pnpm, 30 direct node. All run, all 66 green.
The three that read exit 3 (PREREQUISITE NOT MET) in the pre-merge round —
check:dual-build-cjs-loads,check:i18n,check:type-check-debt— plus the artifact-roster gatecheck:published-readme-exportsthat read exit 1 for the same missing-dist/reason, were all re-run after building the whole workspace (turbo run build --filter './packages/*' --filter './packages/*/*', 71/71 successful) and all four are exit 0.check:type-check-debtre-measured 14 ledger entries in 147.9s, 153 raw tsc errors total, none above its recorded number.check:type-check-coverageverdict on the merged tree:OK — 75/79 workspace packages type-checked (plus the root), 4 in the DEBT ledger (53 frozen raw errors), 1 exempt.check:type-source-resolutionverdict:OK — 123 tsc program(s) across 78 packages scanned; 61 registered as still resolving a workspace dep's types through dist/.Package-level, on the merged head:
pnpm --filter @objectstack/service-storage typecheck— exit 0, all three legs (tsc --noEmit,tsc --noEmit -p tsconfig.scripts.json,check:test-typecheck) clean,0 file(s) / 0 error(s) / 0 pinned signature(s).pnpm --filter @objectstack/service-storage exec vitest run— exit 0, 35 files / 536 tests passed, the same counts the pre-merge round recorded.Both edited gate scripts (
check-type-check-coverage.mjs,check-type-source-resolution.mjs) ran their own--self-testgreen on the merged tree.pnpm-lock.yamlwas not hand-merged:pnpm installover the merged manifest set leaves the auto-merged file byte-identical (git hash-object=000bb7dfbcebaca6c6b92a26d9d970d464c3f6fdbefore and after), andpnpm install --frozen-lockfileexits 0.Figures NOT re-measured after the merge, stated as such rather than implied: the before columns of the error table (51 BUILD / 10 test layer), their tier decomposition (8 / 26 / 17), the intermediate
41 → 3extension-fix reading, and the 306-errorpathsmeasurement in Term 3. All four are readings of the pre-fix tree; the after column (0 / 0) is the live claim and it is re-measured above.Findings (out of scope, reported)
@objectstack/objectql's DTS worker was truncated by the ~10-minute foreground kill on the firstpnpm --filter '@objectstack/service-storage^...' buildattempt in a fresh worktree:dist/had.js/.mjsbut zero.d.tsfiles,check-dts-emitted.mjsnever ran (the wholepnpm buildrecursive run was SIGTERM'd mid-flight), yet thedist/*.jsfiles were already on disk. This produced 16 spuriousTS7016("Could not find a declaration file for module '@objectstack/objectql'") errors in the first undivided measurement (73, not 51) — a false read that would have been invisible without the ledger's 51 as a positive control to notice the mismatch against. Not filed as a separate issue: it's a restatement of the platform-fact AGENTS.md already documents (front-of-process ~10-minute SIGTERM cap can land mid-build), not a new defect — rebuilding the one truncated package (pnpm --filter '@objectstack/objectql' build, ~17s for its DTS step alone) fixed it.Declined, with reasons
pathsrules added — measured at +306TS6059in other packages' source (above).@ts-expect-error/@ts-ignoreadded;strictuntouched; BUILDtsconfig.json'slibuntouched (the 3TS2550sites were rewritten instead).packages/specnot touched (single-owner lane) — only read, to confirmPubSubHandler/IStorageServicecontracts were not implicated.content/docs/releasesedit.service-automationorservice-knowledge, whose PRs have landed; the diff againstmainis the same 20 files it was when reviewed.Changeset
.changeset/service-storage-test-tsc-program.md,patchon@objectstack/service-storage. No runtime code changed —src/**excluding tests is byte-identical — so no shipped behaviour moves; the level reflects the publishedpackage.jsongainingtypecheck/check:test-typecheckscripts and atsxdevDependency.The merge-conflict round was carried out by a second Claude Code session, https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y; the implementation round's session is named in the footer below.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
Generated by Claude Code