[PR #2775] [MERGED] fix(sqlite): classify a missing database path as an invalid value #5384

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

📋 Pull Request Information

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

Base: main ← Head: fix/sqlite-missing-database-value


📝 Commits (5)

  • 61ec55b fix(sqlite): classify a missing database path as an invalid value
  • 5f586d4 fix(sqlite): classify a missing database path as an invalid value
  • 52c820b chore: merge upstream CI baseline fixes
  • 3e3481d chore: preserve concurrent contributor updates
  • 4f0d759 fix(sqlite): reconcile missing-path and selection diagnostics

📊 Changes

2 files changed (+36 additions, -5 deletions)

View changed files

📝 src/sqlite/SqliteDatabaseSnapshot.ts (+12 -5)
📝 tests/boundary/sqlite/sqliteDatabaseProvider.test.ts (+24 -0)

📄 Description

Summary

Fixes #2614.

Selecting a SQLite database path that does not exist returned the input reason invalid_format, the same machine-readable reason used for malformed content. The adapter already classifies native ENOENT/ENOTDIR read failures as invalid_value, so callers could not tell absence from bad content without parsing message text.

Problem and expected behavior

captureSqliteDatabaseSnapshot observed the absent file and raised ArtifactReaderFailure("path", …). sqliteDatabaseFailure projects every artifact path failure as invalid_format, which is correct for symlinks and special files but not for absence. A missing path, including one under a missing parent directory, should be invalid_value with the existing "does not exist" message.

Change and scope

The snapshot owns the absence observation, so it now returns an AnalysisInputError with an invalid_value issue on path directly. sqliteDatabaseFailure already passes AnalysisError through unchanged, so no downstream reinterpretation is added. Symlink, special-file, malformed-content and limit failures keep their existing classifications, and the message is unchanged. On POSIX, a parent component that is a file (ENOTDIR) continues through the existing native-error mapping to invalid_value.

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

Contract and boundary impact

  • Semantic owner and earliest changed stage: absence detection in captureSqliteDatabaseSnapshot (src/sqlite/SqliteDatabaseSnapshot.ts).

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

  • CLI and MCP/tool-catalog contract: schema unchanged; the issue reason for a missing database changes from invalid_format to invalid_value.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: unchanged; the failure occurs before any snapshot is captured.

  • Target selection, authority, upstream behavior, and cleanup details: unchanged; no worker is launched for a missing path.

  • 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 covers absent.db and absent-parent/absent.db and requires {path:["path"], reason:"invalid_value"} with the "does not exist" message. The existing malformed-database test still covers invalid_format. The new test is not gated on native SQLite limits because it fails before the worker runs, so it also applies to the Node 22 runtime lane.

  • Base reproduction or other evidence: on the base revision both cases return invalid_format, as in the issue.

  • User-visible CLI/MCP output:

    {"path":["path"],"reason":"invalid_value","message":"Selected SQLite database does not exist: <selected path>"}
    
  • Remaining proof gaps: on Windows the committed boundary test needs the Linux/MinGW-built native addon for the private runtime root. 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 can only be built on Linux with MinGW, so the SQLite provider boundary file cannot complete on this host (17 existing tests fail identically on the base revision).

  • Ran an end-to-end check through SqliteDatabaseProvider with a plain temporary root (not committed): both missing-path cases return invalid_value with the expected message; a malformed file still fails at the worker boundary.
  • npm run check: passed (no new lint warnings).
  • npm run verify:test-discovery: passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. A client that branched on invalid_format for a missing path now receives invalid_value, which matches the adapter's existing native-ENOENT classification.
  • 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/2775 **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-missing-database-value` --- ### 📝 Commits (5) - [`61ec55b`](https://github.com/morluto/rea/commit/61ec55b873b2d46e314bc99d31be7e8a690cffe3) fix(sqlite): classify a missing database path as an invalid value - [`5f586d4`](https://github.com/morluto/rea/commit/5f586d48bb15c3ff0be57d2a0214b0151d1899ae) fix(sqlite): classify a missing database path as an invalid value - [`52c820b`](https://github.com/morluto/rea/commit/52c820b4168f0c94ab5079b96256f236b9f725b9) chore: merge upstream CI baseline fixes - [`3e3481d`](https://github.com/morluto/rea/commit/3e3481d3a8ea1b15af723394c41b03b141c17aa7) chore: preserve concurrent contributor updates - [`4f0d759`](https://github.com/morluto/rea/commit/4f0d75996a9855c1ebb98b34954e7f9b46ea7d05) fix(sqlite): reconcile missing-path and selection diagnostics ### 📊 Changes **2 files changed** (+36 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `src/sqlite/SqliteDatabaseSnapshot.ts` (+12 -5) 📝 `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` (+24 -0) </details> ### 📄 Description ## Summary Fixes #2614. Selecting a SQLite database path that does not exist returned the input reason `invalid_format`, the same machine-readable reason used for malformed content. The adapter already classifies native `ENOENT`/`ENOTDIR` read failures as `invalid_value`, so callers could not tell absence from bad content without parsing message text. ## Problem and expected behavior `captureSqliteDatabaseSnapshot` observed the absent file and raised `ArtifactReaderFailure("path", …)`. `sqliteDatabaseFailure` projects every artifact `path` failure as `invalid_format`, which is correct for symlinks and special files but not for absence. A missing path, including one under a missing parent directory, should be `invalid_value` with the existing "does not exist" message. ## Change and scope The snapshot owns the absence observation, so it now returns an `AnalysisInputError` with an `invalid_value` issue on `path` directly. `sqliteDatabaseFailure` already passes `AnalysisError` through unchanged, so no downstream reinterpretation is added. Symlink, special-file, malformed-content and limit failures keep their existing classifications, and the message is unchanged. On POSIX, a parent component that is a file (`ENOTDIR`) continues through the existing native-error mapping to `invalid_value`. Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision applies. ## Contract and boundary impact - Semantic owner and earliest changed stage: absence detection in `captureSqliteDatabaseSnapshot` (`src/sqlite/SqliteDatabaseSnapshot.ts`). - Contribution scope fit and linked maintainer decision: not applicable (bug fix). - CLI and MCP/tool-catalog contract: schema unchanged; the issue reason for a missing database changes from `invalid_format` to `invalid_value`. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: unchanged; the failure occurs before any snapshot is captured. - Target selection, authority, upstream behavior, and cleanup details: unchanged; no worker is launched for a missing path. - 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` covers `absent.db` and `absent-parent/absent.db` and requires `{path:["path"], reason:"invalid_value"}` with the "does not exist" message. The existing malformed-database test still covers `invalid_format`. The new test is not gated on native SQLite limits because it fails before the worker runs, so it also applies to the Node 22 runtime lane. - Base reproduction or other evidence: on the base revision both cases return `invalid_format`, as in the issue. - User-visible CLI/MCP output: ```json {"path":["path"],"reason":"invalid_value","message":"Selected SQLite database does not exist: <selected path>"} ``` - Remaining proof gaps: on Windows the committed boundary test needs the Linux/MinGW-built native addon for the private runtime root. 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 can only be built on Linux with MinGW, so the SQLite provider boundary file cannot complete on this host (17 existing tests fail identically on the base revision). - Ran an end-to-end check through `SqliteDatabaseProvider` with a plain temporary root (not committed): both missing-path cases return `invalid_value` with the expected message; a malformed file still fails at the worker boundary. - `npm run check`: passed (no new lint warnings). - `npm run verify:test-discovery`: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. A client that branched on `invalid_format` for a missing path now receives `invalid_value`, which matches the adapter's existing native-ENOENT classification. - 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:13 +00:00
bsakaguchi changed title from [PR #2775] [MERGED] fix(sqlite): classify a missing database path as an invalid value to [PR #2775] fix(sqlite): classify a missing database path as an invalid value 2026-11-04 18:24:56 +00:00
bsakaguchi changed title from [PR #2775] fix(sqlite): classify a missing database path as an invalid value to [PR #2775] [MERGED] fix(sqlite): classify a missing database path as an invalid value 2026-11-04 19:25:52 +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#5384
No description provided.