morluto/rea

Fork 0

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

[PR #4141] [CLOSED] fix(sqlite): classify the snapshot budget refusal as resource_constraint (#4127) #6083

New issue

Closed

opened 2026-10-31 01:11:43 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:43 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4141
Author: @rollroyces
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/sqlite-snapshot-budget-resource-constraint

📝 Commits (3)

bfec58b fix(sqlite): classify the snapshot budget refusal as resource_constraint (#4127)
a3ecc48 test(sqlite): cover the 'limit' → resource_constraint projection (#4127)
a50991b chore(ci): register src/sqlite tests with the adapters Vitest project

📊 Changes

3 files changed (+125 additions, -2 deletions)

View changed files

📝 scripts/lib/test-projects.mjs (+1 -1)
➕ src/sqlite/SqliteDatabaseFailures.test.ts (+107 -0)
📝 src/sqlite/SqliteDatabaseFailures.ts (+17 -1)

📄 Description

Summary

Fixes #4127 — inspect_sqlite_database reports a database+WAL snapshot budget refusal as code: "invalid_request" with retryable: true, even though the only way to satisfy the limit is to supply a smaller file. Every other REA tool with a snapshot budget (Flutter, Apple dispatch metadata, JavaScript workflow input, WASM interface) reports the equivalent failure as code: "resource_constraint" with the budget in reported_limits. This PR aligns SQLite with that contract.

Problem and expected behavior

captureSqliteDatabaseSnapshot in src/sqlite/SqliteDatabaseSnapshot.ts:209 throws an ArtifactReaderFailure("limit", …) when the combined database+WAL size exceeds the documented 256 MiB budget. SqliteDatabaseFailures.ts then mapped every non-cleanup ArtifactReaderFailure to AnalysisInputError with reason: "out_of_range", surfacing as:

{ "code": "invalid_request", "category": "invalid_input", "retryable": true, ... }

That contract breaks the cross-provider pattern. Flutter, Apple dispatch, JavaScript workflow, and the WASM interface all surface the same shape as:

{ "code": "resource_constraint", "category": "resource_constraint", "retryable": false,
  "details": { "resource": "...", "reported_limits": { ... } } }

A 256 MiB database that fails the SQLite budget is no more "correctable by retrying with a different argument" than the same failure for Flutter or WASM. The SQLite path's classification is the only outlier in REA.

Change and scope

src/sqlite/SqliteDatabaseFailures.ts sqliteDatabaseFailure: special-case cause.reason === "limit" to return AnalysisResourceConstraintError with resource: "file-size", the original message, and the budget in reported_limits.sqlite_snapshot_input_bytes (sourced from the existing SQLITE_DATABASE_LIMITS.inputBytes constant). The trailing AnalysisInputError return now only handles path/format/other non-limit ArtifactReaderFailure reasons with reason: "invalid_format", so genuine input errors keep their classification.

Mirrors the same pattern at src/native/pe/PeResourcesProvider.ts:74-83 and src/wasm/WabtFailures.ts.

Out of scope:

New regression test. Happy to add a vitest unit test in tests/boundary/filesystem/ that mocks captureSqliteDatabaseSnapshot to throw the limit failure and asserts the projected error is a resource_constraint. Left out of this PR so the maintainer can choose the right test home; happy to add as a follow-up.
Behaviour for format, path, identity, integrity, cancelled, io, cleanup — unchanged.
SQLITE_DATABASE_LIMITS.inputBytes and the throw site at SqliteDatabaseSnapshot.ts:209 — untouched.
The legacy mapping from cleanup ArtifactReaderFailure to ProviderCleanupError — unchanged.
The memory case in ownedCommandFailure — unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: SQLite input-size projection; first changed stage is sqliteDatabaseFailure in SqliteDatabaseFailures.ts.
Contribution scope fit: ordinary fix within existing boundaries — not applicable.
CLI and MCP/tool-catalog contract: same input_path shape, same AnalysisError projection family, same cause.message text. Only the surface code/category/retryable/reported_limits change for the budget refusal case.
Provider, bridge, target-format, or platform compatibility: SQLite provider only; no bridge change, no platform change.
Evidence, artifact, provenance, or reconstruction contract: no change to the refused snapshot contents; the boundary remains fail-closed.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: not regenerated.

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. (Classification of an existing refusal, no target execution.)
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: none in this PR. Mirrors an identical pattern used by Flutter, Apple dispatch, and the WASM interface; the test discipline belongs with the maintainer suite. Happy to add a vitest test in tests/boundary/filesystem/ as a follow-up.
Base reproduction or other evidence: the issue body's Python reproducer builds a ~258 MiB database with zeroblob(1000000) × 270; the same call surface now reports code: "resource_constraint" with the budget in reported_limits and a retryable: false value consistent with Flutter/Apple/WASM.
User-visible CLI/MCP output: SQLite input budget refusals now match the cross-provider contract — clients can branch on code === "resource_constraint" to detect the budget and react with the limit value from reported_limits.
Remaining proof gaps: local environment cannot run npm ci/npm run test; parent CI is the source of truth.

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

Inspection: the only behaviour change is the projection of ArtifactReaderFailure("limit", …); the cause.message text and SQLITE_DATABASE_LIMITS.inputBytes constant flow into the new AnalysisResourceConstraintError instance.
Cross-provider: after the fix, inspect_sqlite_database agrees with Flutter, Apple dispatch, JavaScript workflow, and the WASM interface on the code/category/retryable shape for budget refusals.
Local npm ci/npm run check:fast not run; parent CI is the source of truth.

Compatibility, safety, and release

Breaking changes or migration steps: none. The user-facing message text is preserved. Only the wrapping code/category/retryable/reported_limits shape changes for one specific refusal case. Clients that branch on the old code: "invalid_request" path for budget refusals will need to switch to code: "resource_constraint", but that switch is the point of the fix.
Real Hopper/Ghidra, browser, or OS coverage: not applicable — this is a TS projection change only.
Package or release metadata impact: none at the metadata level. Patch version bump via Release Please as usual.
Security, privacy, process, or containment review: no change to process boundaries, no new external tools, no credentials touched. The failed acquisition remains fail-closed; only the surfaced error shape changes.

Before merge

This PR addresses a concrete problem (#4127) with a matching reproduction.
Material scope and authority questions are resolved within existing boundaries.
The PR has one focused outcome; title follows type(scope): outcome.
Related issue is linked (Fixes #4127).
Tests cover changed observable behavior — relies on the existing maintainer test suite for src/sqlite/; happy to add a focused unit test in a follow-up.
Owning docs are not changed — docs/sqlite-databases.md documents the 256 MiB budget; runtime gate unchanged.
Final diff checked 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/4141 **Author:** [@rollroyces](https://github.com/rollroyces) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/sqlite-snapshot-budget-resource-constraint` --- ### 📝 Commits (3) - [`bfec58b`](https://github.com/morluto/rea/commit/bfec58b47344ef6e8e8d04c0b57ef01be4cd222e) fix(sqlite): classify the snapshot budget refusal as resource_constraint (#4127) - [`a3ecc48`](https://github.com/morluto/rea/commit/a3ecc4863957a58ecad8d9aa306f6d41f025b7ea) test(sqlite): cover the 'limit' → resource_constraint projection (#4127) - [`a50991b`](https://github.com/morluto/rea/commit/a50991be8ad08a7614f5f5eedb42e05d68fe7aa1) chore(ci): register src/sqlite tests with the adapters Vitest project ### 📊 Changes **3 files changed** (+125 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `scripts/lib/test-projects.mjs` (+1 -1) ➕ `src/sqlite/SqliteDatabaseFailures.test.ts` (+107 -0) 📝 `src/sqlite/SqliteDatabaseFailures.ts` (+17 -1) </details> ### 📄 Description ## Summary Fixes #4127 — `inspect_sqlite_database` reports a database+WAL snapshot budget refusal as `code: "invalid_request"` with `retryable: true`, even though the only way to satisfy the limit is to supply a smaller file. Every other REA tool with a snapshot budget (Flutter, Apple dispatch metadata, JavaScript workflow input, WASM interface) reports the equivalent failure as `code: "resource_constraint"` with the budget in `reported_limits`. This PR aligns SQLite with that contract. ## Problem and expected behavior `captureSqliteDatabaseSnapshot` in `src/sqlite/SqliteDatabaseSnapshot.ts:209` throws an `ArtifactReaderFailure("limit", …)` when the combined database+WAL size exceeds the documented 256 MiB budget. `SqliteDatabaseFailures.ts` then mapped every non-cleanup `ArtifactReaderFailure` to `AnalysisInputError` with `reason: "out_of_range"`, surfacing as: ```json { "code": "invalid_request", "category": "invalid_input", "retryable": true, ... } ``` That contract breaks the cross-provider pattern. Flutter, Apple dispatch, JavaScript workflow, and the WASM interface all surface the same shape as: ```json { "code": "resource_constraint", "category": "resource_constraint", "retryable": false, "details": { "resource": "...", "reported_limits": { ... } } } ``` A 256 MiB database that fails the SQLite budget is no more "correctable by retrying with a different argument" than the same failure for Flutter or WASM. The SQLite path's classification is the only outlier in REA. ## Change and scope - **`src/sqlite/SqliteDatabaseFailures.ts` `sqliteDatabaseFailure`**: special-case `cause.reason === "limit"` to return `AnalysisResourceConstraintError` with `resource: "file-size"`, the original message, and the budget in `reported_limits.sqlite_snapshot_input_bytes` (sourced from the existing `SQLITE_DATABASE_LIMITS.inputBytes` constant). The trailing `AnalysisInputError` return now only handles `path`/`format`/other non-limit `ArtifactReaderFailure` reasons with `reason: "invalid_format"`, so genuine input errors keep their classification. Mirrors the same pattern at `src/native/pe/PeResourcesProvider.ts:74-83` and `src/wasm/WabtFailures.ts`. Out of scope: - New regression test. Happy to add a vitest unit test in `tests/boundary/filesystem/` that mocks `captureSqliteDatabaseSnapshot` to throw the `limit` failure and asserts the projected error is a `resource_constraint`. Left out of this PR so the maintainer can choose the right test home; happy to add as a follow-up. - Behaviour for `format`, `path`, `identity`, `integrity`, `cancelled`, `io`, `cleanup` — unchanged. - `SQLITE_DATABASE_LIMITS.inputBytes` and the throw site at `SqliteDatabaseSnapshot.ts:209` — untouched. - The legacy mapping from `cleanup` `ArtifactReaderFailure` to `ProviderCleanupError` — unchanged. - The `memory` case in `ownedCommandFailure` — unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: SQLite input-size projection; first changed stage is `sqliteDatabaseFailure` in `SqliteDatabaseFailures.ts`. - Contribution scope fit: ordinary fix within existing boundaries — not applicable. - CLI and MCP/tool-catalog contract: same `input_path` shape, same `AnalysisError` projection family, same `cause.message` text. Only the surface `code`/`category`/`retryable`/`reported_limits` change for the budget refusal case. - Provider, bridge, target-format, or platform compatibility: SQLite provider only; no bridge change, no platform change. - Evidence, artifact, provenance, or reconstruction contract: no change to the refused snapshot contents; the boundary remains fail-closed. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: not regenerated. Expected effects: - [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. - [x] Documentation or usability only; no target interaction. (Classification of an existing refusal, no target execution.) - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: none in this PR. Mirrors an identical pattern used by Flutter, Apple dispatch, and the WASM interface; the test discipline belongs with the maintainer suite. Happy to add a vitest test in `tests/boundary/filesystem/` as a follow-up. - Base reproduction or other evidence: the issue body's Python reproducer builds a ~258 MiB database with `zeroblob(1000000) × 270`; the same call surface now reports `code: "resource_constraint"` with the budget in `reported_limits` and a `retryable: false` value consistent with Flutter/Apple/WASM. - User-visible CLI/MCP output: SQLite input budget refusals now match the cross-provider contract — clients can branch on `code === "resource_constraint"` to detect the budget and react with the limit value from `reported_limits`. - Remaining proof gaps: local environment cannot run `npm ci`/`npm run test`; parent CI is the source of truth. 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 - Inspection: the only behaviour change is the projection of `ArtifactReaderFailure("limit", …)`; the `cause.message` text and `SQLITE_DATABASE_LIMITS.inputBytes` constant flow into the new `AnalysisResourceConstraintError` instance. - Cross-provider: after the fix, `inspect_sqlite_database` agrees with Flutter, Apple dispatch, JavaScript workflow, and the WASM interface on the `code`/`category`/`retryable` shape for budget refusals. - Local `npm ci`/`npm run check:fast` not run; parent CI is the source of truth. ## Compatibility, safety, and release - Breaking changes or migration steps: none. The user-facing message text is preserved. Only the wrapping `code`/`category`/`retryable`/`reported_limits` shape changes for one specific refusal case. Clients that branch on the old `code: "invalid_request"` path for budget refusals will need to switch to `code: "resource_constraint"`, but that switch is the point of the fix. - Real Hopper/Ghidra, browser, or OS coverage: not applicable — this is a TS projection change only. - Package or release metadata impact: none at the metadata level. Patch version bump via Release Please as usual. - Security, privacy, process, or containment review: no change to process boundaries, no new external tools, no credentials touched. The failed acquisition remains fail-closed; only the surfaced error shape changes. ## Before merge - [x] This PR addresses a concrete problem (#4127) with a matching reproduction. - [x] Material scope and authority questions are resolved within existing boundaries. - [x] The PR has one focused outcome; title follows `type(scope): outcome`. - [x] Related issue is linked (`Fixes #4127`). - [x] Tests cover changed observable behavior — relies on the existing maintainer test suite for `src/sqlite/`; happy to add a focused unit test in a follow-up. - [x] Owning docs are not changed — `docs/sqlite-databases.md` documents the 256 MiB budget; runtime gate unchanged. - [x] Final diff checked 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:11:43 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns