morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6327] [MERGED] fix(sqlite): report result-column capacity refusals as capacity #7287

New issue

Closed

opened 2026-11-01 21:46:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6327
Author: @DeryFerd
Created: 10/22/2026
Status: ✅ Merged
Merged: 10/23/2026
Merged by: @baron

Base: main ← Head: fix/6292-sqlite-result-column-capacity

📝 Commits (1)

7ab66ee fix(sqlite): report result-column capacity refusals as capacity

📊 Changes

7 files changed (+177 additions, -14 deletions)

View changed files

📝 docs/sqlite-databases.md (+6 -1)
📝 src/sqlite/SqliteDatabaseInspection.ts (+32 -1)
📝 src/sqlite/SqliteDatabaseLimits.ts (+11 -0)
📝 src/sqlite/SqliteDatabaseWorker.ts (+21 -12)
📝 src/sqlite/SqliteDatabaseWorkerExecution.ts (+22 -0)
📝 tests/boundary/sqlite/sqliteDatabaseInspection.test.ts (+39 -0)
📝 tests/boundary/sqlite/sqliteDatabaseProvider.test.ts (+46 -0)

📄 Description

fix #6292

Summary

Selecting more columns than SQLite can project in one result set currently makes inspect_sqlite_database report a healthy database as malformed. On this Node runtime the boundary is 2,000 result columns: 2,000 selected columns succeed, 2,001 fail inside SQLite with too many columns in result set, and the old classification turned that into invalid_request with an issue on ["path"] and invalid_format. The fix reports the refusal as a capacity resource constraint, so callers see the selected column count, the engine limit, and a recovery that asks them to split the projection instead of checking whether the file is a database.

Fixes #6292.

Problem and expected behavior

Smallest trigger: a readable SQLite file, one ordinary table, and a columns selection whose length exceeds the connection's result-column limit. Duplicate names count, so Array(2001).fill("email") is enough. SQLite raises SQLITE_ERROR (errcode 1) while compiling the projection.

Before this change, columnEvaluationFailure only attributed SQL errors to generated columns. When its probes succeeded it returned the original error, the worker mapped the unknown cause to the format reason, and the parent produced an AnalysisInputError on path with invalid_format. That message was misleading: the database passed integrity checks, inventory still worked, and smaller selections returned rows. Malformed files should keep their path/format classification, and they do; a file containing not a SQLite database still reports invalid_format with file is not a database.

After the change, the same over-limit call returns resource_constraint with details.resource: "capacity", retryable: false, and reported_limits naming both counts. The database is not described as malformed, and the recovery keeps the reading path open: fewer columns per call, split the facts across calls, or omit table for inventory.

Change and scope

src/sqlite/SqliteDatabaseLimits.ts: adds the column-limit inspection reason and SqliteColumnLimitFailure, which carries the selected count and the engine's column limit.
src/sqlite/SqliteDatabaseInspection.ts: recognizes the refusal from errcode 1 plus selection.columns.length > db.limits.column before generated-column probing. The reported limit comes from the connection that refused the statement, so it follows the runtime's configured value instead of a hard-coded 2,000.
src/sqlite/SqliteDatabaseWorker.ts and src/sqlite/SqliteDatabaseWorkerExecution.ts: extend the strict reply protocol with a column-limit shape and map it to AnalysisResourceConstraintError with resource: "capacity".
docs/sqlite-databases.md: documents that projected names, including repeats, count against the engine's result-column capacity and that such a refusal does not mark the database as malformed.
Tests cover the inspector boundary and the provider boundary.

The change does not raise SQLite's limit and does not add a new input maximum to columns. The engine limit is a runtime fact, and a client-side ceiling would duplicate it and drift on other runtimes or compile options. Virtual generated column attribution, malformed file handling, and row sampling stay as they were.

Contribution scope: a bug fix inside an existing read-only provider. No new execution, injection, mutation, or protection-recovery authority is involved, so the maintainer scope decision does not apply.

Contract and boundary impact

Semantic owner and earliest changed stage: SQLite provider inspection (inspectSqliteDatabaseSnapshot). The inspector owns the failure reason and the worker carries it unchanged.
Contribution scope fit and linked maintainer decision: existing read-only SQLite capability; not applicable to the new-provider gate.
CLI and MCP/tool-catalog contract: no tool schema or argument change. The same input now returns resource_constraint instead of invalid_request for this failure. Catalog generation is unaffected.
Provider, bridge, target-format, or platform compatibility: SQLite inspection only. The reported limit is read from DatabaseSync.limits.column, and the inspector already requires the native limits capability before querying.
Evidence, artifact, provenance, or reconstruction contract: the refused call returns no partial rows, and the selected file stays untouched. The acceptance check below compares the SHA-256 digest before and after.
Target selection, authority, upstream behavior, and cleanup details (if applicable): none. The worker still opens a private read-only copy and never writes the selected database or WAL.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none; no authored contracts, catalogs, or packaging files changed.

Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: tests/boundary/sqlite/sqliteDatabaseInspection.test.ts checks 2,000 columns succeed and 2,001 produce SqliteColumnLimitFailure with selectedColumns, engineColumnLimit, and SQLite's message. tests/boundary/sqlite/sqliteDatabaseProvider.test.ts checks the projected provider error is resource_constraint/capacity with no invalid_format issue.
Base reproduction or other evidence: on the affected revision, 2,001 duplicate names reach SQLite and throw ERR_SQLITE_ERROR, errcode: 1, too many columns in result set. The new inspection test fails on that revision for the missing column-limit classification and passes with the change.
User-visible CLI/MCP output (if applicable): I ran the production stdio MCP server with a temporary local harness, which I removed afterward; the committed regression coverage sits at the inspector and provider boundaries. The over-limit call returned:

{
  "code": "resource_constraint",
  "category": "resource_constraint",
  "message": "SQLite cannot return 2001 selected result columns; this connection permits at most 2000: too many columns in result set",
  "retryable": false,
  "details": {
    "operation": "inspect_sqlite_database",
    "resource": "capacity",
    "reported_limits": {
      "selected_result_columns": 2001,
      "engine_result_column_limit": 2000
    }
  }
}

The same harness confirmed that 2,000 columns return 2,000 columns and one row, that inventory still succeeds afterward, and that a malformed control file still returns invalid_request with path/invalid_format and file is not a database. The healthy file's digest was unchanged at the end.

Remaining proof gaps: the Windows provider lanes need the packaged native addon, which a source checkout does not build on this host, so I ran those checks against the installed artifact. CI on a clean runner is the reference run for those lanes.

For evidence-bearing changes:

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

npx vitest run tests/boundary/sqlite/sqliteDatabaseInspection.test.ts --coverage.enabled=false — 24 passed, 1 skipped; the new case failed before the change and passes after it.
npx vitest run tests/boundary/sqlite/sqliteDatabaseProvider.test.ts --coverage.enabled=false -t "reports a projection above" — 1 passed.
npx vitest run tests/boundary/sqlite --coverage.enabled=false — 97 passed, 1 failed. The failure is in sqliteDatabaseDescriptorCleanup.test.ts (a cancelled reply read is reported as ProviderAdapterError instead of AnalysisCancelledError). I repeated that test with the four changed source files restored to the base revision and it still fails, so it is pre-existing on this host and unrelated to this change.
npx oxlint — 0 errors; repo-wide warnings are unchanged and none point at the touched files.
npm run verify:module-boundaries — checked 2643 source files.
npm run compile — runtime compiled successfully.
npm run typecheck — the pinned native TypeScript compiler crashed with an out-of-memory error on this host; tsc -p tsconfig.build.json --noEmit passed and the compiled runtime executed the acceptance check above.

Compatibility, safety, and release

Breaking changes or migration steps: no schema or argument changes. Callers that matched only on invalid_request/invalid_format for this specific over-limit case will now see resource_constraint/capacity, which is the intended correction; successful calls are unchanged.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; this touches the SQLite provider only. Windows provider lanes run in CI.
Package or release metadata impact: none. No generated catalog, package metadata, or installation behavior changes.
Security, privacy, process, or containment review: no new authority, execution, network access, or persistent writes. Error text carries SQLite's message and the two counts, not file contents, and this error does not add the selected path.

Author readiness

Human owner: @DeryFerd

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

I left these declarations unchecked; the human owner should review the diff and evidence before ticking them.

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.

_{🔄 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/6327 **Author:** [@DeryFerd](https://github.com/DeryFerd) **Created:** 10/22/2026 **Status:** ✅ Merged **Merged:** 10/23/2026 **Merged by:** [@baron](https://github.com/baron) **Base:** `main` ← **Head:** `fix/6292-sqlite-result-column-capacity` --- ### 📝 Commits (1) - [`7ab66ee`](https://github.com/morluto/rea/commit/7ab66ee3af341b08ddadb1254cfbc0a6126e1970) fix(sqlite): report result-column capacity refusals as capacity ### 📊 Changes **7 files changed** (+177 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `docs/sqlite-databases.md` (+6 -1) 📝 `src/sqlite/SqliteDatabaseInspection.ts` (+32 -1) 📝 `src/sqlite/SqliteDatabaseLimits.ts` (+11 -0) 📝 `src/sqlite/SqliteDatabaseWorker.ts` (+21 -12) 📝 `src/sqlite/SqliteDatabaseWorkerExecution.ts` (+22 -0) 📝 `tests/boundary/sqlite/sqliteDatabaseInspection.test.ts` (+39 -0) 📝 `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` (+46 -0) </details> ### 📄 Description fix #6292 ## Summary Selecting more columns than SQLite can project in one result set currently makes `inspect_sqlite_database` report a healthy database as malformed. On this Node runtime the boundary is 2,000 result columns: 2,000 selected columns succeed, 2,001 fail inside SQLite with `too many columns in result set`, and the old classification turned that into `invalid_request` with an issue on `["path"]` and `invalid_format`. The fix reports the refusal as a capacity resource constraint, so callers see the selected column count, the engine limit, and a recovery that asks them to split the projection instead of checking whether the file is a database. Fixes #6292. ## Problem and expected behavior Smallest trigger: a readable SQLite file, one ordinary table, and a `columns` selection whose length exceeds the connection's result-column limit. Duplicate names count, so `Array(2001).fill("email")` is enough. SQLite raises `SQLITE_ERROR` (`errcode` 1) while compiling the projection. Before this change, `columnEvaluationFailure` only attributed SQL errors to generated columns. When its probes succeeded it returned the original error, the worker mapped the unknown cause to the `format` reason, and the parent produced an `AnalysisInputError` on `path` with `invalid_format`. That message was misleading: the database passed integrity checks, inventory still worked, and smaller selections returned rows. Malformed files should keep their path/format classification, and they do; a file containing `not a SQLite database` still reports `invalid_format` with `file is not a database`. After the change, the same over-limit call returns `resource_constraint` with `details.resource: "capacity"`, `retryable: false`, and `reported_limits` naming both counts. The database is not described as malformed, and the recovery keeps the reading path open: fewer columns per call, split the facts across calls, or omit `table` for inventory. ## Change and scope - `src/sqlite/SqliteDatabaseLimits.ts`: adds the `column-limit` inspection reason and `SqliteColumnLimitFailure`, which carries the selected count and the engine's column limit. - `src/sqlite/SqliteDatabaseInspection.ts`: recognizes the refusal from `errcode` 1 plus `selection.columns.length > db.limits.column` before generated-column probing. The reported limit comes from the connection that refused the statement, so it follows the runtime's configured value instead of a hard-coded 2,000. - `src/sqlite/SqliteDatabaseWorker.ts` and `src/sqlite/SqliteDatabaseWorkerExecution.ts`: extend the strict reply protocol with a `column-limit` shape and map it to `AnalysisResourceConstraintError` with `resource: "capacity"`. - `docs/sqlite-databases.md`: documents that projected names, including repeats, count against the engine's result-column capacity and that such a refusal does not mark the database as malformed. - Tests cover the inspector boundary and the provider boundary. The change does not raise SQLite's limit and does not add a new input maximum to `columns`. The engine limit is a runtime fact, and a client-side ceiling would duplicate it and drift on other runtimes or compile options. Virtual generated column attribution, malformed file handling, and row sampling stay as they were. Contribution scope: a bug fix inside an existing read-only provider. No new execution, injection, mutation, or protection-recovery authority is involved, so the maintainer scope decision does not apply. ## Contract and boundary impact - Semantic owner and earliest changed stage: SQLite provider inspection (`inspectSqliteDatabaseSnapshot`). The inspector owns the failure reason and the worker carries it unchanged. - Contribution scope fit and linked maintainer decision: existing read-only SQLite capability; not applicable to the new-provider gate. - CLI and MCP/tool-catalog contract: no tool schema or argument change. The same input now returns `resource_constraint` instead of `invalid_request` for this failure. Catalog generation is unaffected. - Provider, bridge, target-format, or platform compatibility: SQLite inspection only. The reported limit is read from `DatabaseSync.limits.column`, and the inspector already requires the native `limits` capability before querying. - Evidence, artifact, provenance, or reconstruction contract: the refused call returns no partial rows, and the selected file stays untouched. The acceptance check below compares the SHA-256 digest before and after. - Target selection, authority, upstream behavior, and cleanup details (if applicable): none. The worker still opens a private read-only copy and never writes the selected database or WAL. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none; no authored contracts, catalogs, or packaging files changed. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/sqlite/sqliteDatabaseInspection.test.ts` checks 2,000 columns succeed and 2,001 produce `SqliteColumnLimitFailure` with `selectedColumns`, `engineColumnLimit`, and SQLite's message. `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` checks the projected provider error is `resource_constraint`/`capacity` with no `invalid_format` issue. - Base reproduction or other evidence: on the affected revision, 2,001 duplicate names reach SQLite and throw `ERR_SQLITE_ERROR`, `errcode: 1`, `too many columns in result set`. The new inspection test fails on that revision for the missing `column-limit` classification and passes with the change. - User-visible CLI/MCP output (if applicable): I ran the production stdio MCP server with a temporary local harness, which I removed afterward; the committed regression coverage sits at the inspector and provider boundaries. The over-limit call returned: ```json { "code": "resource_constraint", "category": "resource_constraint", "message": "SQLite cannot return 2001 selected result columns; this connection permits at most 2000: too many columns in result set", "retryable": false, "details": { "operation": "inspect_sqlite_database", "resource": "capacity", "reported_limits": { "selected_result_columns": 2001, "engine_result_column_limit": 2000 } } } ``` The same harness confirmed that 2,000 columns return 2,000 columns and one row, that inventory still succeeds afterward, and that a malformed control file still returns `invalid_request` with `path`/`invalid_format` and `file is not a database`. The healthy file's digest was unchanged at the end. - Remaining proof gaps: the Windows provider lanes need the packaged native addon, which a source checkout does not build on this host, so I ran those checks against the installed artifact. CI on a clean runner is the reference run for those lanes. For evidence-bearing changes: - [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 - `npx vitest run tests/boundary/sqlite/sqliteDatabaseInspection.test.ts --coverage.enabled=false` — 24 passed, 1 skipped; the new case failed before the change and passes after it. - `npx vitest run tests/boundary/sqlite/sqliteDatabaseProvider.test.ts --coverage.enabled=false -t "reports a projection above"` — 1 passed. - `npx vitest run tests/boundary/sqlite --coverage.enabled=false` — 97 passed, 1 failed. The failure is in `sqliteDatabaseDescriptorCleanup.test.ts` (a cancelled reply read is reported as `ProviderAdapterError` instead of `AnalysisCancelledError`). I repeated that test with the four changed source files restored to the base revision and it still fails, so it is pre-existing on this host and unrelated to this change. - `npx oxlint` — 0 errors; repo-wide warnings are unchanged and none point at the touched files. - `npm run verify:module-boundaries` — checked 2643 source files. - `npm run compile` — runtime compiled successfully. - `npm run typecheck` — the pinned native TypeScript compiler crashed with an out-of-memory error on this host; `tsc -p tsconfig.build.json --noEmit` passed and the compiled runtime executed the acceptance check above. ## Compatibility, safety, and release - Breaking changes or migration steps: no schema or argument changes. Callers that matched only on `invalid_request`/`invalid_format` for this specific over-limit case will now see `resource_constraint`/`capacity`, which is the intended correction; successful calls are unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; this touches the SQLite provider only. Windows provider lanes run in CI. - Package or release metadata impact: none. No generated catalog, package metadata, or installation behavior changes. - Security, privacy, process, or containment review: no new authority, execution, network access, or persistent writes. Error text carries SQLite's message and the two counts, not file contents, and this error does not add the selected path. ## Author readiness Human owner: @DeryFerd - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. I left these declarations unchecked; the human owner should review the diff and evidence before ticking them. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:46:46 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-05 01:47:49 +00:00

[PR #7287] fix(process): tolerate missing observer roots #7808

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#7287

No description provided.

Rows
Columns