Skip to content

Commit b30b5c2

Browse files
krisnyeclaude
andauthored
fix(ci): add PR build/test gate and repair all tsc -b + lint failures (#115)
* fix(ci): add PR build/test gate and repair all tsc -b + lint failures The repo had no pull_request CI workflow and main had no branch protection, so PR #114 merged green while `tsc -b` and lint were red. This adds a CI gate and fixes every pre-existing and new failure so the gate is green. Type errors (tsc -b, the real check — bare `tsc --noEmit` is a no-op here because tsconfig.json only has `references`): - database.index.test.ts: ordered-select routing tests referenced where/order columns not in `include`, and a `seed` helper typed `db` as the empty Database; both fixed. - TransactionResult.changedComponents: Set<keyof C | string> -> Set<string>. Component names are always strings; the `keyof C` only widened to string|number|symbol for generic C, breaking the type-erased TransactionResult<unknown> boundary the reconciler/strategy is written against. Cleared the producer-side `as keyof C` workaround casts. - reconciling applier: store entries as ReconcilingEntry<any,any,any> (the layer is type-erased over getTransaction's any-ctx transactions). - create-store.test.ts: guard optional `.health?.current`. Lint: - create-database.test.ts: describe each @ts-expect-error directive. - test-setup.ts: import { webcrypto } instead of require(). CI / tooling: - .github/workflows/ci.yml: typecheck + lint + test on every PR and push to main, scoped to @adobe/data (where the regression class occurred). - packages/data: add `typecheck` (build wasm + tsc -b, correct for the project-reference build) and `test:ci` (SKIP_PERF=1) scripts. - vite.config.js / vitest.workspace.ts: exclude *.performance.test.ts when SKIP_PERF=1 so the gate is deterministic (timing-ratio perf tests are flaky on shared runners); a normal `pnpm test` still runs them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(ci): RED probe — deliberate type error to verify gate blocks merge Intentionally broken; reverted in the next commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revert CI probe — restore green state Removes the deliberate type error from the previous commit; the gate is verified to block on red. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: bump actions to v6 (Node 24 runtime) and node-version to 22 Clears the Node.js 20 action-runtime deprecation warning. checkout, setup-node, and pnpm/action-setup -> v6; project runtime node-version 20 -> 22 (LTS). Applied to both ci.yml and deploy-docs.yml. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent cd3fd2e commit b30b5c2

12 files changed

Lines changed: 98 additions & 24 deletions

File tree

.github/workflows/ci.yml

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
name: CI
2+
3+
# Validates every pull request (and pushes to main) before merge. Prior to
4+
# this workflow the only GitHub Action was the post-merge docs deploy, so a PR
5+
# whose `tsc -b` / unit tests were red could still be merged green — exactly
6+
# what happened with #114. This gate makes those failures block the merge.
7+
#
8+
# Scope: the published core library `@adobe/data`, where the regression class
9+
# occurred. Type-checking uses the project-reference build (`tsc -b`, via the
10+
# package's `typecheck` script, which builds the wasm artifact the source
11+
# imports first). Tests run with SKIP_PERF=1 (`test:ci`) so the gate excludes
12+
# the non-deterministic timing-ratio perf tests and stays reliable.
13+
14+
on:
15+
pull_request:
16+
push:
17+
branches: [main]
18+
19+
permissions:
20+
contents: read
21+
22+
concurrency:
23+
group: ci-${{ github.ref }}
24+
cancel-in-progress: true
25+
26+
jobs:
27+
data:
28+
name: "@adobe/data — typecheck, lint, test"
29+
runs-on: ubuntu-latest
30+
steps:
31+
- uses: actions/checkout@v6
32+
33+
- uses: pnpm/action-setup@v6
34+
with:
35+
version: 9
36+
37+
- uses: actions/setup-node@v6
38+
with:
39+
node-version: 22
40+
cache: pnpm
41+
42+
- run: pnpm install --frozen-lockfile
43+
44+
- name: Type-check (tsc -b, project references)
45+
run: pnpm --filter @adobe/data run typecheck
46+
47+
- name: Lint
48+
run: pnpm --filter @adobe/data run lint
49+
50+
- name: Unit tests
51+
run: pnpm --filter @adobe/data run test:ci

.github/workflows/deploy-docs.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@ jobs:
1818
build:
1919
runs-on: ubuntu-latest
2020
steps:
21-
- uses: actions/checkout@v4
21+
- uses: actions/checkout@v6
2222

23-
- uses: pnpm/action-setup@v4
23+
- uses: pnpm/action-setup@v6
2424
with:
2525
version: 9
2626

27-
- uses: actions/setup-node@v4
27+
- uses: actions/setup-node@v6
2828
with:
29-
node-version: 20
29+
node-version: 22
3030
cache: pnpm
3131

3232
- run: pnpm install --frozen-lockfile

packages/data/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"build": "pnpm build-assembly && run-p build:*",
2121
"build:code": "tsc -b",
2222
"build:apidocs": "typedoc",
23+
"typecheck": "pnpm build-assembly && tsc -b",
2324
"build-assembly": "run-p asbuild:release",
2425
"clean": "rm -rf dist build node_modules",
2526
"clean-cache": "rm -rf node_modules/.cache",
@@ -38,6 +39,7 @@
3839
"publish-public": "pnpm build && pnpm publish --no-git-checks --access public",
3940
"perftest": "node dist/perftest/index.js",
4041
"test": "vitest --run",
42+
"test:ci": "SKIP_PERF=1 vitest --run",
4143
"asbuild:debug": "asc assembly/index.ts -o dist/assembly/index.wasm --target debug --enable simd && echo built dist/assembly/index.wasm",
4244
"asbuild:release": "asc assembly/index.ts -o dist/assembly/index.wasm --target release --enable simd --optimize && echo built dist/assembly/index.wasm",
4345
"start": "npx serve ."

packages/data/src/ecs/database/database.index.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
10101010
return { state, restore: () => { handle.find = original; } };
10111011
};
10121012

1013-
const seed = (db: ReturnType<typeof Database.create>) => {
1013+
const seed = (db: Database.FromPlugin<ReturnType<typeof plugin>>) => {
10141014
const mid = db.transactions.add({ owner: 1, priority: 2, due: 10, title: "mid" });
10151015
const low = db.transactions.add({ owner: 1, priority: 1, due: 20, title: "low" });
10161016
const high = db.transactions.add({ owner: 1, priority: 3, due: 30, title: "high" });
@@ -1024,7 +1024,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
10241024

10251025
const sorted = spyFind((db.indexes as any).byOwnerSorted);
10261026
try {
1027-
const result = db.select(["title"], { where: { owner: 1 }, order: { priority: true } });
1027+
const result = db.select(["owner", "priority"], { where: { owner: 1 }, order: { priority: true } });
10281028
// GREEN: the ordered query was served by the sorted index...
10291029
expect(sorted.state.calls).toBe(1);
10301030
// ...and the result is in the index's ascending priority order.
@@ -1041,7 +1041,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
10411041
const unsorted = spyFind((db.indexes as any).byOwner);
10421042
const sorted = spyFind((db.indexes as any).byOwnerSorted);
10431043
try {
1044-
db.select(["title"], { where: { owner: 1 }, order: { priority: true } });
1044+
db.select(["owner", "priority"], { where: { owner: 1 }, order: { priority: true } });
10451045
expect(sorted.state.calls).toBe(1);
10461046
expect(unsorted.state.calls).toBe(0);
10471047
} finally {
@@ -1056,7 +1056,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
10561056

10571057
const sorted = spyFind((db.indexes as any).byOwnerSorted);
10581058
try {
1059-
const result = db.select(["title"], { where: { owner: 1 }, order: { priority: false } });
1059+
const result = db.select(["owner", "priority"], { where: { owner: 1 }, order: { priority: false } });
10601060
// Not routed — the index can't serve descending without re-sorting.
10611061
expect(sorted.state.calls).toBe(0);
10621062
// The scan still produces a correct descending result.
@@ -1073,7 +1073,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
10731073
const sorted = spyFind((db.indexes as any).byOwnerSorted);
10741074
try {
10751075
// Index sorts by `priority`; this asks for `due`.
1076-
const result = db.select(["title"], { where: { owner: 1 }, order: { due: true } });
1076+
const result = db.select(["owner", "due"], { where: { owner: 1 }, order: { due: true } });
10771077
expect(sorted.state.calls).toBe(0);
10781078
expect(result).toHaveLength(3);
10791079
} finally {
@@ -1088,7 +1088,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
10881088
// No order: the first matching key index (byOwner) serves it.
10891089
const unsorted = spyFind((db.indexes as any).byOwner);
10901090
try {
1091-
const result = db.select(["title"], { where: { owner: 1 } });
1091+
const result = db.select(["owner"], { where: { owner: 1 } });
10921092
expect(unsorted.state.calls).toBe(1);
10931093
expect(result).toHaveLength(3);
10941094
} finally {
@@ -1103,7 +1103,7 @@ describe("auto-routing of ordered select through a sorted index", () => {
11031103
const sorted = spyFind((db.indexes as any).byOwnerSorted);
11041104
try {
11051105
const emissions: number[][] = [];
1106-
const unsub = db.observe.select(["title"], { where: { owner: 1 }, order: { priority: true } })(
1106+
const unsub = db.observe.select(["owner", "priority"], { where: { owner: 1 }, order: { priority: true } })(
11071107
(entities) => { emissions.push([...entities]); },
11081108
);
11091109
// Initial snapshot routed and sorted.

packages/data/src/ecs/database/public/create-database.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1305,15 +1305,15 @@ describe("createDatabase", () => {
13051305
});
13061306

13071307
// Wrong value type: token should be string, not number
1308-
// @ts-expect-error
1308+
// @ts-expect-error token must be a string
13091309
Database.create(plugin, { services: { auth: { token: 123, isAuthenticated: false } } });
13101310

13111311
// Unknown service key
1312-
// @ts-expect-error
1312+
// @ts-expect-error nonexistent is not a declared service
13131313
Database.create(plugin, { services: { nonexistent: { foo: 'bar' } } });
13141314

13151315
// Missing required property (isAuthenticated)
1316-
// @ts-expect-error
1316+
// @ts-expect-error auth override is missing isAuthenticated
13171317
Database.create(plugin, { services: { auth: { token: 'x' } } });
13181318
});
13191319
});

packages/data/src/ecs/database/reconciling/create-rebase-replay-applier.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,12 @@ export function createRebaseReplayApplier(
3232
replayAllTransients: () => TransactionResult<unknown> | undefined;
3333
} {
3434
const [execute, getTransaction] = args;
35-
const reconcilingEntries: ReconcilingEntry[] = [];
35+
// The applier is a type-erased layer: `getTransaction` hands back
36+
// transactions typed over `TransactionContext<any, any, any>`, so the
37+
// entries it stores are erased too. Pinning the generics to `any` keeps the
38+
// stored `transaction` assignable from `getTransaction`'s result without a
39+
// per-assignment cast.
40+
const reconcilingEntries: ReconcilingEntry<any, any, any>[] = [];
3641

3742
const rollbackEntryResult = (entry: ReconcilingEntry) => {
3843
if (entry.result) {
@@ -90,7 +95,7 @@ export function createRebaseReplayApplier(
9095
rollbackAllTransients();
9196
spliceTransientEntry(id, userId);
9297

93-
const entry: ReconcilingEntry = {
98+
const entry: ReconcilingEntry<any, any, any> = {
9499
id,
95100
userId,
96101
name: envelope.name,

packages/data/src/ecs/database/transactional-store/create-transactional-store.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export function createTransactionalStore<
3939
};
4040
const changed = {
4141
entities: new Map<Entity, EntityUpdateValues<C> | null>(),
42-
components: new Set<keyof C>(),
42+
components: new Set<string>(),
4343
archetypes: new Set<ArchetypeId>(),
4444
};
4545

@@ -62,7 +62,7 @@ export function createTransactionalStore<
6262
changed.entities.set(entity, values);
6363
changed.archetypes.add(id);
6464
for (const key in values) {
65-
changed.components.add(key as never);
65+
changed.components.add(key);
6666
}
6767
return entity;
6868
},
@@ -98,7 +98,7 @@ export function createTransactionalStore<
9898
oldValue = DELETE;
9999
}
100100
replacedValues[name] = oldValue;
101-
changed.components.add(name as keyof C);
101+
changed.components.add(name);
102102
}
103103
}
104104

@@ -136,7 +136,7 @@ export function createTransactionalStore<
136136

137137
const { id: _ignore, ...oldValuesWithoutId } = oldValues as any;
138138
for (const key in oldValuesWithoutId) {
139-
changed.components.add(key as keyof C);
139+
changed.components.add(key);
140140
}
141141

142142
store.delete(entity);

packages/data/src/ecs/database/transactional-store/transactional-store.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ export interface TransactionResult<C = unknown> {
9696
readonly redo: TransactionWriteOperation<C>[];
9797
readonly undo: TransactionWriteOperation<C>[];
9898
readonly changedEntities: Map<Entity, EntityUpdateValues<C> | null>;
99-
readonly changedComponents: Set<keyof C | string>;
99+
// Component names are always strings. Keeping this `Set<string>` (rather
100+
// than `Set<keyof C | string>`) avoids widening to `string | number |
101+
// symbol` for a generic `C`, which otherwise makes `TransactionResult<C>`
102+
// fail to satisfy the type-erased `TransactionResult<unknown>` boundary the
103+
// concurrency strategy / reconciler is written against.
104+
readonly changedComponents: Set<string>;
100105
readonly changedArchetypes: Set<ArchetypeId>;
101106
}

packages/data/src/ecs/store/public/create-store.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,7 @@ describe("createStore", () => {
630630
const fresh = createStore({ components: { health: healthSchema }, resources: {}, archetypes: {} });
631631
fresh.fromData(snapshot);
632632
const restored = fresh.select(["health"]);
633-
return fresh.read(restored[0])?.health.current;
633+
return fresh.read(restored[0])?.health?.current;
634634
};
635635

636636
// Detached: mutating after the snapshot must NOT change it.

packages/data/src/test-setup.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// © 2026 Adobe. MIT License. See /LICENSE for details.
22

3+
import { webcrypto } from "node:crypto";
4+
35
// Polyfills for Node 18 test environment
46

57
// window polyfill (minimal — only what the source files actually use)
@@ -58,7 +60,7 @@ if (typeof globalThis.caches === "undefined") {
5860

5961
// Web Crypto API: Node 18 has globalThis.crypto but not the bare `crypto` name in all contexts
6062
if (typeof (globalThis as any).crypto === "undefined") {
61-
(globalThis as any).crypto = require("node:crypto").webcrypto;
63+
(globalThis as any).crypto = webcrypto;
6264
}
6365

6466
// requestAnimationFrame polyfill for Node

0 commit comments

Comments
 (0)