feat(editor): Add SQLite WASM integration with useDatabase and run-data workers (no-changelog)#1
Conversation
…-data` workers (no-changelog)
9233d87 to
2a9e0e7
Compare
|
/agentic_review |
Code Review by Qodo
1. Workspace importer removed
|
| const worker = new Worker(new URL('./run-data/worker.ts', import.meta.url), { | ||
| type: 'module', | ||
| }); |
There was a problem hiding this comment.
1. Worker url resolves wrong 🐞 Bug ✓ Correctness
AYT runDataWorker constructs a Worker URL that resolves to a non-existent path (run-data/run-data/worker.ts), so the worker will fail to load at runtime.
Agent Prompt
### Issue description
The run-data worker is instantiated with a URL that includes an extra `run-data/` path segment, which makes the resolved path incorrect and prevents the worker from loading.
### Issue Context
`instance.ts` and `worker.ts` are in the same folder (`src/workers/run-data/`). The current URL resolves relative to `instance.ts`.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/run-data/instance.ts[4-6]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export const actions = { | ||
| async initialize() { | ||
| const { promiser, dbId } = await useDatabase(databaseConfig); | ||
|
|
||
| state.promiser = promiser; | ||
| state.dbId = dbId; | ||
| }, | ||
| }; | ||
|
|
||
| export type RunDataWorker = typeof actions; |
There was a problem hiding this comment.
2. Worker methods not exposed 🐞 Bug ✓ Correctness
AYT the run-data worker never calls Comlink.expose(actions), so Comlink.wrap() clients cannot invoke initialize() and the worker will be unusable.
Agent Prompt
### Issue description
The run-data worker defines an `actions` API but does not expose it using Comlink, so the main thread cannot call `initialize()` via `Comlink.wrap()`.
### Issue Context
The repo’s existing Comlink worker (`typescript.worker.ts`) exposes its API using `Comlink.expose(...)`.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/run-data/worker.ts[1-22]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| data TEXT CHECK (json_valid(data)) NOT NULL, | ||
| workflow TEXT CHECK (json_valid(data)) NOT NULL, | ||
| created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP |
There was a problem hiding this comment.
3. Workflow json check wrong 🐞 Bug ✓ Correctness
AYT the workflow column CHECK constraint validates json_valid(data) instead of json_valid(workflow), so invalid workflow JSON will not be rejected by the schema.
Agent Prompt
### Issue description
The `workflow` column’s CHECK constraint validates the wrong column (`data`), so workflow JSON is not actually validated.
### Issue Context
This is in the schema used during initialization via `useDatabase()`.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/run-data/db.ts[9-15]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const cfg = await promiser('config-get', {}); | ||
| const openResponse = await promiser('open', { | ||
| filename: config.filename, | ||
| }); | ||
|
|
||
| if (openResponse.type === 'error') { | ||
| throw new Error(openResponse.result.message); | ||
| } | ||
|
|
||
| dbId = openResponse.result.dbId as string; | ||
|
|
||
| await promiser('exec', { | ||
| dbId, | ||
| sql: config.tables.executions.schema, | ||
| }); |
There was a problem hiding this comment.
4. Sqlite errors ignored 🐞 Bug ⛯ Reliability
AYT useDatabase does not validate config-get or exec responses for type === 'error', so initialization can return successfully even if the worker reports an error (including schema creation failure).
Agent Prompt
### Issue description
`useDatabase()` ignores possible `'error'` responses from `config-get` and `exec`, which can allow initialization to proceed with an unusable DB.
### Issue Context
The local `PromiserResponse` type is a union of success and error; callers must branch on `type`.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/database/useDatabase.ts[28-42]
- packages/frontend/editor-ui/src/workers/sqlite-wasm.d.ts[71-86]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| dbId = openResponse.result.dbId as string; | ||
|
|
||
| await promiser('exec', { | ||
| dbId, | ||
| sql: config.tables.executions.schema, |
There was a problem hiding this comment.
5. Dbid cast hides undefined 🐞 Bug ✓ Correctness
AYT useDatabase casts openResponse.result.dbId to string even though the declared type allows undefined, which can propagate an invalid dbId into later calls.
Agent Prompt
### Issue description
`dbId` is cast to `string` even though it may be `undefined` per the declared types, which can allow later SQL operations to run without a valid DB id.
### Issue Context
`DbId` is declared as `string | undefined` in `sqlite-wasm.d.ts` and used by `open` responses.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/database/useDatabase.ts[33-42]
- packages/frontend/editor-ui/src/workers/sqlite-wasm.d.ts[14-15]
- packages/frontend/editor-ui/src/workers/sqlite-wasm.d.ts[37-42]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Persistent review updated to latest commit 2a9e0e7 |
| const worker = new Worker(new URL('./run-data/worker.ts', import.meta.url), { | ||
| type: 'module', | ||
| }); |
There was a problem hiding this comment.
1. Bad worker url 🐞 Bug ✓ Correctness
The run-data worker is instantiated with a URL that resolves to a non-existent path (./run-data/worker.ts relative to run-data/instance.ts), so Worker creation will fail at runtime.
Agent Prompt
### Issue description
The run-data worker URL is constructed as `./run-data/worker.ts` from within `src/workers/run-data/instance.ts`, which resolves to `src/workers/run-data/run-data/worker.ts` and fails to load.
### Issue Context
`instance.ts` and `worker.ts` are in the same directory.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/run-data/instance.ts[4-6]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export const actions = { | ||
| async initialize() { | ||
| const { promiser, dbId } = await useDatabase(databaseConfig); | ||
|
|
||
| state.promiser = promiser; | ||
| state.dbId = dbId; | ||
| }, | ||
| }; | ||
|
|
||
| export type RunDataWorker = typeof actions; |
There was a problem hiding this comment.
2. Worker not comlink-exposed 🐞 Bug ✓ Correctness
The run-data worker exports actions but never calls Comlink.expose(), so Comlink.wrap() in the main thread will not be able to invoke initialize().
Agent Prompt
### Issue description
The run-data worker is wrapped via Comlink on the main thread but never exposes any API via `Comlink.expose()`, so RPC calls will fail.
### Issue Context
There is an established Comlink pattern in the repo (`typescript.worker.ts`) that calls `Comlink.expose(...)`.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/run-data/worker.ts[1-22]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| workflow_id INTEGER NOT NULL, | ||
| data TEXT CHECK (json_valid(data)) NOT NULL, | ||
| workflow TEXT CHECK (json_valid(data)) NOT NULL, | ||
| created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP | ||
| ); |
There was a problem hiding this comment.
3. Wrong json check column 🐞 Bug ✓ Correctness
The executions.workflow column CHECK constraint validates json_valid(data) instead of json_valid(workflow), allowing invalid JSON to be stored in workflow.
Agent Prompt
### Issue description
`workflow` column validation is incorrectly checking `data` instead of `workflow`, so invalid JSON can be persisted.
### Issue Context
Both `data` and `workflow` are intended to be JSON strings.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/run-data/db.ts[11-15]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const cfg = await promiser('config-get', {}); | ||
| const openResponse = await promiser('open', { | ||
| filename: config.filename, | ||
| }); | ||
|
|
||
| if (openResponse.type === 'error') { | ||
| throw new Error(openResponse.result.message); | ||
| } | ||
|
|
||
| dbId = openResponse.result.dbId as string; | ||
|
|
||
| await promiser('exec', { | ||
| dbId, | ||
| sql: config.tables.executions.schema, | ||
| }); |
There was a problem hiding this comment.
4. Unchecked promiser error results 🐞 Bug ⛯ Reliability
useDatabase() does not validate config-get or exec responses for type: 'error' and force-casts dbId to string even though it can be undefined, so initialization can proceed with invalid state.
Agent Prompt
### Issue description
`useDatabase()` treats promiser calls as always-successful (except `open`) even though the declared response type is a success/error union, and it force-casts possibly-undefined `dbId`.
### Issue Context
The type definition for the sqlite-wasm promiser explicitly models error responses (`type: 'error'`).
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/database/useDatabase.ts[28-42]
- packages/frontend/editor-ui/src/workers/sqlite-wasm.d.ts[14-16]
- packages/frontend/editor-ui/src/workers/sqlite-wasm.d.ts[84-91]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| import type { Worker } from 'node:worker_threads'; | ||
|
|
||
| declare module '@sqlite.org/sqlite-wasm' { | ||
| type OnreadyFunction = () => void; | ||
|
|
||
| export type Sqlite3Worker1PromiserConfig = { | ||
| onready?: OnreadyFunction; | ||
| worker?: Worker | (() => Worker); | ||
| generateMessageId?: (messageObject: unknown) => string; | ||
| debug?: (...args: any[]) => void; | ||
| onunhandled?: (event: MessageEvent) => void; | ||
| }; |
There was a problem hiding this comment.
5. Wrong worker type definition 🐞 Bug ✓ Correctness
The ambient @sqlite.org/sqlite-wasm typings import Worker from node:worker_threads, which is incompatible with browser Web Workers and will prevent correctly typing a DOM Worker for sqlite3Worker1Promiser() configuration.
Agent Prompt
### Issue description
The sqlite-wasm ambient typings are using Node's `worker_threads` Worker type, which is incompatible with the DOM Worker type used in the editor UI.
### Issue Context
editor-ui compiles with DOM libs and uses browser workers.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/sqlite-wasm.d.ts[1-12]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Persistent review updated to latest commit 2a9e0e7 |
Code Review by QodoSorry, something went wrongWe weren't able to complete the code review on our side. Please try againⓘ The new review experience is currently in Beta. Learn more |
|
Persistent review updated to latest commit 2a9e0e7 |
| await promiser('exec', { | ||
| dbId, | ||
| sql: config.tables.executions.schema, | ||
| }); |
There was a problem hiding this comment.
1. Hardcoded executions table init 🐞 Bug ⚙ Maintainability
useDatabase() always runs config.tables.executions.schema instead of initializing all configured tables, so adding any other table to DatabaseConfig.tables will be ignored and omitting executions will crash. This contradicts the generic Record<string, DatabaseTable> config shape.
Agent Prompt
### Issue description
Database initialization is hardcoded to the `executions` table despite `tables` being a generic record.
### Fix
Iterate over `Object.values(config.tables)` (or entries) and execute each table's `schema` with `promiser('exec', ...)`.
### Fix Focus Areas
- packages/frontend/editor-ui/src/workers/database/useDatabase.ts[9-12]
- packages/frontend/editor-ui/src/workers/database/useDatabase.ts[39-42]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4 similar comments
|
Persistent review updated to latest commit 2a9e0e7 |
| specifier: workspace:* | ||
| version: link:../typescript-config | ||
|
|
||
| packages/@n8n/scan-community-package: |
There was a problem hiding this comment.
1. Workspace importer removed 🐞 Bug ☼ Reliability
pnpm-lock.yaml deletes the importer entry for packages/@n8n/scan-community-package even though that workspace package and its dependencies still exist, leaving the lockfile inconsistent with the workspace manifests.
Agent Prompt
### Issue description
The `packages/@n8n/scan-community-package` importer stanza was removed from `pnpm-lock.yaml` while the package still exists in the workspace, making the lockfile stale/incomplete.
### Issue Context
`pnpm-workspace.yaml` includes `packages/@n8n/*`, and `packages/@n8n/scan-community-package/package.json` still declares dependencies.
### Fix Focus Areas
- pnpm-lock.yaml[1255-1272]
- pnpm-workspace.yaml[1-4]
- packages/@n8n/scan-community-package/package.json[1-16]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Persistent review updated to latest commit 2a9e0e7 |
1 similar comment
|
Persistent review updated to latest commit 2a9e0e7 |

Summary
This pull request introduces a new SQLite database integration to the frontend editor UI using the
@sqlite.org/sqlite-wasmpackage. The changes include setting up configuration and initialization logic for the database, and implementing the boilerplate code for a worker for managing run data storage. The code is currently unused, and will be introduced using a feature flag.Database Integration
useDatabaseutility insrc/workers/database/useDatabase.tsto initialize and manage the SQLite database using a Web Worker.executionstable insrc/workers/run-data/db.ts.src/workers/run-data/worker.tsand worker instantiation insrc/workers/run-data/instance.ts.Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/CAT-1273/add-database-code-and-worker-initialization