morluto/rea

Fork 0

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

[PR #5564] [MERGED] fix(sqlite): name the selected path in malformed-database errors #6964

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5564
Author: @kaoru0822-kitauji
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/sqlite-malformed-path

📝 Commits (3)

c621988 fix(sqlite): name the selected path in malformed-database errors
4b13bcc Merge origin/main into fix/sqlite-malformed-path
cff634c Merge remote-tracking branch 'origin/main' into fix/sqlite-malformed-path

📊 Changes

2 files changed (+35 additions, -13 deletions)

View changed files

📝 src/sqlite/SqliteDatabaseWorkerExecution.ts (+9 -3)
📝 tests/acceptance/sqliteDatabaseCli.test.ts (+26 -10)

📄 Description

What broke: a malformed SQLite file failed with the worker's snapshot-only message, so two bad inputs gave identical errors with no selected path (#5520).
Fix: the worker-execution boundary rewrites path rejections to name the caller-selected path; other argument rejections keep their message.
Risk: low; message text only for invalid_format path rejections, error code and details unchanged.

Summary

Fixes #5520. inspect-sqlite-database rejected malformed (non-SQLite) files correctly, but the error never named the selected file, so two distinct malformed selections produced identical error documents. docs/sqlite-databases.md ("Source files and failures") promises the selected path and a meaningful reason for malformed databases. Each malformed-database error now names its own selected path while keeping the refusal, error code and format reason.

Problem and expected behavior

Trigger: inspect any readable regular file that is not a SQLite database. Before: details.issues[0] was {"path":["path"],"reason":"invalid_format","message":"file is not a database"}. ["path"] names the argument key, not the selected value. The directory and missing-file refusals already name the selected path.

After (CLI and MCP):

{"path":["path"],"reason":"invalid_format","message":"Selected SQLite database is invalid (file is not a database): /abs/selected.db"}

Change and scope

Root cause: the owned worker opens only the private snapshot copy, so its SQLite exception message cannot know the caller's selection. rejectedReply in src/sqlite/SqliteDatabaseWorkerExecution.ts maps worker rejections into caller-facing input issues, and it moved the worker message across unchanged. The outer sqliteDatabaseFailure mapper returns existing AnalysisErrors as they are, so nothing added the path later.

The fix is made where caller-facing issues are produced: rejectedReply now receives request.input.path, and the rejections it attributes to the path argument (format, unsupported) name it in the same Selected …: <path> form as the existing missing-file messages. Table and column selection rejections are unchanged because they already name their own selections. No downstream repair was added.

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

Contract and boundary impact

Semantic owner and earliest changed stage: SQLite worker reply projection (rejectedReply).
Contribution scope fit and linked maintainer decision: not applicable (bug fix inside the existing adapter).
CLI and MCP/tool-catalog contract: only the error issue message text changes for path-scoped worker rejections. Code, category, issue path and reason are unchanged, and there is no schema change.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: errors now carry the caller-selected path as the docs promise.
Target selection, authority, upstream behavior, and cleanup details: none.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: tests/acceptance/sqliteDatabaseCli.test.ts ("returns actionable public errors for corrupt input…") now inspects two distinct malformed files through the public CLI and MCP and requires each details.issues entry to carry invalid_format and a message naming that file's own selected path. On the base revision this assertion fails because the message was only file is not a database. That conclusion comes from reading the source; I did not run the test on base.
Base reproduction or other evidence: source inspection of SqliteDatabaseWorker.ts, SqliteDatabaseWorkerExecution.ts and SqliteDatabaseFailures.ts on origin/main. The reporter's runtime reproduction is in #5520.
User-visible CLI/MCP output: see above.
Remaining proof gaps: I validated only on macOS. The unsupported worker reason is listed in the reply schema, but no current worker code produces it, so it has no executed coverage.

Validation performed

npm run test:focused -- tests/acceptance/sqliteDatabaseCli.test.ts tests/boundary/sqlite/sqliteDatabaseProvider.test.ts tests/boundary/sqlite/sqliteColumnIdentity.test.ts: 3 files passed, 42 tests passed, 1 skipped (a test that was already skipped; the updated corrupt-input test ran and passed).
npm run check:fast: typecheck and lint passed.

Compatibility, safety, and release

Breaking changes or migration steps: none. Only the error message text changes.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; local node:sqlite on macOS.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the path is the caller-selected input, which other SQLite errors already report.

Author readiness

Human owner: @kaoru0822-kitauji

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.

🤖 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/5564 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/sqlite-malformed-path` --- ### 📝 Commits (3) - [`c621988`](https://github.com/morluto/rea/commit/c6219887aaaaefc5becfe266ceb9fd42d80e5d56) fix(sqlite): name the selected path in malformed-database errors - [`4b13bcc`](https://github.com/morluto/rea/commit/4b13bcc319c36bd43f23a5530d2ed12c682c578e) Merge origin/main into fix/sqlite-malformed-path - [`cff634c`](https://github.com/morluto/rea/commit/cff634c0a8cf04792d4c6de00ddb7eb0426453ef) Merge remote-tracking branch 'origin/main' into fix/sqlite-malformed-path ### 📊 Changes **2 files changed** (+35 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `src/sqlite/SqliteDatabaseWorkerExecution.ts` (+9 -3) 📝 `tests/acceptance/sqliteDatabaseCli.test.ts` (+26 -10) </details> ### 📄 Description **What broke:** a malformed SQLite file failed with the worker's snapshot-only message, so two bad inputs gave identical errors with no selected path (#5520). **Fix:** the worker-execution boundary rewrites `path` rejections to name the caller-selected path; other argument rejections keep their message. **Risk:** low; message text only for `invalid_format` path rejections, error code and details unchanged. ## Summary Fixes #5520. `inspect-sqlite-database` rejected malformed (non-SQLite) files correctly, but the error never named the selected file, so two distinct malformed selections produced identical error documents. `docs/sqlite-databases.md` ("Source files and failures") promises the selected path and a meaningful reason for malformed databases. Each malformed-database error now names its own selected path while keeping the refusal, error code and format reason. ## Problem and expected behavior Trigger: inspect any readable regular file that is not a SQLite database. Before: `details.issues[0]` was `{"path":["path"],"reason":"invalid_format","message":"file is not a database"}`. `["path"]` names the argument key, not the selected value. The directory and missing-file refusals already name the selected path. After (CLI and MCP): ```json {"path":["path"],"reason":"invalid_format","message":"Selected SQLite database is invalid (file is not a database): /abs/selected.db"} ``` ## Change and scope Root cause: the owned worker opens only the private snapshot copy, so its SQLite exception message cannot know the caller's selection. `rejectedReply` in `src/sqlite/SqliteDatabaseWorkerExecution.ts` maps worker rejections into caller-facing input issues, and it moved the worker message across unchanged. The outer `sqliteDatabaseFailure` mapper returns existing `AnalysisError`s as they are, so nothing added the path later. The fix is made where caller-facing issues are produced: `rejectedReply` now receives `request.input.path`, and the rejections it attributes to the `path` argument (`format`, `unsupported`) name it in the same `Selected …: <path>` form as the existing missing-file messages. Table and column selection rejections are unchanged because they already name their own selections. No downstream repair was added. Contribution scope: an ordinary fix within existing boundaries; no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: SQLite worker reply projection (`rejectedReply`). - Contribution scope fit and linked maintainer decision: not applicable (bug fix inside the existing adapter). - CLI and MCP/tool-catalog contract: only the error issue `message` text changes for path-scoped worker rejections. Code, category, issue path and reason are unchanged, and there is no schema change. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: errors now carry the caller-selected path as the docs promise. - Target selection, authority, upstream behavior, and cleanup details: none. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `tests/acceptance/sqliteDatabaseCli.test.ts` ("returns actionable public errors for corrupt input…") now inspects two distinct malformed files through the public CLI and MCP and requires each `details.issues` entry to carry `invalid_format` and a message naming that file's own selected path. On the base revision this assertion fails because the message was only `file is not a database`. That conclusion comes from reading the source; I did not run the test on base. - Base reproduction or other evidence: source inspection of `SqliteDatabaseWorker.ts`, `SqliteDatabaseWorkerExecution.ts` and `SqliteDatabaseFailures.ts` on `origin/main`. The reporter's runtime reproduction is in #5520. - User-visible CLI/MCP output: see above. - Remaining proof gaps: I validated only on macOS. The `unsupported` worker reason is listed in the reply schema, but no current worker code produces it, so it has no executed coverage. ## Validation performed - `npm run test:focused -- tests/acceptance/sqliteDatabaseCli.test.ts tests/boundary/sqlite/sqliteDatabaseProvider.test.ts tests/boundary/sqlite/sqliteColumnIdentity.test.ts`: 3 files passed, 42 tests passed, 1 skipped (a test that was already skipped; the updated corrupt-input test ran and passed). - `npm run check:fast`: typecheck and lint passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Only the error message text changes. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; local `node:sqlite` on macOS. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the path is the caller-selected input, which other SQLite errors already report. ## Author readiness Human owner: @kaoru0822-kitauji - [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 - [ ] 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](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-11-01 21:44:21 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:48:07 +00:00

[PR #6963] [MERGED] fix(browser): parse repeated CSP and policy header field lines separately #7476

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

No description provided.

Rows
Columns