morluto/rea

Fork 0

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

[GH-ISSUE #2612] fix(sqlite): preserve completed worker replies when cleanup cannot be confirmed #930

New issue

Closed

opened 2026-10-15 18:29:40 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:29:40 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2612

Summary

Inspecting an ordinary SQLite table can return a cleanup error without any of the rows already written to the worker reply. This happens when the worker completes successfully but its process cleanup cannot be confirmed. The provider retains the process owner and private snapshot for retry, yet the inspection result loses the available observation.

Steps to reproduce

Use the stated source baseline and create a quiescent SQLite database containing CREATE TABLE records(id INTEGER, label TEXT); INSERT INTO records VALUES(1,'alpha');.
Instantiate SqliteDatabaseProvider with its supported launcher seam. In that launcher, read the worker request, produce the reply using the production inspectSqliteDatabaseSnapshot(request.snapshot_path, request.input) function, and write {ok:true,inspection:...} to request.reply_path. Return a launched Node child that exits with code zero, its ownership record, and a cleanup callback that returns {cleaned:false,reason:"fixture cleanup denial"} on its first call and {cleaned:true,signaled:true} on its second call.
Call provider.inspect({path: databasePath, table:"records"}). Check the projected error code, the error's partialObservation, and the retained private root's reply file.
Call provider.close() and check that it retries cleanup and removes the private root.

This was executed as a focused source test. The fixture uses the production snapshot and inspection implementations with an injected launcher; it does not establish the frequency of cleanup refusals with the real worker launcher.

Expected behavior

Once a successful inspection reply exists, an inability to confirm process cleanup should preserve the available rows and schema together with the cleanup status. Ownership must remain available for retry. Cleanup must not convert a successful inspection into an error that loses all observation data.

Actual behavior

The inspection returned cleanup_incomplete and had no partialObservation. The retained reply contained ok:true and one row with integer 1 and text alpha. Closing the provider made the second cleanup attempt and removed the private root. The parent had not read or validated that reply before constructing the error.

Evidence

Expected source: AGENTS.md, Architecture and Cleanup, requires preservation of partial facts and source evidence; docs/sqlite-databases.md:113-115 documents preservation of completed observations during later private-root cleanup failure; src/process/OwnedCommand.ts:47 defines the ownership-retention callback that preserves an execution outcome.
Failure source: src/sqlite/SqliteDatabaseWorkerExecution.ts:134-150 omits that callback and reads the reply only after command completion; src/process/OwnedCommand.ts:199 throws on unconfirmed cleanup; src/sqlite/SqliteDatabaseFailures.ts:125 maps that throw to a cleanup error without an observation. A focused injected-launcher test observed the missing observation and the still-readable successful reply.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

The real inspection implementation generated the reply from the provider-owned snapshot. The process completion and cleanup refusal were deterministic fixture conditions.

Restoration check

For the same database and cleanup-refusal fixture, return the selected schema and row either in a successful observation or in the cleanup error's partial observation. Keep the private root and process owner until a successful cleanup retry. That retry must still remove the root. A worker that fails to produce a valid successful reply must continue to report its actual failure rather than an invented observation.

Runtime or environment

Source baseline 8b181716ac; Node.js v26.10.0, npm 11.19.1, Vitest 5.0.3. Focused source test with a deterministic injected launcher.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: A bounded cleanup-refusal condition loses already-produced table evidence from the caller's result while retaining the reply on disk. The test establishes this condition, not its real-host frequency.
Workaround status: unverified
Workaround basis: The test confirms that a later cleanup retry succeeds. It does not establish that another inspection preserves an equivalent observation, and continued cleanup refusal prevents further inspection admission.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2612 ## Summary Inspecting an ordinary SQLite table can return a cleanup error without any of the rows already written to the worker reply. This happens when the worker completes successfully but its process cleanup cannot be confirmed. The provider retains the process owner and private snapshot for retry, yet the inspection result loses the available observation. ## Steps to reproduce 1. Use the stated source baseline and create a quiescent SQLite database containing `CREATE TABLE records(id INTEGER, label TEXT); INSERT INTO records VALUES(1,'alpha');`. 2. Instantiate `SqliteDatabaseProvider` with its supported launcher seam. In that launcher, read the worker request, produce the reply using the production `inspectSqliteDatabaseSnapshot(request.snapshot_path, request.input)` function, and write `{ok:true,inspection:...}` to `request.reply_path`. Return a launched Node child that exits with code zero, its ownership record, and a cleanup callback that returns `{cleaned:false,reason:"fixture cleanup denial"}` on its first call and `{cleaned:true,signaled:true}` on its second call. 3. Call `provider.inspect({path: databasePath, table:"records"})`. Check the projected error code, the error's `partialObservation`, and the retained private root's reply file. 4. Call `provider.close()` and check that it retries cleanup and removes the private root. This was executed as a focused source test. The fixture uses the production snapshot and inspection implementations with an injected launcher; it does not establish the frequency of cleanup refusals with the real worker launcher. ## Expected behavior Once a successful inspection reply exists, an inability to confirm process cleanup should preserve the available rows and schema together with the cleanup status. Ownership must remain available for retry. Cleanup must not convert a successful inspection into an error that loses all observation data. ## Actual behavior The inspection returned `cleanup_incomplete` and had no `partialObservation`. The retained reply contained `ok:true` and one row with integer `1` and text `alpha`. Closing the provider made the second cleanup attempt and removed the private root. The parent had not read or validated that reply before constructing the error. ## Evidence - Expected source: `AGENTS.md`, Architecture and Cleanup, requires preservation of partial facts and source evidence; `docs/sqlite-databases.md:113-115` documents preservation of completed observations during later private-root cleanup failure; `src/process/OwnedCommand.ts:47` defines the ownership-retention callback that preserves an execution outcome. - Failure source: `src/sqlite/SqliteDatabaseWorkerExecution.ts:134-150` omits that callback and reads the reply only after command completion; `src/process/OwnedCommand.ts:199` throws on unconfirmed cleanup; `src/sqlite/SqliteDatabaseFailures.ts:125` maps that throw to a cleanup error without an observation. A focused injected-launcher test observed the missing observation and the still-readable successful reply. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The real inspection implementation generated the reply from the provider-owned snapshot. The process completion and cleanup refusal were deterministic fixture conditions. ## Restoration check For the same database and cleanup-refusal fixture, return the selected schema and row either in a successful observation or in the cleanup error's partial observation. Keep the private root and process owner until a successful cleanup retry. That retry must still remove the root. A worker that fails to produce a valid successful reply must continue to report its actual failure rather than an invented observation. ## Runtime or environment Source baseline 8b181716aca2cb467be499cba226abfb5ae19141; Node.js v26.10.0, npm 11.19.1, Vitest 5.0.3. Focused source test with a deterministic injected launcher. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: A bounded cleanup-refusal condition loses already-produced table evidence from the caller's result while retaining the reply on disk. The test establishes this condition, not its real-host frequency. - Workaround status: unverified - Workaround basis: The test confirms that a later cleanup retry succeeds. It does not establish that another inspection preserves an equivalent observation, and continued cleanup refusal prevents further inspection admission.

bsakaguchi

2026-10-15 18:29:40 +00:00

closed this issue
added the
area: domain

bug
labels

bsakaguchi commented

2026-10-15 18:29:41 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Addressed on main in b940ad0d65a4db6de1d8416d2d538c191c692708.

Completed SQLite replies remain available as partial Evidence when process cleanup is unconfirmed. Process and private snapshot ownership remain retained for retry, and malformed replies preserve their primary failure.

Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit.

Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [b940ad0d65a4db6de1d8416d2d538c191c692708](https://github.com/morluto/rea/commit/b940ad0d65a4db6de1d8416d2d538c191c692708). Completed SQLite replies remain available as partial Evidence when process cleanup is unconfirmed. Process and private snapshot ownership remain retained for retry, and malformed replies preserve their primary failure. Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit. Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

bsakaguchi referenced this issue

2026-10-16 21:17:46 +00:00

[PR #930] [MERGED] docs: add website and showcase links to READMEs #2356

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

No description provided.

Rows
Columns