[PR #2911] [CLOSED] fix(sqlite): report a selected database held by another process as retryable #5489

New issue

Closed

opened 2026-10-31 01:07:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2911
Author: @pablodiazjorge
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/sqlite-busy-selected-file

📝 Commits (5)

664c669 fix(sqlite): report a selected database held by another process as retryable
7eab09c test(sqlite): isolate the busy-database cleanup check and order holder cleanup
2585143 fix(sqlite): name the busy member of the database file set
2f14124 merge(main): bring PR #2911 to CI checkpoint 2895a18
fe9aca2 test(sqlite): fail fast when the busy-file holder cannot start

📊 Changes

2 files changed (+184 additions, -0 deletions)

View changed files

📝 src/sqlite/SqliteDatabaseFailures.ts (+25 -0)
➕ tests/boundary/sqlite/sqliteDatabaseBusy.test.ts (+159 -0)

📄 Description

Summary

Fixes #2822 for the SQLite provider. On Windows, when another process holds the selected database open without read sharing, the read fails with EBUSY and inspect_sqlite_database reported a generic execution_failure with retryable: false, telling the caller to run rea doctor and not to retry. The same request succeeds once the other process releases the file.

Problem and expected behavior

Smallest trigger, on Windows: hold a database with [System.IO.File]::Open('<path>', 'Open', 'ReadWrite', 'None') and run rea inspect-sqlite-database <path> --json. Before this change: code: execution_failure, retryable: false. Expected: a retryable outcome that says the file is in use and how to recover.

Change and scope

src/sqlite/SqliteDatabaseFailures.ts: in the artifact-read phase, EBUSY now maps to AnalysisCapabilityUnavailableError with retryable: true, next to the existing EACCES/EPERM and ENOENT/ENOTDIR mappings. It projects as capability_unavailable, category unavailable, with a message to close the owning application or inspect a copy, and details.reason keeps the path and EBUSY. This is the same retryable unavailable route that #2467 uses for transient Windows sharing conflicts.
tests/boundary/sqlite/sqliteDatabaseBusy.test.ts: a portable test that injects the EBUSY open failure and checks the projection and that no private root is left, and a Windows-only test where a PowerShell process holds the file without sharing.

Scope: SQLite only. As #2822 notes, about 22 providers map EACCES/EPERM on selected-file reads and none maps EBUSY; a shared mapping is left for your decision there.

Contribution scope: not applicable (error classification within an existing provider).

Contract and boundary impact

Semantic owner and earliest changed stage: the SQLite provider's failure classification.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: no schema change; the error for this case changes from execution_failure to the existing capability_unavailable with retryable: true.
Provider, bridge, target-format, or platform compatibility: Windows behavior improves; POSIX hosts rarely return EBUSY for regular files.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): no target is read; the private root is still cleaned up (checked in the test).
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

Tests added or updated: tests/boundary/sqlite/sqliteDatabaseBusy.test.ts (2 tests).
Base reproduction or other evidence: without the change, both tests fail with code: execution_failure; the real Windows case is in #2822.
User-visible CLI/MCP output (if applicable): {"code":"capability_unavailable","category":"unavailable","retryable":true,"message":"The selected SQLite database is held open by another process without read sharing. Close that application or inspect a copy, then retry.", ...}
Remaining proof gaps: macOS and Linux not run locally (the portable test covers the mapping there).

Validation performed

Windows 11 x64 (10.0.26300), non-elevated, Node 24.18.0, Windows native addon built from the same commit.

npx vitest run tests/boundary/sqlite/sqliteDatabaseBusy.test.ts on 1b6c899b0: 2 passed; without the change, both fail with code: execution_failure.
The SQLite test files: 96 passed, 9 skipped and 2 failed. The 2 failures are the Windows-only test assumptions fixed in #2824, not yet merged.
npm run check:fast: passed.
npx oxfmt --check on the two files: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none; callers that retried only on retryable: true now retry this case.
Real Hopper/Ghidra, browser, or OS coverage: native Windows with a real sharing violation.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the test spawns PowerShell only on Windows, to hold a file it created.

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/2911 **Author:** [@pablodiazjorge](https://github.com/pablodiazjorge) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/sqlite-busy-selected-file` --- ### 📝 Commits (5) - [`664c669`](https://github.com/morluto/rea/commit/664c669bd5dc92e924907b21c92f6f7d055bfc72) fix(sqlite): report a selected database held by another process as retryable - [`7eab09c`](https://github.com/morluto/rea/commit/7eab09c64c05ffa42d8a79775dbe4288f072fcac) test(sqlite): isolate the busy-database cleanup check and order holder cleanup - [`2585143`](https://github.com/morluto/rea/commit/2585143a69eb603d9449b22e0348ca2efa65ea68) fix(sqlite): name the busy member of the database file set - [`2f14124`](https://github.com/morluto/rea/commit/2f14124f402f1a3b658cf942fe4e66ad67ab0476) merge(main): bring PR #2911 to CI checkpoint 2895a18 - [`fe9aca2`](https://github.com/morluto/rea/commit/fe9aca2239c175e1690054bc7ced4735f82b2c32) test(sqlite): fail fast when the busy-file holder cannot start ### 📊 Changes **2 files changed** (+184 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/sqlite/SqliteDatabaseFailures.ts` (+25 -0) ➕ `tests/boundary/sqlite/sqliteDatabaseBusy.test.ts` (+159 -0) </details> ### 📄 Description ## Summary Fixes #2822 for the SQLite provider. On Windows, when another process holds the selected database open without read sharing, the read fails with `EBUSY` and `inspect_sqlite_database` reported a generic `execution_failure` with `retryable: false`, telling the caller to run `rea doctor` and not to retry. The same request succeeds once the other process releases the file. ## Problem and expected behavior Smallest trigger, on Windows: hold a database with `[System.IO.File]::Open('<path>', 'Open', 'ReadWrite', 'None')` and run `rea inspect-sqlite-database <path> --json`. Before this change: `code: execution_failure`, `retryable: false`. Expected: a retryable outcome that says the file is in use and how to recover. ## Change and scope - `src/sqlite/SqliteDatabaseFailures.ts`: in the artifact-read phase, `EBUSY` now maps to `AnalysisCapabilityUnavailableError` with `retryable: true`, next to the existing `EACCES`/`EPERM` and `ENOENT`/`ENOTDIR` mappings. It projects as `capability_unavailable`, category `unavailable`, with a message to close the owning application or inspect a copy, and `details.reason` keeps the path and `EBUSY`. This is the same retryable `unavailable` route that #2467 uses for transient Windows sharing conflicts. - `tests/boundary/sqlite/sqliteDatabaseBusy.test.ts`: a portable test that injects the `EBUSY` open failure and checks the projection and that no private root is left, and a Windows-only test where a PowerShell process holds the file without sharing. Scope: SQLite only. As #2822 notes, about 22 providers map `EACCES`/`EPERM` on selected-file reads and none maps `EBUSY`; a shared mapping is left for your decision there. Contribution scope: not applicable (error classification within an existing provider). ## Contract and boundary impact - Semantic owner and earliest changed stage: the SQLite provider's failure classification. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema change; the error for this case changes from `execution_failure` to the existing `capability_unavailable` with `retryable: true`. - Provider, bridge, target-format, or platform compatibility: Windows behavior improves; POSIX hosts rarely return `EBUSY` for regular files. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): no target is read; the private root is still cleaned up (checked in the test). - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/sqlite/sqliteDatabaseBusy.test.ts` (2 tests). - Base reproduction or other evidence: without the change, both tests fail with `code: execution_failure`; the real Windows case is in #2822. - User-visible CLI/MCP output (if applicable): `{"code":"capability_unavailable","category":"unavailable","retryable":true,"message":"The selected SQLite database is held open by another process without read sharing. Close that application or inspect a copy, then retry.", ...}` - Remaining proof gaps: macOS and Linux not run locally (the portable test covers the mapping there). ## Validation performed Windows 11 x64 (10.0.26300), non-elevated, Node 24.18.0, Windows native addon built from the same commit. - `npx vitest run tests/boundary/sqlite/sqliteDatabaseBusy.test.ts` on `1b6c899b0`: 2 passed; without the change, both fail with `code: execution_failure`. - The SQLite test files: 96 passed, 9 skipped and 2 failed. The 2 failures are the Windows-only test assumptions fixed in #2824, not yet merged. - `npm run check:fast`: passed. - `npx oxfmt --check` on the two files: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; callers that retried only on `retryable: true` now retry this case. - Real Hopper/Ghidra, browser, or OS coverage: native Windows with a real sharing violation. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the test spawns PowerShell only on Windows, to hold a file it created. ## 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:07:03 +00:00