fix: align all packages with @objectstack/spec v2.0.1 protocol#358
fix: align all packages with @objectstack/spec v2.0.1 protocol#358
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Fix IsolationLevel to use snake_case per spec (read_uncommitted, read_committed, repeatable_read) and add missing 'snapshot' - Move mutationLog/changeTracking to RuntimeDriverCapabilities (not in spec's DriverCapabilitiesSchema which has additionalProperties: false) - Remove redundant 'location' and 'vector' from FieldType extension (now in spec's FieldType enum) - Add spec-aligned Driver methods: commit, rollback, upsert, findStream, getPoolStats, syncSchema, dropTable, explain - Mark commitTransaction/rollbackTransaction as deprecated in favor of spec-aligned commit/rollback - Update pg-wasm driver isolation levels to snake_case - Remove mutationLog/changeTracking from pg-wasm and sqlite-wasm drivers - Update pg-wasm tests for snake_case isolation levels Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Aligns the monorepo’s runtime/type surfaces with the @objectstack/spec v2.0.1 protocol, mainly by correcting protocol enums/unions and ensuring driver capability declarations don’t violate spec schema constraints.
Changes:
- Updated
IsolationLevelto snake_case and addedsnapshot; introducedRuntimeDriverCapabilitiesto keep sync-only flags out of spec-alignedDriverCapabilities. - Removed redundant runtime
FieldTypeunion members now covered by the spec. - Updated WASM drivers/tests to use the spec-aligned isolation level strings and removed non-spec capability flags from their declarations.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/foundation/types/src/field.ts | Removes redundant runtime FieldType members now included in the protocol enum. |
| packages/foundation/types/src/driver.ts | Aligns isolation levels; splits spec vs runtime-only capabilities; adds spec-surface driver methods. |
| packages/drivers/sqlite-wasm/src/index.ts | Removes non-spec capability flags from the driver’s supports declaration. |
| packages/drivers/pg-wasm/src/index.ts | Converts isolation level strings to snake_case; removes non-spec capability flags. |
| packages/drivers/pg-wasm/test/index.test.ts | Updates isolation-level assertions to match snake_case strings. |
Comments suppressed due to low confidence (2)
packages/drivers/pg-wasm/src/index.ts:9
- Unused import ObjectQLError.
import { Driver, DriverCapabilities, ObjectQLError } from '@objectql/types';
packages/drivers/sqlite-wasm/src/index.ts:9
- Unused import ObjectQLError.
import { Driver, DriverCapabilities, ObjectQLError } from '@objectql/types';
| * @param query - The structured QueryAST. | ||
| * @param options - Driver options. | ||
| * @returns AsyncIterable/ReadableStream of records. | ||
| */ | ||
| findStream?(objectName: string, query: any, options?: any): AsyncIterable<any> | any; | ||
|
|
There was a problem hiding this comment.
findStream is documented as returning an AsyncIterable/ReadableStream, but the current return type AsyncIterable<any> | any collapses to any (because of the | any) and loses all type information. Consider removing | any and modeling the intended stream types explicitly (and typing query as QueryAST if that's the required input).
| it('should support all isolation levels', () => { | ||
| expect(driver.supports.isolationLevels).toContain('read-uncommitted'); | ||
| expect(driver.supports.isolationLevels).toContain('read-committed'); | ||
| expect(driver.supports.isolationLevels).toContain('repeatable-read'); | ||
| expect(driver.supports.isolationLevels).toContain('read_uncommitted'); | ||
| expect(driver.supports.isolationLevels).toContain('read_committed'); | ||
| expect(driver.supports.isolationLevels).toContain('repeatable_read'); | ||
| expect(driver.supports.isolationLevels).toContain('serializable'); | ||
| }); |
There was a problem hiding this comment.
Test name says "support all isolation levels", but the assertions only cover 4 values and omit snapshot (which is now part of the IsolationLevel union). Either include snapshot in the driver capability + assertions, or rename the test to avoid claiming full coverage.
Audit of all packages against
@objectstack/specv2.0.1 revealed several protocol violations in types, drivers, and tests.IsolationLevel— wrong casing, missing valueSpec mandates snake_case (
read_uncommitted,read_committed,repeatable_read) and includessnapshot. We had kebab-case and nosnapshot.DriverCapabilities— extra properties violate specSpec sets
additionalProperties: false.mutationLogandchangeTrackingwere on the base interface, which would fail schema validation. Moved to a newRuntimeDriverCapabilitiesextension interface. Removed from pg-wasm and sqlite-wasm driver capability declarations.FieldType— redundant union members'location'and'vector'are now in the spec'sFieldTypeenum. Removed the duplicate entries from the runtime extension union.Driverinterface — missing spec methodsAdded methods required by
DriverInterfaceSchema:upsert,findStream,getPoolStats,syncSchema,dropTable,explain,commit,rollback. DeprecatedcommitTransaction/rollbackTransaction(removal in v5.0).Driver updates
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.