chore: enable lint and CI #17
Conversation
WalkthroughThis change set introduces a unified expression type system by moving expression type definitions to a new SDK module and standardizing the usage of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK
participant Runtime
participant ExpressionUtils
User->>SDK: Import expression types from SDK
SDK->>ExpressionUtils: Provide expression utilities (type guards, factories)
User->>Runtime: Use ExpressionUtils for expression construction and checks
Runtime->>SDK: Import types from SDK (e.g., Expression, SchemaDef)
Runtime->>ExpressionUtils: Use utilities for runtime logic and validation
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (2)
packages/runtime/src/plugins/policy/expression-transformer.ts (1)
109-136: 🛠️ Refactor suggestionPrefer eliminating
// @ts-expect-errorby tightening the method signaturesThe three handlers now use
// @ts-expect-error, which is definitely an improvement over a blanket@ts-ignore, but we still rely on a suppression instead of typing the parameters properly.
Given that each handler’s first parameter is strongly typed (LiteralExpression,ArrayExpression,FieldExpression) and the second parameter is either unused or already typed, the remaining error usually comes fromthis’s contextualthis: anythat decorators inject.
Consider declaring the methods as plain functions (or extracting them outside the class) and registering them explicitly, or addingprivate _literal = (expr: LiteralExpression) => {/* … */}which solves the contextual-
thistype problem and removes the need for the directive altogether. At a minimum, add a short reason comment (// @ts-expect-error <why>) to keep future maintainers from wondering what error is expected.🧰 Tools
🪛 Biome (1.9.4)
[error] 109-109: This parameter is used before its declaration.
The parameter is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 122-122: This parameter is used before its declaration.
The parameter is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 133-133: This parameter is used before its declaration.
The parameter is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
packages/runtime/src/client/query-utils.ts (1)
210-214: 🛠️ Refactor suggestionUse a typed function instead of
Function | undefinedUsing the generic
Functiondefeats type-safety and forces an ESLint suppression.
Keep the intent but with an explicit signature:- // eslint-disable-next-line @typescript-eslint/ban-types - let computer: Function | undefined; + let computer: + | ((eb: ExpressionBuilder<any, any>) => ExpressionWrapper<any, any, unknown>) + | undefined;You can then remove the
eslint-disablecomment.🧰 Tools
🪛 Biome (1.9.4)
[error] 211-211: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🧹 Nitpick comments (18)
packages/runtime/src/client/executor/zenstack-driver.ts (1)
133-135: Suppress false-positiveno-this-aliaslint warning.
The inline ESLint directive is justified to preserve thethisalias required for logging. Consider adding a brief inline comment explaining why the alias is necessary for clarity.packages/sdk/src/ts-schema-generator.ts (3)
636-642: Minor: union order changed, but alias type name would communicate intent better
| { type: string; env: undefined; url: string } | …is verbose and repeated.
Creating a named discriminated union (e.g.type StaticUrlDS = { … }) improves readability and keeps call-sites tidy.
1034-1040: Silent catch block still swallows parsing issuesThe added
// ignoreclarifies intent, but silently discarding an error when a malformed SQLite URL is passed can hide configuration bugs. At least log the error at debug level or return it in the thrownErrorbelow so the user knows why the URL was rejected.
1138-1147: Same non-null assertion pattern appears multiple timesThe non-null assertion on
reference!.$refTextis repeated for parameters and return types.
Extract a helper:const toTypeName = (t: { type?: string; reference?: { $refText: string } }) => t.type ?? t.reference?.$refText ?? 'Unknown';and replace all occurrences. DRY, safer, and ESLint-friendly.
Also applies to: 1170-1177, 1205-1210
packages/runtime/src/client/options.ts (1)
1-2: Disable rule locally instead of file-wide
ban-typesis turned off for the whole file, but onlyFunction/{}usages usually trigger it.
Wrap the offending type with a block suppression instead:// eslint-disable-next-line @typescript-eslint/ban-types type Whatever = Function;so that future code still benefits from the rule.
packages/runtime/src/client/contract.ts (1)
1-2: Same observation as inoptions.ts: scope theban-typesdisablePrefer a narrow
// eslint-disable-next-line …above the specific line(s) that needFunctionor{}rather than disabling the rule for the whole file.packages/cli/package.json (1)
24-27: Consider adding an autofix variant of the lint scriptRunning
eslint --ext ts srcis great for CI, but dev UX improves if we also provide a quick-fix command:"lint:fix": "eslint src --ext ts --fix"This mirrors the other packages’ scripts and shortens the feedback loop.
.eslintignore (1)
1-1:.eslintignoreentry is fine but consider future-proofing
**/dist/**covers most build artefacts, but if some packages output tobuild/orout/they will still be linted.
If that is intentional, keep as-is; otherwise consider adding common build folders to avoid noisy CI.packages/sdk/package.json (1)
6-10: Add.tsxand stricter exit flag to the lint script
- Source may eventually include React/JSX helpers – adding
tsxextension now avoids missing files.eslintexits with 0 by default when no files match; adding--ext ts,tsx --max-warnings 0makes CI stricter.- "lint": "eslint src --ext ts", + "lint": "eslint src --ext .ts,.tsx --max-warnings 0",packages/runtime/src/utils/clone.ts (1)
1-2: Avoid blanket@ts-expect-error; install proper typings foris-plain-object.Suppressing all type checks on the import hides potential issues and defeats the purpose of enabling strict lint rules. The package
is-plain-objecthas community typings (@types/is-plain-object). Adding that (or declaring your own module) allows you to drop the directive and keep full type-safety.-// @ts-expect-error -import { isPlainObject } from 'is-plain-object'; +import { isPlainObject } from 'is-plain-object';Then add
pnpm add -w -D @types/is-plain-objector create
types/is-plain-object.d.tswith a minimal declaration.packages/runtime/package.json (2)
10-11: Lint script uses default config but ignores TS/JS files outsidesrc/.If tests or build scripts live under other folders (e.g.
test/), they will be skipped. Consider:- "lint": "eslint src --ext ts", + "lint": "eslint \"src/**/*.ts\" \"test/**/*.ts\"",or rely on
.eslintignore.
87-90: Add typings for new dependencyis-plain-object.Same concern as in
clone.ts: include@types/is-plain-object(dev-dep) or declare a module so consumers of@zenstackhq/runtimedon’t need to suppress errors themselves..eslintrc.json (1)
1-22: Consider turningno-explicit-anytowarninstead of disabling.You already relaxed the rule for
Function; keepinganyas a warning provides a safety net without blocking builds.- "@typescript-eslint/no-explicit-any": "off", + "@typescript-eslint/no-explicit-any": "warn",Optional, but aligns with the objective of “improved consistency”.
packages/runtime/src/client/crud/dialects/sqlite.ts (1)
52-57: Generic parameter relaxed toany– confirm the trade-offChanging the third type argument of
SelectQueryBuilderfrom{}toanyeliminates the compile-time guarantee that only plain objects are passed around, effectively disabling useful structural checks on the selected row type.
If the original constraint was merely too strict, consider replacinganywith a more descriptive interface (e.g.Record<string, unknown>), or introducing a helper type alias so the loss of type-safety is explicit and documented.packages/runtime/src/client/crud/dialects/base.ts (1)
58-63: Same loosening ofSelectQueryBuildergeneric as in SQLite dialectWhile this mirrors other files, it propagates
anythroughout the filtering pipeline. If total flexibility is required, consider introducing an alias (type SQB = SelectQueryBuilder<any, any, unknownRow>;) so intent is explicit and can be tightened later.packages/runtime/src/client/crud/operations/base.ts (2)
149-153: Rename +constis 👍 but watch the shadowingPromoting the flattened filter to a
const _filteris a good immutability tweak.
Just make sure callers never confuse the originalfilterarg with_filter– the rename helps, but a short inline comment explaining why the extra pass is required would avoid future refactors accidentally dropping it.
465-493: Manual cursor pagination logic is brittleThe
filtersconstruction loopsorderByItems.lengthtimes, recomputing a
sub-query for each level. With large compound keys this generates anORwith
quadratic predicates that Postgres optimiser may struggle with.A lighter pattern is:
-- pseudo SQL WHERE (key1, key2, …) >= (SELECT key1, key2 … FROM t WHERE cursorFilter)Kysely allows tuple comparisons with
eb.refTuple.
Worth revisiting for performance.packages/runtime/src/client/crud/dialects/postgresql.ts (1)
261-266: Object-args union widened – OK but documentIncluding
SelectQueryBuilder<any,…>inobjArgsbroadens acceptable argument
types forjsonb_build_object. That’s powerful, yet future maintainers might
mix in builders not intended for JSON aggregation. A short JSDoc comment on
buildRelationObjectArgsdescribing the allowed elements would help contain
misuse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
.eslintignore(1 hunks).eslintrc.json(1 hunks).github/workflows/build-test.yml(1 hunks)package.json(1 hunks)packages/cli/package.json(1 hunks)packages/language/.eslintrc.json(0 hunks)packages/language/src/validators/expression-validator.ts(0 hunks)packages/language/src/validators/schema-validator.ts(1 hunks)packages/runtime/package.json(3 hunks)packages/runtime/src/client/client-impl.ts(1 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud-types.ts(1 hunks)packages/runtime/src/client/crud/dialects/base.ts(4 hunks)packages/runtime/src/client/crud/dialects/postgresql.ts(2 hunks)packages/runtime/src/client/crud/dialects/sqlite.ts(2 hunks)packages/runtime/src/client/crud/operations/base.ts(8 hunks)packages/runtime/src/client/executor/zenstack-driver.ts(1 hunks)packages/runtime/src/client/executor/zenstack-query-executor.ts(1 hunks)packages/runtime/src/client/helpers/schema-db-pusher.ts(1 hunks)packages/runtime/src/client/options.ts(1 hunks)packages/runtime/src/client/query-utils.ts(1 hunks)packages/runtime/src/plugins/policy/expression-transformer.ts(3 hunks)packages/runtime/src/plugins/policy/policy-handler.ts(1 hunks)packages/runtime/src/schema/schema.ts(1 hunks)packages/runtime/src/utils/clone.ts(1 hunks)packages/runtime/src/utils/is-plain-object.ts(0 hunks)packages/runtime/src/utils/type-utils.ts(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/ts-schema-generator.ts(6 hunks)
💤 Files with no reviewable changes (3)
- packages/language/src/validators/expression-validator.ts
- packages/language/.eslintrc.json
- packages/runtime/src/utils/is-plain-object.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/runtime/src/client/helpers/schema-db-pusher.ts (1)
packages/runtime/src/schema/schema.ts (1)
BuiltinType(84-92)
packages/runtime/src/client/crud/dialects/base.ts (4)
packages/runtime/src/client/crud-types.ts (1)
FindArgs(572-591)packages/runtime/src/schema/schema.ts (1)
GetModels(106-109)packages/runtime/src/client/query-utils.ts (1)
flattenCompoundUniqueFilters(312-337)packages/runtime/src/schema/expression.ts (2)
Expression(1-10)Expression(75-195)
packages/runtime/src/client/crud/operations/base.ts (1)
packages/runtime/src/client/query-utils.ts (2)
flattenCompoundUniqueFilters(312-337)isRelationField(140-147)
packages/runtime/src/client/crud/dialects/postgresql.ts (2)
packages/runtime/src/client/crud-types.ts (1)
FindArgs(572-591)packages/runtime/src/schema/schema.ts (1)
GetModels(106-109)
🪛 actionlint (1.7.7)
.github/workflows/build-test.yml
14-14: label "buildjet-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
36-36: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (10)
packages/language/src/validators/schema-validator.ts (1)
3-3: Cleaned up unused imports.The AST import now only brings in
isDataSourceand theModeltype, which are actually used invalidateDataSourcesand class signature. Removing dead imports improves readability and adheres to the new lint rules.packages/runtime/src/client/helpers/schema-db-pusher.ts (1)
201-201: Enforce immutability by usingconstforresult.
Switching fromlettoconstfor theresultvariable aligns with the repository’s immutability guidelines and ESLint rules.packages/runtime/src/client/executor/zenstack-query-executor.ts (1)
85-85: Enforce immutability foroldQueryNode.
Changing the declaration toconstensures the original query node reference is preserved unchanged, complying with the new linting standards.packages/runtime/src/utils/type-utils.ts (1)
44-45: Disableban-typesfor theSimplifyalias.
The added ESLint directive suppresses the ban on empty object types for this utility type while retaining its intended functionality.packages/runtime/src/plugins/policy/policy-handler.ts (1)
281-281: Enforce immutability forvalue.
Switching fromlettoconstfor thevaluevariable aligns with the project’s immutability and linting practices.packages/sdk/src/ts-schema-generator.ts (1)
455-462: Potentialundefineddereference via non-null assertion
field.type.reference!.$refTextis now asserted non-null. Although we short-circuit whenfield.type.typeis truthy, we still rely on an implicit invariant that “iftypeis falsy,referenceis non-null”.
Add an explicit guard or aninvariantto make that assumption guaranteed at runtime and easier to reason about:- field.type.type ?? field.type.reference!.$refText + field.type.type ?? + (invariant(field.type.reference, 'Expected reference type'), field.type.reference.$refText)packages/runtime/src/client/crud/dialects/sqlite.ts (1)
187-187:ArgsTypenow allows a fullSelectQueryBuilder– verify downstream calls
json_objectultimately expects scalar or JSON-producing expressions. A rawSelectQueryBuilderworks only if SQLite accepts it as a correlated sub-query returning a single column / single row. Please double-check every call site that pushes aSelectQueryBuilderintoobjArgs; otherwisejson_group_array(json_object(...))may raise “row value misused” errors at runtime.packages/runtime/src/client/crud/dialects/base.ts (1)
288-288: 👍 Switchedlet→constfor immutable collectionGood catch – declaring
conditionsasconstcommunicates immutability and satisfies ESLintprefer-const.packages/runtime/src/client/crud/operations/base.ts (2)
179-186:skipswitched toconst– safe given current usage
skipis now immutable whereastakecan still be rewritten.
Current code never mutatesskip, so the change is safe and conveys intent.
No action needed.
416-426: Nice micro-refactorStreaming all scalar fields via
reducedirectly intoselectFieldremoves the
tempfieldsaccumulator and clarifies intent.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/runtime/src/schema/auth.ts (1)
1-3: Guard against circular imports from.index re-export.
import type { GetModels, SchemaDef } from '.'relies onindex.tsre-exporting these entities.
Ifindex.tsalso re-imports from./auth, you will trigger a type-level circular reference that may slow TS or break in incremental builds.
Consider importing directly from'./schema'instead.packages/sdk/package.json (1)
9-10: Lint script: consider all TS/JS variants
eslint src --ext tsskips.mts,.cts, and.tsxfiles.
If you introduce any of those (or generated.d.ts) the CI lint job will silently ignore them.packages/runtime/src/schema/expression.ts (1)
67-73: Minor nit: use object shorthandTiny readability tweak – the property value matches the identifier name.
- return { - kind: 'member', - receiver: receiver, - members, - }; + return { + kind: 'member', + receiver, + members, + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
package.json(1 hunks)packages/cli/test/ts-schema-gen.test.ts(4 hunks)packages/language/package.json(0 hunks)packages/runtime/src/client/client-impl.ts(2 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud-types.ts(2 hunks)packages/runtime/src/client/crud/dialects/base.ts(4 hunks)packages/runtime/src/client/crud/dialects/postgresql.ts(3 hunks)packages/runtime/src/client/crud/dialects/sqlite.ts(3 hunks)packages/runtime/src/client/crud/operations/base.ts(10 hunks)packages/runtime/src/client/crud/validator.ts(1 hunks)packages/runtime/src/client/helpers/schema-db-pusher.ts(4 hunks)packages/runtime/src/client/options.ts(2 hunks)packages/runtime/src/client/query-builder.ts(1 hunks)packages/runtime/src/client/query-utils.ts(2 hunks)packages/runtime/src/client/result-processor.ts(1 hunks)packages/runtime/src/plugins/policy/expression-evaluator.ts(2 hunks)packages/runtime/src/plugins/policy/expression-transformer.ts(9 hunks)packages/runtime/src/plugins/policy/policy-handler.ts(4 hunks)packages/runtime/src/schema/auth.ts(1 hunks)packages/runtime/src/schema/expression.ts(3 hunks)packages/runtime/src/schema/index.ts(1 hunks)packages/runtime/test/client-api/default-values.test.ts(2 hunks)packages/runtime/test/client-api/name-mapping.test.ts(2 hunks)packages/runtime/test/test-schema.ts(11 hunks)packages/runtime/test/utils.ts(1 hunks)packages/sdk/package.json(3 hunks)packages/sdk/src/schema/expression.ts(1 hunks)packages/sdk/src/schema/index.ts(1 hunks)packages/sdk/src/schema/schema.ts(1 hunks)packages/sdk/src/ts-schema-generator.ts(16 hunks)packages/sdk/tsup.config.ts(1 hunks)packages/testtools/package.json(1 hunks)packages/testtools/src/schema.ts(2 hunks)samples/blog/zenstack/schema.ts(7 hunks)
💤 Files with no reviewable changes (1)
- packages/language/package.json
✅ Files skipped from review due to trivial changes (13)
- packages/runtime/test/utils.ts
- packages/runtime/src/client/query-builder.ts
- packages/cli/test/ts-schema-gen.test.ts
- packages/runtime/src/client/result-processor.ts
- packages/runtime/src/client/options.ts
- packages/runtime/test/client-api/name-mapping.test.ts
- packages/runtime/src/client/crud/validator.ts
- packages/sdk/src/schema/index.ts
- samples/blog/zenstack/schema.ts
- packages/runtime/src/client/query-utils.ts
- packages/runtime/src/plugins/policy/expression-evaluator.ts
- packages/runtime/test/test-schema.ts
- packages/sdk/src/schema/expression.ts
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/runtime/src/client/client-impl.ts
- packages/runtime/src/client/crud-types.ts
- packages/runtime/src/client/contract.ts
- packages/runtime/src/client/helpers/schema-db-pusher.ts
- packages/runtime/src/plugins/policy/policy-handler.ts
- package.json
- packages/runtime/src/plugins/policy/expression-transformer.ts
- packages/sdk/src/ts-schema-generator.ts
- packages/runtime/src/client/crud/dialects/postgresql.ts
- packages/runtime/src/client/crud/dialects/sqlite.ts
- packages/runtime/src/client/crud/dialects/base.ts
- packages/runtime/src/client/crud/operations/base.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/runtime/test/client-api/default-values.test.ts (1)
packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-136)
packages/runtime/src/schema/auth.ts (2)
packages/sdk/src/schema/schema.ts (2)
SchemaDef(11-18)GetModels(105-108)packages/runtime/src/client/crud-types.ts (1)
ModelResult(100-146)
packages/runtime/src/schema/expression.ts (1)
packages/sdk/src/schema/expression.ts (9)
Expression(1-10)ArrayExpression(17-20)CallExpression(46-50)NullExpression(56-58)ThisExpression(52-54)UnaryExpression(33-37)BinaryExpression(39-44)FieldExpression(22-25)MemberExpression(27-31)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (6)
packages/testtools/src/schema.ts (2)
2-2: Import path migration looks goodThe switch to
@zenstackhq/sdk/schemareflects the refactor done in the SDK package.
69-78: Path assumption may fail whendist/is not yet built
../../${pkg}/distpresumes each ZenStack package has already been built locally.
When running tests from a clean checkout (e.g. CI),distmay be missing, causingENOENT.Options:
- Build the packages before creating symlinks.
- Point the symlink to the package root instead of
dist, letting Node resolveexports.Either change will make the test helper more resilient.
packages/sdk/tsup.config.ts (1)
4-7:❓ Verification inconclusive
Double-check DTS + sub-path alignment
A new
schemaentry is great, but make sure the generated artifacts match the paths declared inpackage.json:
tsupusually emitsdist/schema.d.ts, notschema.d.cts.- Mismatches will break
typesresolution for consumers usingrequire.
🏁 Script executed:
#!/usr/bin/env bash # After running `pnpm -F @zenstackhq/sdk build`, ensure the files expected # in the export map actually exist. fd --extension d.ts dist fd --extension d.cts distLength of output: 54
Double-check DTS + sub-path alignment
File: packages/sdk/tsup.config.ts (lines 4–7):
entry: { index: 'src/index.ts', schema: 'src/schema/index.ts', },A new
schemaentry is great, but ensure your build output aligns with the export paths inpackage.json:
tsupshould emitdist/schema.d.ts(notschema.d.cts)- After building, confirm both
dist/index.d.tsanddist/schema.d.tsexist- Missing or misnamed
.d.tsfiles will break type resolution for consumers usingrequireTo verify locally:
pnpm -F @zenstackhq/sdk build ls -1 dist/*.d.tspackages/runtime/test/client-api/default-values.test.ts (1)
70-83:❌ Incorrect review comment
expect(...).toSatisfymay be undefined under VitestVitest doesn’t ship
toSatisfyby default.
If you haven’t already set upjest-extended(or the Vitest equivalent) in your test runner, this spec will throw at runtime.-import { describe, expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; +// import 'vitest-extended'; // or 'jest-extended', whichever you wire upOr switch to core matchers (e.g.
toBeTruthy) if you prefer to avoid an extra dependency.
To verify if
toSatisfyis available out-of-the-box or via a configured plugin, let’s check for its usage and anyjest-extended/vitest-extendedsetup in the repo:
🏁 Script executed:
#!/bin/bash set -e echo "1. Instances of 'toSatisfy' in tests:" rg -n "toSatisfy" || true echo echo "2. 'vitest-extended' in package.json dependencies/devDependencies:" grep -R "\"vitest-extended\"" -n package.json || true echo echo "3. 'jest-extended' in package.json dependencies/devDependencies:" grep -R "\"jest-extended\"" -n package.json || true echo echo "4. 'setupFiles' references in vitest config files:" grep -R "setupFiles" -n . echo echo "5. Direct imports of 'vitest-extended':" rg -n "import .*vitest-extended" || trueLength of output: 1976
toSatisfyIs Already Registered via Your Vitest Setup
Yourpackages/runtime/vitest.config.tsincludessetupFiles: [path.resolve(__dirname, './test/vitest-ext.ts')],and
packages/runtime/test/vitest-ext.tspulls in and registers the matchers (e.g. fromjest-extended), soexpect(...).toSatisfyis defined at runtime. No import changes are needed here.Likely an incorrect or invalid review comment.
packages/runtime/src/schema/index.ts (1)
1-4: LGTM – runtime correctly re-exports SDK schemaThe new re-export keeps runtime lean and avoids divergent type definitions.
Nice cleanup.packages/runtime/src/schema/expression.ts (1)
87-99: Confirm operator literals are included inBinaryOperator
'&&'and'||'are hard-coded here. Ensure theBinaryOperatorunion imported from the SDK includes both; otherwise TypeScript will flag these as incompatible.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/build-test.yml (2)
14-14:⚠️ Unknown runner label
Theruns-on: buildjet-8vcpu-ubuntu-2204label is not a default GitHub runner and was flagged previously. If this is a custom self-hosted runner, update youractionlint.yamlto include it; otherwise use a supported label (e.g.,ubuntu-latest-8-cores).🧰 Tools
🪛 actionlint (1.7.7)
14-14: label "buildjet-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
72-75: 🛠️ Refactor suggestionRemove redundant
pnpm install
Runningpnpm install --frozen-lockfiletwice wastes CI minutes. If you need to rebuild native or internal modules instead, replace it withpnpm -r rebuild.- # install again for internal dependencies - - name: Install internal dependencies - run: pnpm install --frozen-lockfile + - name: Rebuild native/internal modules + run: pnpm -r rebuild
🧹 Nitpick comments (1)
.github/workflows/build-test.yml (1)
16-29: Suggestion: Pin PostgreSQL service image
For deterministic CI, pin the PostgreSQL image to a specific tag (e.g.postgres:15.3) instead oflatest.services: postgres: - image: postgres + image: postgres:15.3 env: POSTGRES_PASSWORD: abc123 options: >-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/build-test.yml(1 hunks)package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-test.yml
14-14: label "buildjet-8vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-test (20.x)
🔇 Additional comments (12)
.github/workflows/build-test.yml (12)
1-8: Approve: Workflow trigger configuration
Thenameandon.pull_request.branchessettings correctly scope CI to PRs onmainanddev.
9-11: Approve: Permissions scope
Limitingcontents: readis sufficient for lint, build, and test steps.
12-13: Approve: Job definition
Thebuild-testjob is clearly declared underjobs.
30-33: Approve: Matrix strategy
Running against Node.js 20.x in a matrix is set up correctly.
35-37: Approve: Checkout step
Usingactions/checkout@v4is up-to-date and correct.
38-42: Approve: pnpm setup
pnpm/action-setup@v2with the specified version is valid.
43-47: Approve: Node.js setup
buildjet/setup-node@v3with pnpm caching is correctly configured.
49-62: Approve: pnpm cache configuration
Caching the pnpm store directory by lockfile hash will speed up CI.
63-65: Approve: Install dependencies
The firstpnpm install --frozen-lockfileis required for build and lint.
66-68: Approve: Build step
pnpm run buildcorrectly compiles the project before testing.
69-71: Approve: Lint step
pnpm run lintintegrates the new ESLint configs as intended.
76-78: Approve: Test step
pnpm run testwill execute your test suite after build and lint.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores