You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix: protect the active database during history rollback - #323
Workspace history copied the live OpenKyrozen SQLite database and its WAL/SHM files when the configured database lived in the workspace. Restoring an old snapshot could replace that database and silently discard later durable events. A Windows reproduction also shows snapshot copies can fail on SQLite file locks; the previously reported intermittent reviewer and cancellation failures are not yet attributed to this bug.
Change
Exclude the active database, its SQLite sidecars, and database directory symlink from snapshots and restoration, including snapshots created before this fix. Preserve database ancestor directories and their permissions, and reject a legacy snapshot before mutation if it would replace an ancestor with a file. Ordinary neighboring workspace files remain tracked. Reusing the history boundary avoids changing the storage format or adding dependencies. The internal HistoryStore protocol now declares the database path already exposed by EventStore; custom internal store implementations must expose that path too so history can protect their durable data.
Stacked on #322 (Evan/v3-legacy-result-archival). This is a separate data-integrity fix discovered during R260 investigation; it does not by itself close or waive the unresolved R260 reliability gate.
Verification
Regression tests reproduced loss of durable events and inclusion of live SQLite files before the fix; added coverage for old snapshots, sidecars, absent database directories, permissions, and symlinked database directories.
Full local suite: 1,185 tests, 2 platform skips; make check, make lint, and make docs-check passed.
Python source and wheel build, agent and sub-agent workflow acceptance passed.
Windows comparison on the preceding candidate: unmodified 24/25, diagnostic database exclusion 25/25; this is not proof of causation. A separate SQLite reduction reproduced 60 file-lock copy failures in 3,501 copies and no SQLite write failures. Windows testing of this commit and the six-platform candidate workflow are pending.
Risks and remaining work
The old Windows reviewer, SQLite readonly, and cancellation failures remain unexplained. Candidate-bound release evidence and the 30-minute platform matrix must be regenerated for this commit. The matched benchmark remains incomplete. No full or preview release is authorized by this PR alone.
Update: The database-protection history regression suite passed on Windows Python 3.12 and 3.13 in run 41612616502. The repeated cancellation and reviewer checks in that run are still in progress and do not yet explain the older failures.
A subsequent full-suite run on the clean signed candidate had one error in test_failed_verification_blocks_and_feedback_keeps_attempt_ids: its temporary directory was not empty during cleanup. The prior full local run passed. This later failure is under investigation and the clean-candidate local suite cannot currently be reported as passing.
The repeated Windows run completed: cancellation and reviewer acceptance each passed 20/20 times on Python 3.12 and 20/20 times on Python 3.13 (80 total). This verifies the tested behavior on this commit but does not determine the cause of the earlier failures. The local cleanup investigation found background learning still pending during 3 of 50 test fixture cleanups; a separately stacked test-isolation change is being prepared. Whether that overlap caused the one observed cleanup error is not proven.
A follow-up Windows SQLite reduction closed all connections between bursts, as the application's store does. With concurrent database/WAL/SHM copies, 326 writes failed with SQLITE_READONLY and 154 copies hit file locks; the control without copies had no errors. This independently reproduces the readonly failure mechanism, but tracing is still underway to establish how it relates to the historical acceptance failures. The cancellation timeout remains unexplained.
Draft PRs are not automatically reviewed by default.
Trigger a manual review
To automatically review draft PRs, update your CodeRabbit configuration:
reviews:
auto_review:
drafts: true
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
area: persistenceSQLite state, schema migration, event history, and recoveryarea: testingTest dependencies, fixtures, and acceptance coveragebugSomething isn't workingreview: highghfind author score; see https://ghfind.com
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem and root cause
Workspace history copied the live OpenKyrozen SQLite database and its WAL/SHM files when the configured database lived in the workspace. Restoring an old snapshot could replace that database and silently discard later durable events. A Windows reproduction also shows snapshot copies can fail on SQLite file locks; the previously reported intermittent reviewer and cancellation failures are not yet attributed to this bug.
Change
Exclude the active database, its SQLite sidecars, and database directory symlink from snapshots and restoration, including snapshots created before this fix. Preserve database ancestor directories and their permissions, and reject a legacy snapshot before mutation if it would replace an ancestor with a file. Ordinary neighboring workspace files remain tracked. Reusing the history boundary avoids changing the storage format or adding dependencies. The internal HistoryStore protocol now declares the database path already exposed by EventStore; custom internal store implementations must expose that path too so history can protect their durable data.
Stacked on #322 (
Evan/v3-legacy-result-archival). This is a separate data-integrity fix discovered during R260 investigation; it does not by itself close or waive the unresolved R260 reliability gate.Verification
make check,make lint, andmake docs-checkpassed.Risks and remaining work
The old Windows reviewer, SQLite readonly, and cancellation failures remain unexplained. Candidate-bound release evidence and the 30-minute platform matrix must be regenerated for this commit. The matched benchmark remains incomplete. No full or preview release is authorized by this PR alone.
Update: The database-protection history regression suite passed on Windows Python 3.12 and 3.13 in run 41612616502. The repeated cancellation and reviewer checks in that run are still in progress and do not yet explain the older failures.
A subsequent full-suite run on the clean signed candidate had one error in
test_failed_verification_blocks_and_feedback_keeps_attempt_ids: its temporary directory was not empty during cleanup. The prior full local run passed. This later failure is under investigation and the clean-candidate local suite cannot currently be reported as passing.The repeated Windows run completed: cancellation and reviewer acceptance each passed 20/20 times on Python 3.12 and 20/20 times on Python 3.13 (80 total). This verifies the tested behavior on this commit but does not determine the cause of the earlier failures. The local cleanup investigation found background learning still pending during 3 of 50 test fixture cleanups; a separately stacked test-isolation change is being prepared. Whether that overlap caused the one observed cleanup error is not proven.
A follow-up Windows SQLite reduction closed all connections between bursts, as the application's store does. With concurrent database/WAL/SHM copies, 326 writes failed with
SQLITE_READONLYand 154 copies hit file locks; the control without copies had no errors. This independently reproduces the readonly failure mechanism, but tracing is still underway to establish how it relates to the historical acceptance failures. The cancellation timeout remains unexplained.