[PR #4216] [MERGED] fix(sqlite): honor selected-table reply budgets #6128

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4216
Author: @ogutdgn
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/sqlite-selected-table-reply-budget


📝 Commits (1)

  • b23d016 fix(sqlite): honor selected-table reply budgets

📊 Changes

2 files changed (+65 additions, -9 deletions)

View changed files

📝 src/sqlite/SqliteDatabaseInspection.ts (+18 -9)
📝 tests/boundary/sqlite/sqliteDatabaseInspection.test.ts (+47 -0)

📄 Description

Summary

Fixes #4184. Inspecting one small SQLite table succeeds even when unrelated catalog names collectively exceed the 16 MiB reply budget.

Problem and expected behavior

The schema reader charged every catalog name against an output budget before applying selected-table scope. A database containing one selected row and 60 unrelated 300 KiB table names therefore returned resource_constraint, although native SQLite could read the selected data within the documented limits. Selected-table replies should account for the selected table and its dependents.

Change and scope

Retain only the exact selected table's metadata and the first relevant unresolved virtual-table parent in selected scope. Collect virtual-parent information during the existing scan instead of deriving it from a full retained catalog. Whole-catalog name accounting, actual reply accounting, native limits, and shadow-table refusal remain enforced.

Contract and boundary impact

  • Semantic owner and earliest changed stage: readSchema in SqliteDatabaseInspection.ts, during table metadata enumeration.
  • Contribution scope fit and linked maintainer decision: ordinary correction within existing offline SQLite inspection; separate scope approval is not applicable.
  • CLI and MCP/tool-catalog contract: shared implementation correction; no schema or tool-catalog change. Valid selected requests can now succeed, and absent selections reach the existing invalid_request error.
  • Provider, bridge, target-format, or platform compatibility: built-in Node SQLite; no bridge or target-format changes. Local verification uses Node 24.18.0 on macOS arm64.
  • Evidence, artifact, provenance, or reconstruction contract: existing Evidence shape and complete-output semantics are preserved.
  • Target selection, authority, upstream behavior, and cleanup: exact selected-table lookup and existing read-only snapshot lifecycle remain unchanged.
  • Generated metadata, package, or installation impact: none; only the producer and its existing boundary test file change.

Expected effects:

  • 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.

The correction adds no new effects. The existing workflow uses a local worker and private temporary snapshots; it does not execute the inspected application's code or alter its source database.

Evidence and regression coverage

  • Tests added: a real SQLite regression with 60 unrelated 300 KiB table names checks selected schema/row success, the missing-table error, and retained full-catalog/inventory refusals.

  • Base reproduction: the regression failed before the fix at OutputBudget.retain; the public CLI likewise returned resource_constraint for the selected table. Native SQLite controls successfully read the selected data. The affected producer is byte-identical between the reproduced baseline 1b828dbb1 and the preparation base 663d07f02.

  • User-visible CLI output: before, --table selected returned resource_constraint; after, it returns complete selected-table schema and the expected single row. --table absent returns invalid_request; oversized database/inventory scopes still return resource_constraint.

  • Remaining proof gaps: the trigger is synthetic and does not establish real-world frequency. The specific large-catalog case is exercised through CLI and the production boundary, not directly through MCP. The suite includes other real CLI/MCP SQLite workflows. Other platforms and Node versions remain for CI.

  • 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

Executed on preparation base 663d07f02 plus this patch, with Node 24.18.0 / npm 11.16.0 on macOS arm64:

  • npm run test:sqlite — 10 files passed; 118 tests passed and 3 conditional skips (two runtime-inapplicable native-limit diagnostics and one Windows-only sharing case).
  • npm run check — passed, including architecture guards; existing lint warnings remain.
  • npm run check:fast — passed through the pre-push hook. The pre-commit formatter/linter also passed and left the tested file contents unchanged.
  • npm run verify:test-discovery — passed.
  • git diff --check — passed.
  • Six public CLI calls against the original 50/60-table fixtures — expected exit sequence 0,0,1,1,1,1; exact selected schema/rows match the successful baseline control, missing selections report the table error, full/inventory limits remain enforced, fixture hashes unchanged, stderr empty.

The earlier baseline reproduction and failing regression were run at 1b828dbb1; they were not rerun against the preparation base. Source identity above establishes that the affected producer has not changed. Full deterministic suite/coverage and other platform/runtime lanes were not run locally.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; corrects rejection of valid selected-table requests.
  • Real Hopper/Ghidra, browser, or OS coverage: no external analysis provider is involved. Tested on macOS arm64 with Node 24.18.0 and built-in SQLite 3.53.1; other platforms are unverified locally.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: existing input/native/heap/deadline and actual-output bounds remain. No new target execution, network access, or source-database mutation. No performance improvement is claimed; CPU time and token costs were not measured.

Author readiness

Human owner: @ogutdgn

AI-assisted implementation and validation; the human owner completes these declarations.

  • 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.

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/4216 **Author:** [@ogutdgn](https://github.com/ogutdgn) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/sqlite-selected-table-reply-budget` --- ### 📝 Commits (1) - [`b23d016`](https://github.com/morluto/rea/commit/b23d016a5b7f55cbbab0dacd6aa005ad745d34a0) fix(sqlite): honor selected-table reply budgets ### 📊 Changes **2 files changed** (+65 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/sqlite/SqliteDatabaseInspection.ts` (+18 -9) 📝 `tests/boundary/sqlite/sqliteDatabaseInspection.test.ts` (+47 -0) </details> ### 📄 Description ## Summary Fixes #4184. Inspecting one small SQLite table succeeds even when unrelated catalog names collectively exceed the 16 MiB reply budget. ## Problem and expected behavior The schema reader charged every catalog name against an output budget before applying selected-table scope. A database containing one selected row and 60 unrelated 300 KiB table names therefore returned `resource_constraint`, although native SQLite could read the selected data within the documented limits. Selected-table replies should account for the selected table and its dependents. ## Change and scope Retain only the exact selected table's metadata and the first relevant unresolved virtual-table parent in selected scope. Collect virtual-parent information during the existing scan instead of deriving it from a full retained catalog. Whole-catalog name accounting, actual reply accounting, native limits, and shadow-table refusal remain enforced. ## Contract and boundary impact - Semantic owner and earliest changed stage: `readSchema` in `SqliteDatabaseInspection.ts`, during table metadata enumeration. - Contribution scope fit and linked maintainer decision: ordinary correction within existing offline SQLite inspection; separate scope approval is not applicable. - CLI and MCP/tool-catalog contract: shared implementation correction; no schema or tool-catalog change. Valid selected requests can now succeed, and absent selections reach the existing `invalid_request` error. - Provider, bridge, target-format, or platform compatibility: built-in Node SQLite; no bridge or target-format changes. Local verification uses Node 24.18.0 on macOS arm64. - Evidence, artifact, provenance, or reconstruction contract: existing Evidence shape and complete-output semantics are preserved. - Target selection, authority, upstream behavior, and cleanup: exact selected-table lookup and existing read-only snapshot lifecycle remain unchanged. - Generated metadata, package, or installation impact: none; only the producer and its existing boundary test file change. Expected effects: - [x] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] 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. The correction adds no new effects. The existing workflow uses a local worker and private temporary snapshots; it does not execute the inspected application's code or alter its source database. ## Evidence and regression coverage - Tests added: a real SQLite regression with 60 unrelated 300 KiB table names checks selected schema/row success, the missing-table error, and retained full-catalog/inventory refusals. - Base reproduction: the regression failed before the fix at `OutputBudget.retain`; the public CLI likewise returned `resource_constraint` for the selected table. Native SQLite controls successfully read the selected data. The affected producer is byte-identical between the reproduced baseline `1b828dbb1` and the preparation base `663d07f02`. - User-visible CLI output: before, `--table selected` returned `resource_constraint`; after, it returns complete selected-table schema and the expected single row. `--table absent` returns `invalid_request`; oversized database/inventory scopes still return `resource_constraint`. - Remaining proof gaps: the trigger is synthetic and does not establish real-world frequency. The specific large-catalog case is exercised through CLI and the production boundary, not directly through MCP. The suite includes other real CLI/MCP SQLite workflows. Other platforms and Node versions remain for CI. - [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 Executed on preparation base `663d07f02` plus this patch, with Node 24.18.0 / npm 11.16.0 on macOS arm64: - `npm run test:sqlite` — 10 files passed; 118 tests passed and 3 conditional skips (two runtime-inapplicable native-limit diagnostics and one Windows-only sharing case). - `npm run check` — passed, including architecture guards; existing lint warnings remain. - `npm run check:fast` — passed through the pre-push hook. The pre-commit formatter/linter also passed and left the tested file contents unchanged. - `npm run verify:test-discovery` — passed. - `git diff --check` — passed. - Six public CLI calls against the original 50/60-table fixtures — expected exit sequence `0,0,1,1,1,1`; exact selected schema/rows match the successful baseline control, missing selections report the table error, full/inventory limits remain enforced, fixture hashes unchanged, stderr empty. The earlier baseline reproduction and failing regression were run at `1b828dbb1`; they were not rerun against the preparation base. Source identity above establishes that the affected producer has not changed. Full deterministic suite/coverage and other platform/runtime lanes were not run locally. ## Compatibility, safety, and release - Breaking changes or migration steps: none; corrects rejection of valid selected-table requests. - Real Hopper/Ghidra, browser, or OS coverage: no external analysis provider is involved. Tested on macOS arm64 with Node 24.18.0 and built-in SQLite 3.53.1; other platforms are unverified locally. - Package or release metadata impact: none. - Security, privacy, process, or containment review: existing input/native/heap/deadline and actual-output bounds remain. No new target execution, network access, or source-database mutation. No performance improvement is claimed; CPU time and token costs were not measured. ## Author readiness Human owner: @ogutdgn AI-assisted implementation and validation; the human owner completes these declarations. - [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. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:12:05 +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#6128
No description provided.