Skip to content

Commit ebb5550

Browse files
os-salesclaude
andauthored
fix(service-storage): put the test layer in front of tsc, and repair what it was hiding (#15157)
* fix(service-storage): put the test layer in front of tsc, and repair 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 * tooling(gates): re-measure the #15050 entry's provenance table over the 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e77635b commit ebb5550

20 files changed

Lines changed: 313 additions & 133 deletions
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/service-storage": patch
3+
---
4+
5+
fix(service-storage): put the test layer in front of tsc, and repair what it was hiding (#15050)
6+
7+
`packages/services/service-storage` had **no `typecheck` script at all** — its
8+
scripts were `build` and `test` — so no tsc program anywhere read this
9+
package's test layer, and its errors were carried instead as a 51-error DEBT
10+
entry in `scripts/check-type-check-coverage.mjs`. Gives it the #14062 /
11+
#14181 "checked test zone" shape: a sibling `tsconfig.test.json` (module
12+
semantics only — `esnext` / `bundler` / `lib: ES2022` — matching how vitest
13+
actually executes these files; strictness inherited and untouched) plus a
14+
`tsconfig.scripts.json` for `scripts/i18n-extract.config.ts` (the ninth
15+
instance of #11351, previously excluded from that ledger only because this
16+
package had no `typecheck` script to hang it on), both named by a new
17+
`typecheck` script.
18+
19+
Measured before repair: 51 errors under BUILD semantics (`tsc --noEmit -p
20+
tsconfig.json`, which already includes the tests — matching the DEBT entry's
21+
recorded number exactly), 10 under the split. Unlike `service-cluster`
22+
(#14181), this package's BUILD reading was *not* already clean, so both
23+
programs needed genuine repair, not just the test-only split: 23 `TS2835`
24+
(relative imports missing their `.js` extension, required under BUILD's
25+
NodeNext resolution) were fixed by *adding* the extension — which resolves
26+
correctly under both NodeNext and the split's bundler mode — and clearing
27+
that also cleared all 15 `TS7006` "implicitly any" as a downstream cascade
28+
from the same unresolved imports (the shape `@objectstack/core` reported at
29+
98 → 4). The remaining 3 `TS2550` (`Array.prototype.at` needing `lib`
30+
es2022) are rewritten to indexed access rather than widening the shared
31+
BUILD `tsconfig.json`. The 8 code-tier errors (`TS2339` × 4 — a test
32+
helper's object-spread dropped its `Record<string, unknown>` index
33+
signature, fixed with an explicit return-shape annotation; `TS2347` × 4 — a
34+
fake `ctx: any`'s `getService<T>(...)` calls converted to `getService(...)
35+
as T`, the pattern one call site in the same file had already adopted for
36+
exactly this reason) are genuine test-file fixes. Both readings now agree at
37+
0/0 — the same result `service-cluster` reported, reached by a longer road.
38+
39+
The package's DEBT entry (51 errors) is **deleted**, not lowered — the
40+
graduation this ratchet's invariant requires. No `test-typecheck-debt.json`
41+
is added: residue is 0, so none is owed (#5286, maintainer-only to open).
42+
`check:type-source-resolution` went red from onboarding the two new
43+
programs (the documented onboarding-limb case): a registry entry is added
44+
rather than `paths`, measured both ways — `paths` takes this package's test
45+
layer from 0 errors to 306, all in other packages' source.
46+
47+
No runtime code changes: `src/**` excluding tests is byte-identical, so no
48+
shipped behaviour moves. The `patch` level reflects the published
49+
`package.json` gaining `typecheck` / `check:test-typecheck` scripts and a
50+
`tsx` devDependency.

packages/services/service-storage/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
},
2121
"scripts": {
2222
"build": "tsup --config ../../../tsup.config.ts && node ../../../scripts/check-dts-emitted.mjs",
23-
"test": "vitest run"
23+
"test": "vitest run",
24+
"typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json && pnpm check:test-typecheck",
25+
"check:test-typecheck": "tsx ../../../scripts/check-test-typecheck.mts --self-test && tsx ../../../scripts/check-test-typecheck.mts --package packages/services/service-storage --project tsconfig.test.json"
2426
},
2527
"dependencies": {
2628
"@objectstack/core": "workspace:*",
@@ -45,6 +47,7 @@
4547
"@objectstack/objectql": "workspace:*",
4648
"@objectstack/driver-sql": "workspace:*",
4749
"@types/node": "^26.2.0",
50+
"tsx": "^4.23.12",
4851
"typescript": "^6.0.3",
4952
"vitest": "^4.1.10"
5053
},

packages/services/service-storage/src/error-envelope.conformance.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ import { join } from 'node:path';
4040
import { tmpdir } from 'node:os';
4141
import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
4242
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
43-
import { LocalStorageAdapter } from './local-storage-adapter';
44-
import { StorageMetadataStore } from './metadata-store';
45-
import { registerStorageRoutes } from './storage-routes';
43+
import { LocalStorageAdapter } from './local-storage-adapter.js';
44+
import { StorageMetadataStore } from './metadata-store.js';
45+
import { registerStorageRoutes } from './storage-routes.js';
4646

4747
const BASE = '/api/v1/storage';
4848

packages/services/service-storage/src/file-reference-lifecycle.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,13 @@ function recordDispatch(scope: Record<string, unknown> = {}) {
127127
async function driveInsert(engine: Engine, object: string, data: Record<string, unknown>, id: string) {
128128
const ctx: any = { object, event: 'beforeInsert', input: { data }, dispatch: recordDispatch() };
129129
await engine.trigger('beforeInsert', ctx);
130-
const row = { ...(ctx.input.data as Record<string, unknown>), id };
130+
// Explicitly typed rather than left to spread-inference: TS drops a spread
131+
// source's index signature when it has no KNOWN properties, so an
132+
// un-annotated `row` here infers as bare `{ id: string }` and every
133+
// `row.<dataKey>` read below is an error — the shape IS
134+
// `Record<string, unknown>` at runtime (the caller's `data` plus `id`), this
135+
// just states it so the checker agrees.
136+
const row: Record<string, unknown> & { id: string } = { ...(ctx.input.data as Record<string, unknown>), id };
131137
(engine.tables[object] ??= []).push(row);
132138
ctx.event = 'afterInsert';
133139
ctx.result = row;

packages/services/service-storage/src/local-storage-adapter.metrics.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { mkdtemp, rm } from 'node:fs/promises';
55
import { tmpdir } from 'node:os';
66
import { join } from 'node:path';
77
import { InMemoryMetricsRegistry, SEMCONV } from '@objectstack/observability';
8-
import { LocalStorageAdapter } from './local-storage-adapter';
8+
import { LocalStorageAdapter } from './local-storage-adapter.js';
99

1010
describe('LocalStorageAdapter instrumentation', () => {
1111
let rootDir: string;

packages/services/service-storage/src/local-storage-adapter.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { describe, it, expect, afterEach } from 'vitest';
44
import { promises as fs } from 'node:fs';
55
import { join } from 'node:path';
66
import { tmpdir } from 'node:os';
7-
import { LocalStorageAdapter } from './local-storage-adapter';
7+
import { LocalStorageAdapter } from './local-storage-adapter.js';
88
import type { IStorageService } from '@objectstack/spec/contracts';
99

1010
describe('LocalStorageAdapter', () => {

packages/services/service-storage/src/storage-adapter-list-contract.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@
3030
*/
3131

3232
import { describe, it, expect } from 'vitest';
33-
import { LocalStorageAdapter } from './local-storage-adapter';
34-
import { S3StorageAdapter } from './s3-storage-adapter';
33+
import { LocalStorageAdapter } from './local-storage-adapter.js';
34+
import { S3StorageAdapter } from './s3-storage-adapter.js';
3535

3636
/** Every method name reachable on an instance, own + prototype chain. */
3737
function reachableMethodNames(instance: object): string[] {

packages/services/service-storage/src/storage-adapter-list.conformance.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@ vi.mock('@aws-sdk/client-s3', () => {
155155
// `vi.mock` is hoisted above every import in this file, so the lazy
156156
// `await import('@aws-sdk/client-s3')` inside `S3StorageAdapter` resolves to the
157157
// fake above no matter where these two sit.
158-
import { LocalStorageAdapter } from './local-storage-adapter';
159-
import { S3StorageAdapter } from './s3-storage-adapter';
158+
import { LocalStorageAdapter } from './local-storage-adapter.js';
159+
import { S3StorageAdapter } from './s3-storage-adapter.js';
160160

161161
// ---------------------------------------------------------------------------
162162
// Backend harness
@@ -365,8 +365,8 @@ describe.each(BACKENDS)('$name adapter — list() conformance', ({ make }) => {
365365
expect(page.keys).toHaveLength(400);
366366
expect(page.nextCursor).toBeDefined();
367367
}
368-
expect(pages.at(-1)!.keys).toHaveLength(BULK_SET.length % 400);
369-
expect(pages.at(-1)!.nextCursor).toBeUndefined();
368+
expect(pages[pages.length - 1]!.keys).toHaveLength(BULK_SET.length % 400);
369+
expect(pages[pages.length - 1]!.nextCursor).toBeUndefined();
370370
}, 60_000);
371371

372372
it('fills one page past the backend\'s own page size (>1000 in a single call)', async () => {
@@ -434,7 +434,7 @@ describe.each(BACKENDS)('$name adapter — list() conformance', ({ make }) => {
434434
it('issues a cursor the contract codec can read back', async () => {
435435
const page = await backend.adapter.list!('bulk/', { limit: 10 });
436436

437-
expect(page.nextCursor).toBe(encodeStorageListCursor(page.items.at(-1)!.key));
437+
expect(page.nextCursor).toBe(encodeStorageListCursor(page.items[page.items.length - 1]!.key));
438438
});
439439
});
440440

packages/services/service-storage/src/storage-route-ledger.conformance.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
*/
2626

2727
import { describe, it, expect, vi } from 'vitest';
28-
import { registerStorageRoutes } from './storage-routes';
29-
import { STORAGE_ROUTE_LEDGER } from './storage-route-ledger';
28+
import { registerStorageRoutes } from './storage-routes.js';
29+
import { STORAGE_ROUTE_LEDGER } from './storage-route-ledger.js';
3030

3131
/** Minimal IHttpServer mock that records registrations. */
3232
function createMockServer() {

packages/services/service-storage/src/storage-routes.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
44
import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts';
5-
import { LocalStorageAdapter } from './local-storage-adapter';
6-
import { StorageMetadataStore } from './metadata-store';
7-
import { registerStorageRoutes } from './storage-routes';
5+
import { LocalStorageAdapter } from './local-storage-adapter.js';
6+
import { StorageMetadataStore } from './metadata-store.js';
7+
import { registerStorageRoutes } from './storage-routes.js';
88
import { promises as fs } from 'node:fs';
99
import { join } from 'node:path';
1010
import { tmpdir } from 'node:os';
@@ -409,7 +409,7 @@ describe('Storage REST Routes', () => {
409409
});
410410

411411
describe('attachments download gate (#2970 item 2)', () => {
412-
const commit = async (s: StorageMetadataStore, rec: Partial<import('./metadata-store').FileRecord>) =>
412+
const commit = async (s: StorageMetadataStore, rec: Partial<import('./metadata-store.js').FileRecord>) =>
413413
s.createFile({
414414
id: rec.id ?? 'f-dl',
415415
key: rec.key ?? `attachments/${rec.id ?? 'f-dl'}.bin`,
@@ -421,7 +421,7 @@ describe('Storage REST Routes', () => {
421421
...rec,
422422
} as any);
423423

424-
function serverWith(verdict: import('./storage-routes').FileReadVerdict | 'skip', extra: any = {}) {
424+
function serverWith(verdict: import('./storage-routes.js').FileReadVerdict | 'skip', extra: any = {}) {
425425
const server = createMockHttpServer();
426426
const s = new StorageMetadataStore(null);
427427
const authorizeFileRead =

0 commit comments

Comments
 (0)