morluto/rea

Fork 0

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

[GH-ISSUE #5286] [Bug] SQLite provider returns success when cancelled during final root cleanup #3224

New issue

Closed

opened 2026-10-31 00:38:02 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-31 00:38:02 +00:00

Owner

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

Area

SQLite provider lifecycle and cancellation.

Observed behavior

At base e84e17e30, a controlled provider test uses the real SQLite worker to produce one row, then aborts the caller from the acquired snapshot root's close() method. SqliteDatabaseProvider.inspect returns success despite cancellation being observed during awaited final cleanup. This contradicts docs/tool-design.md#own-the-lifecycle, which requires checking the signal after the last awaited finalization step and retaining collected observations under typed cancellation.

Reproduction and evidence

The test preserves completed native-worker observations across late-cancellation in tests/boundary/sqlite/sqliteDatabaseProvider.test.ts failed against e84e17e30: it expected AnalysisCancelledError, exact caller reason, and partial Evidence, but received ok: true. The same boundary suite passes after the local provider fix. A separate cleanup-denial control continues returning ProviderCleanupError with partial Evidence and retains its retry owner.

Red log: .cache/codebase-audit/sqlite5284-red.log (the failing late-cancellation case is among five expected failures on baseline). Green suite: .cache/codebase-audit/sqlite5284-green.log (51 passed, 16 skipped across five SQLite suites).

Expected behavior

After successful worker execution and successful root cleanup, return typed cancellation with the exact signal reason and the completed observation as partial Evidence when cancellation was observed before returning. Preserve a known physical failure or incomplete cleanup as primary.

Scope and limits

Verified locally on Node 25.1.0 / macOS 25.3.0 arm64 using the production SQLite worker and a temporary authored fixture. CLI/MCP projections and other platforms were not exercised. The provider test changes are local and uncommitted.

Originally created by @morluto on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5286 ### Area SQLite provider lifecycle and cancellation. ### Observed behavior At base `e84e17e30`, a controlled provider test uses the real SQLite worker to produce one row, then aborts the caller from the acquired snapshot root's `close()` method. `SqliteDatabaseProvider.inspect` returns success despite cancellation being observed during awaited final cleanup. This contradicts `docs/tool-design.md#own-the-lifecycle`, which requires checking the signal after the last awaited finalization step and retaining collected observations under typed cancellation. ### Reproduction and evidence The test `preserves completed native-worker observations across late-cancellation` in `tests/boundary/sqlite/sqliteDatabaseProvider.test.ts` failed against `e84e17e30`: it expected `AnalysisCancelledError`, exact caller reason, and partial Evidence, but received `ok: true`. The same boundary suite passes after the local provider fix. A separate cleanup-denial control continues returning `ProviderCleanupError` with partial Evidence and retains its retry owner. Red log: `.cache/codebase-audit/sqlite5284-red.log` (the failing late-cancellation case is among five expected failures on baseline). Green suite: `.cache/codebase-audit/sqlite5284-green.log` (51 passed, 16 skipped across five SQLite suites). ### Expected behavior After successful worker execution and successful root cleanup, return typed cancellation with the exact signal reason and the completed observation as partial Evidence when cancellation was observed before returning. Preserve a known physical failure or incomplete cleanup as primary. ### Scope and limits Verified locally on Node 25.1.0 / macOS 25.3.0 arm64 using the production SQLite worker and a temporary authored fixture. CLI/MCP projections and other platforms were not exercised. The provider test changes are local and uncommitted.

bsakaguchi closed this issue

2026-10-31 00:38:02 +00:00

bsakaguchi commented

2026-10-31 00:38:03 +00:00

Author

Owner

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

Runtime attribution clarification: the original local baseline/full-suite logs used the ambient PATH runtime Node 25.1.0 with npm 11.8.0, not the repository-pinned toolchain. Baseline was rerun in an isolated worktree using pinned Node 24.18.0/npm 11.16.0 and the same five provider regressions failed. The full five-suite SQLite set was subsequently run on pinned Node 24.18.0/npm 11.16.0: 67/67 passed, no skips (.cache/codebase-audit/sqlite5284-pinned-final.log). After a directly related artifact-checkpoint cleanup, provider plus snapshot-limit tests passed 29/29 pinned (.cache/codebase-audit/sqlite5284-owner-check-node24.log). The earlier 51 passed/16 skipped count belongs to Node 25.1.0. Detailed log filenames were normalized to sqlite5284-baseline-node25-red.log, sqlite5284-baseline-node24-red.log, and sqlite5284-node25-green.log.

@morluto commented on GitHub (Oct 19, 2026): Runtime attribution clarification: the original local baseline/full-suite logs used the ambient PATH runtime Node 25.1.0 with npm 11.8.0, not the repository-pinned toolchain. Baseline was rerun in an isolated worktree using pinned Node 24.18.0/npm 11.16.0 and the same five provider regressions failed. The full five-suite SQLite set was subsequently run on pinned Node 24.18.0/npm 11.16.0: 67/67 passed, no skips (`.cache/codebase-audit/sqlite5284-pinned-final.log`). After a directly related artifact-checkpoint cleanup, provider plus snapshot-limit tests passed 29/29 pinned (`.cache/codebase-audit/sqlite5284-owner-check-node24.log`). The earlier 51 passed/16 skipped count belongs to Node 25.1.0. Detailed log filenames were normalized to `sqlite5284-baseline-node25-red.log`, `sqlite5284-baseline-node24-red.log`, and `sqlite5284-node25-green.log`.

bsakaguchi commented

2026-10-31 00:38:03 +00:00

Author

Owner

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

@morluto commented on GitHub (Oct 19, 2026): Runtime attribution clarification: the original local baseline/full-suite logs used the ambient PATH runtime Node 25.1.0 with npm 11.8.0, not the repository-pinned toolchain. Baseline was rerun in an isolated worktree using pinned Node 24.18.0/npm 11.16.0 and the same five provider regressions failed. The full five-suite SQLite set was subsequently run on pinned Node 24.18.0/npm 11.16.0: 67/67 passed, no skips (`.cache/codebase-audit/sqlite5284-pinned-final.log`). After a directly related artifact-checkpoint cleanup, provider plus snapshot-limit tests passed 29/29 pinned (`.cache/codebase-audit/sqlite5284-owner-check-node24.log`). The earlier 51 passed/16 skipped count belongs to Node 25.1.0. Detailed log filenames were normalized to `sqlite5284-baseline-node25-red.log`, `sqlite5284-baseline-node24-red.log`, and `sqlite5284-node25-green.log`.

bsakaguchi commented

2026-10-31 00:38:03 +00:00

Author

Owner

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

Closing as fixed on main by 1006714271 (fix(sqlite): retain primary outcomes through cleanup retries), still present at 67a6477f01. After successful final root cleanup, SqliteDatabaseProvider.inspect checks the caller signal and returns AnalysisCancelledError with the exact signal.reason and completedSqliteEvidence. Earlier physical failures or incomplete cleanup return before that success-path check. The existing preserves completed native-worker observations across late-cancellation regression asserts the exact reason and completed partial Evidence. This closure is based on current-main source and regression inspection plus the pinned-runtime validation already recorded in this issue; no new CLI/MCP or cross-platform runtime claim is made. PR #5365 remains open for its distinct combined cancellation-and-cleanup-failure regression coverage.

@morluto commented on GitHub (Oct 21, 2026): Closing as fixed on main by 100671427105791a5c5913a918b63a394f6bc83e (fix(sqlite): retain primary outcomes through cleanup retries), still present at 67a6477f011ae358fc333e5a6e26c86d932654ed. After successful final root cleanup, SqliteDatabaseProvider.inspect checks the caller signal and returns AnalysisCancelledError with the exact signal.reason and completedSqliteEvidence. Earlier physical failures or incomplete cleanup return before that success-path check. The existing preserves completed native-worker observations across late-cancellation regression asserts the exact reason and completed partial Evidence. This closure is based on current-main source and regression inspection plus the pinned-runtime validation already recorded in this issue; no new CLI/MCP or cross-platform runtime claim is made. PR #5365 remains open for its distinct combined cancellation-and-cleanup-failure regression coverage.

bsakaguchi referenced this issue

2026-10-31 01:08:17 +00:00

[PR #3224] [CLOSED] fix(reference-source): recognize Kotlin source files #5654

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

No description provided.

Rows
Columns