[PR #2774] [MERGED] fix(sqlite): attribute unknown column selections to columns #5383

Closed
opened 2026-10-31 01:06:12 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2774
Author: @yasshmittal
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/sqlite-column-selection-path


📝 Commits (1)

  • b543e14 fix(sqlite): attribute unknown column selections to columns

📊 Changes

4 files changed (+56 additions, -2 deletions)

View changed files

📝 src/sqlite/SqliteDatabaseInspection.ts (+1 -1)
📝 src/sqlite/SqliteDatabaseLimits.ts (+1 -0)
📝 src/sqlite/SqliteDatabaseWorkerExecution.ts (+10 -1)
📝 tests/boundary/sqlite/sqliteDatabaseProvider.test.ts (+44 -0)

📄 Description

Summary

Fixes #2613.

Requesting a column that does not exist in an existing table returned an input issue on ["table"]. The message named the missing column correctly, but a client that repairs the argument in issues[].path would change the wrong one.

Problem and expected behavior

SqliteDatabaseInspection raised every selection rejection with the worker reason selection, and SqliteDatabaseWorkerExecution mapped every selection reply to table. The column check therefore inherited the table attribution. An unknown column should identify columns; an unknown table should still identify table.

Change and scope

Column rejections now use their own worker reason, column-selection. The reply projection maps each input reason through one explicit table (format/unsupported → path, selection → table, column-selection → columns), so the TypeScript checker requires every reason to name its argument. Table, view, virtual-table and schema-scope rejections keep table. The message text is unchanged.

Not included: the index of the rejected element in the path (["columns", 1]). The issue marks it optional, and adding it would widen the worker reply protocol.

Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies.

Contract and boundary impact

  • Semantic owner and earliest changed stage: the inspection failure reason in src/sqlite/SqliteDatabaseInspection.ts, then the reply projection in SqliteDatabaseWorkerExecution.ts.

  • Contribution scope fit and linked maintainer decision: not applicable (bug fix).

  • CLI and MCP/tool-catalog contract: schema unchanged; issues[0].path for an unknown column changes from ["table"] to ["columns"].

  • Provider, bridge, target-format, or platform compatibility: the worker reply enum gains an internal reason; the worker and its parent ship together.

  • Evidence, artifact, provenance, or reconstruction contract: unchanged.

  • Target selection, authority, upstream behavior, and cleanup details: unchanged; read-only snapshot inspection.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/sqlite/sqliteDatabaseProvider.test.ts checks the unknown-column issue (path, reason, message), keeps the unknown-table control on table, and confirms a valid columns: ["id"] selection still returns rows.

  • Base reproduction or other evidence: on the base revision the unknown-column request returns {"path":["table"],"reason":"invalid_format",…}, as in the issue.

  • User-visible CLI/MCP output:

    {"path":["columns"],"reason":"invalid_format","message":"Selected column does not exist with that exact name in table records: nope"}
    
  • Remaining proof gaps: the committed boundary test needs the private runtime root, which on Windows requires the Linux/MinGW-built native addon, so it did not run on this host. See Validation.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Host: Windows 11, Node.js 24.21.0. The native Windows addon (native/windows/build) can only be built on Linux with MinGW, so the SQLite provider boundary file cannot complete on this host. That includes 17 existing tests that fail identically on the base revision.

  • Ran an end-to-end check through SqliteDatabaseProvider and the real compiled dist/sqlite/SqliteDatabaseWorker.js, with a plain temporary root and plain child-process launcher (not committed): unknown column → ["columns"], unknown table → ["table"], valid ["id"] selection → 1 row.
  • npm run check: passed.
  • npm run verify:test-discovery: passed.
  • Browser, Ghidra and SQLite test directories run with and without the change: the only differences are the new boundary tests, which hit the same Windows native-addon limitation.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Clients matching only reason are unaffected; clients reading path now receive the correct argument.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows only; the committed boundary test runs on Linux/macOS CI.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new I/O, process, or network behavior.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2774 **Author:** [@yasshmittal](https://github.com/yasshmittal) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/sqlite-column-selection-path` --- ### 📝 Commits (1) - [`b543e14`](https://github.com/morluto/rea/commit/b543e14ef02ffd8564876ee352880118b008e13a) fix(sqlite): attribute unknown column selections to columns ### 📊 Changes **4 files changed** (+56 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/sqlite/SqliteDatabaseInspection.ts` (+1 -1) 📝 `src/sqlite/SqliteDatabaseLimits.ts` (+1 -0) 📝 `src/sqlite/SqliteDatabaseWorkerExecution.ts` (+10 -1) 📝 `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` (+44 -0) </details> ### 📄 Description ## Summary Fixes #2613. Requesting a column that does not exist in an existing table returned an input issue on `["table"]`. The message named the missing column correctly, but a client that repairs the argument in `issues[].path` would change the wrong one. ## Problem and expected behavior `SqliteDatabaseInspection` raised every selection rejection with the worker reason `selection`, and `SqliteDatabaseWorkerExecution` mapped every `selection` reply to `table`. The column check therefore inherited the table attribution. An unknown column should identify `columns`; an unknown table should still identify `table`. ## Change and scope Column rejections now use their own worker reason, `column-selection`. The reply projection maps each input reason through one explicit table (`format`/`unsupported` → `path`, `selection` → `table`, `column-selection` → `columns`), so the TypeScript checker requires every reason to name its argument. Table, view, virtual-table and schema-scope rejections keep `table`. The message text is unchanged. Not included: the index of the rejected element in the path (`["columns", 1]`). The issue marks it optional, and adding it would widen the worker reply protocol. Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies. ## Contract and boundary impact - Semantic owner and earliest changed stage: the inspection failure reason in `src/sqlite/SqliteDatabaseInspection.ts`, then the reply projection in `SqliteDatabaseWorkerExecution.ts`. - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: schema unchanged; `issues[0].path` for an unknown column changes from `["table"]` to `["columns"]`. - Provider, bridge, target-format, or platform compatibility: the worker reply enum gains an internal reason; the worker and its parent ship together. - Evidence, artifact, provenance, or reconstruction contract: unchanged. - Target selection, authority, upstream behavior, and cleanup details: unchanged; read-only snapshot inspection. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` checks the unknown-column issue (path, reason, message), keeps the unknown-table control on `table`, and confirms a valid `columns: ["id"]` selection still returns rows. - Base reproduction or other evidence: on the base revision the unknown-column request returns `{"path":["table"],"reason":"invalid_format",…}`, as in the issue. - User-visible CLI/MCP output: ```json {"path":["columns"],"reason":"invalid_format","message":"Selected column does not exist with that exact name in table records: nope"} ``` - Remaining proof gaps: the committed boundary test needs the private runtime root, which on Windows requires the Linux/MinGW-built native addon, so it did not run on this host. See Validation. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Host: Windows 11, Node.js 24.21.0. The native Windows addon (`native/windows/build`) can only be built on Linux with MinGW, so the SQLite provider boundary file cannot complete on this host. That includes 17 existing tests that fail identically on the base revision. - Ran an end-to-end check through `SqliteDatabaseProvider` and the real compiled `dist/sqlite/SqliteDatabaseWorker.js`, with a plain temporary root and plain child-process launcher (not committed): unknown column → `["columns"]`, unknown table → `["table"]`, valid `["id"]` selection → 1 row. - `npm run check`: passed. - `npm run verify:test-discovery`: passed. - Browser, Ghidra and SQLite test directories run with and without the change: the only differences are the new boundary tests, which hit the same Windows native-addon limitation. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Clients matching only `reason` are unaffected; clients reading `path` now receive the correct argument. - Real Hopper/Ghidra, browser, or OS coverage: Windows only; the committed boundary test runs on Linux/macOS CI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new I/O, process, or network behavior. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:06:12 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#5383
No description provided.