Skip to content

refactor(plugin): initialize workbench databases with Node SQLite - #1335

Open
mldangelo-oai wants to merge 27 commits into
dev/codex/sqlite-shared-migrationsfrom
dev/codex/sqlite-database-info
Open

mldangelo-oai wants to merge 27 commits into
dev/codex/sqlite-shared-migrationsfrom
dev/codex/sqlite-database-info

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The workbench stores scans and findings in SQLite. Initialize that database with Node's built-in SQLite so findings-store startup and database-info can work without Python. Other workbench operations continue to use Python and the same database.

Changes

Reuse the shared migration history and legacy repairs, preserving saved results, complete diagnostics, and transaction errors. Keep existing directory links and permissions; create missing storage privately, including Windows access controls. The SDK resolves the state directory once and uses its trusted Node selection when running under Bun.

Initialization captures the state directory, environment, and original executable-trust context. Relative Python paths retain their original location if the caller changes directories. Python discovery stays lazy until a legacy command runs, then caches the validated executable; a missing explicit interpreter does not prevent Node initialization.

Testing

Checks on Node 22.13 and Node 24 upgraded every released schema through version 42, including editable scan names, to the same current schema, then repeated the upgrade without changing migration history. Workflow fixtures retained embedded-NUL diagnostics and large integer values. A forced storage failure preserved the original error and pre-upgrade data. Concurrent-access cases kept an up-to-date reader usable during a write and retried an upgrade after a held lock.

SDK fixtures initialize two stores concurrently with Python configured to a missing executable, then verify the shared database path. Additional fixtures check that Python operations see the same stored findings and that Bun ignores a repository-local Node shim.

With dependencies and the host's plugin/native bundle built, repeat the database scenarios using node --experimental-strip-types --test tests/test_workbench_database.ts from plugins/codex-security/mcp-app.

Portable plugin checks, SDK build/types/format, build-plugin fixtures, and focused runtime/Deep Scan worker checks pass. The latest fix passes 35 findings-store tests, 93 focused runtime tests, and 11 executable-trust tests. Regressions cover state-location changes, changing directories before and after the first Python operation, an unavailable explicit interpreter, and rejection of an executable from the original repository. Full SDK-suite coverage is reported at the stack tip.

Risk and rollout

Depends on #1333. Node remains 22.13 or newer within supported major versions. Preserve Python database compatibility, foreign keys, concurrent readers, lock retries, path handling, and terminal-control escaping. No public flags are added. The recorded local tests do not establish behavior on every supported platform or packaging path.

Related PRs

Merge order: #1333 → #1335 → #1341 → #1342 → #1343 → #1397 → #1408

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T10:39:19.554571Z c17cd09 Manual request
🔒 Security Review ✅ Completed 2026-10-07T10:38:28.287059Z c17cd09 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@mldangelo-oai mldangelo-oai changed the title refactor(plugin): initialize workbench databases with Node SQLite refactor(plugin): initialize workbench databases with Node SQLite [2/7; base: #1333; above: #1341, #1342, #1343, #1397, #1408] Oct 7, 2026
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

@zcrab-oai zcrab-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6a707aa. I found the following issue and am leaving this PR unapproved.

[P2] Keep initialization and later operations on the same database — sdk/typescript/src/server/sqlite-store.ts:27

Calling runWorkbench directly here leaves this.options unset. If a caller changes CODEX_SECURITY_STATE_DIR (including through process.env), or changes cwd with a relative state directory, after initialize() but before the first list/insert, that operation opens a different database. Previously initialize() went through this.run() and cached the normalized absolute location. Cache that location during initialization and reuse it for subsequent operations, while keeping Python resolution lazy.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 6a707aac6f

ℹ️ 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".

@mldangelo-oai mldangelo-oai changed the title refactor(plugin): initialize workbench databases with Node SQLite [2/7; base: #1333; above: #1341, #1342, #1343, #1397, #1408] refactor(plugin): initialize workbench databases with Node SQLite Oct 7, 2026
@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

Fixed the database-location issue from this review. Initialization now captures the state directory and environment, so later operations keep the same database after directory or environment changes.

The follow-up changes also preserve relative Python paths and the original executable-trust context. Interpreter validation remains lazy, and Bun’s Node selection uses the same captured trust context. Regression tests cover directory changes before and after the first Python operation, missing interpreters, and repository-local executable rejection.

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c17cd094e3

ℹ️ 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".

Comment thread sdk/typescript/src/runtime.ts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants