Skip to content

fix: protect the active database during history rollback - #323

Draft
EvanProgramming wants to merge 1 commit into
Evan/v3-legacy-result-archivalfrom
Evan/v3-history-database-protection
Draft

EvanProgramming wants to merge 1 commit into
Evan/v3-legacy-result-archivalfrom
Evan/v3-history-database-protection

Conversation

@EvanProgramming

@EvanProgramming EvanProgramming commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

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

  • 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.

@ghfind-review ghfind-review Bot added review: high ghfind author score; see https://ghfind.com area: persistence SQLite state, schema migration, event history, and recovery area: testing Test dependencies, fixtures, and acceptance coverage bug Something isn't working labels Nov 3, 2026
@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

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.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: persistence SQLite state, schema migration, event history, and recovery area: testing Test dependencies, fixtures, and acceptance coverage bug Something isn't working review: high ghfind author score; see https://ghfind.com

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant