Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0662aeda99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const { changes, lastInsertRowid } = stmt.run(...sqliteParameters); | ||
| return Promise.resolve({ | ||
| insertId: BigInt(lastInsertRowid), | ||
| numAffectedRows: BigInt(changes), |
There was a problem hiding this comment.
Avoid returning stale insertId from stmt.run results
node:sqlite documents lastInsertRowid as the most recent insert rowid on the connection, not the rowid for the statement that just ran. Because this dialect always maps stmt.run(...).lastInsertRowid into insertId, any non-insert statement (or insert paths that do not actually insert, such as conflict-ignore flows) can surface a stale insertId from an earlier insert, which makes Kysely insert results incorrect for callers that depend on insertId semantics.
Useful? React with 👍 / 👎.
|
Addressed Codex review on stale |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: Add native sqlite Kysely dialect This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 838a59cdfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| numAffectedRows: BigInt(changes), | ||
| rows: [], | ||
| }; | ||
| if (isInsertStatement(sql) && changes > 0) { |
There was a problem hiding this comment.
Preserve insertId for INSERT forms beyond plain INSERT
The insertId assignment is gated by isInsertStatement(sql), which only matches SQL text starting with insert. This drops insertId for valid insert-producing statements like WITH ... INSERT ... (CTE inserts) and REPLACE INTO ..., even when stmt.run() reports changes > 0 and a valid lastInsertRowid. In those cases Kysely callers get an incomplete InsertResult despite a successful write, which can break flows that depend on the returned id.
Useful? React with 👍 / 👎.
|
Landed via squash merge onto
|
Summary
node:sqliteINSERT ... RETURNING>=22.16.0soStatementSync.columns()is always availableReal behavior proof
INSERT ... RETURNINGthrough OpenClaw's owned nativenode:sqlitedialect without depending on a third-party adapter.v25.9.0, nativenode:sqlite, in-memory SQLite database.pnpm exec tsx --no-warnings --eval '...'that importedNodeSqliteKyselyDialect, created a table, inserted/select rows, and exercised raw SQL.{"node":"25.9.0","selected":{"id":1,"name":"Ada"},"returning":{"id":2,"name":"Grace"},"raw":{"name":"Grace"}}INSERT ... RETURNING, and returned the same row through raw SQL.Verification
pnpm test src/infra/runtime-guard.test.ts src/daemon/runtime-paths.test.ts src/daemon/service-env.test.ts src/cli/update-cli.test.ts src/infra/update-check.test.ts src/infra/kysely-node-sqlite.test.ts test/scripts/root-dependency-ownership-audit.test.tspnpm test extensions/memory-corepnpm tsgo:corepnpm tsgo:extensionspnpm exec oxlint src/infra/kysely-node-sqlite.ts src/infra/kysely-node-sqlite.test.ts src/infra/runtime-guard.ts src/infra/runtime-guard.test.ts src/daemon/runtime-paths.ts src/daemon/runtime-paths.test.ts src/daemon/service-env.test.ts src/cli/update-cli/update-command.ts src/cli/update-cli.test.ts src/infra/update-check.test.ts src/commands/doctor-gateway-services.ts src/daemon/service-audit.ts src/daemon/program-args.ts extensions/memory-core/src/memory/provider-adapters.tspnpm deadcode:dependenciespnpm deadcode:unused-filesgit diff --check origin/main...HEAD