[leonlaiyc]

Problem / Motivation

Goal: on Windows a corrupt memory search index is rebuilt automatically, as it already is on Linux and macOS, instead of leaving memory search empty until someone deletes the file by hand.

When the V1 memory index (memory_index.db) is corrupt, MemoryStore._get_db deletes it and opens a new one, which rebuild_index() fills from the source files. On Windows the delete fails:

WARNING kiro_crew.memory: FTS index corrupted (file is not a database), deleting and rebuilding
WARNING kiro_crew.memory: FTS rebuild failed
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: '...\memory_index.db'

The corrupt file stays where it is, rebuild_index() fails, and search() returns nothing. Every later open meets the same file and fails the same way. test_memory_selfheal.py::TestFtsSelfHealGating::test_genuine_corruption_still_self_heals reproduces it (assert 0 >= 1 on the search after the rebuild) and is a strict xfail in test/windows-expected-failures.txt.

Why it matters

Memory search feeds recall into chat turns. On a Windows install whose index is ever damaged, by a crash or a full disk mid-write, that recall stops for good with only a log warning, although the source files are intact and memory-skills-hooks.md says a corrupt V1 index "can be rebuilt from the source files".

Not a goal

• V2 member memory, whose FTS lives in its authoritative database and has no delete-and-recreate recovery.
• Other SQLite stores: the two that move a damaged file aside already close the failed connection first (see Sibling sweep).

What changed (motivation → approach → change)

• Symptom: on Windows the self-heal's delete of the corrupt index fails with WinError 32, so the index is never rebuilt.
• Root cause: _try_create_db connects and then runs its setup statements. When one of them raises file is not a database, the connection is not closed, and the raised error's traceback keeps it open while _get_db deletes the file. Linux and macOS delete a file that is still open; Windows does not while a handle is open.
• src/kiro_crew/memory.py: _try_create_db closes the connection when anything after the connect raises, then re-raises. That is the shape KnowledgeStore.__init__ already has, with a comment naming the same Windows reason, and the task store's open.
• test/windows-expected-failures.txt: the test_genuine_corruption_still_self_heals line goes, since it now passes on Windows. The two comment lines just above it (# Needs symlink-creation privilege: builds an in-vault FILE symlink ...) were written for test_md_notebook.py::test_trashing_a_symlink_leaves_its_target_alone, whose entry test(windows): fix the expected-failures matcher and restore link-guard coverage #4467 removed. They stay for now because test(windows): run the stdio probe's stderr stubs as Python scripts #23799 edits the two lines above them, and removing them here makes the two PRs conflict.

Backwards compatibility

Compatible: only the failure path changes. A connection whose setup succeeds is returned exactly as before, and the error still reaches _get_db unchanged.

Removes nothing: the connection this closes is never handed to a caller.

Tests

test/test_memory_selfheal.py:

• test_self_heal_closes_the_connection_that_found_the_corruption (new): records every connection _get_db opens on a corrupt index, and checks that each one except the connection it returns is closed (ProgrammingError: Cannot operate on a closed database). Without the fix it fails on every OS. On Windows _get_db raises the WinError 32 first. On Linux and macOS the stale connection still answers SELECT 1, so the check fails with DID NOT RAISE. That half was not run here, since there is no POSIX host, but on this host an open connection to a corrupt file does answer SELECT 1.
• test_genuine_corruption_still_self_heals runs on Windows instead of as a strict xfail.

Proof: PROVEN test/test_memory_selfheal.py::TestFtsSelfHealGating::test_genuine_corruption_still_self_heals (head 714ca52)

prove.py names only the test that fails on an assertion. With main's memory.py on this Windows host, the new test fails too, on the WinError 32 PermissionError from _get_db.

• With the fix, on a Windows host: test_memory_selfheal.py 11 passed on Python 3.13.5 and on 3.12.13.
• The 83 test files that use the V1 index (test_memory*.py plus every test naming rebuild_index, _get_db, memory_index or _try_create_db): 2727 passed, 202 skipped, 8 xfailed, with 3 failed and 1 error. All four fail identically on clean main on this host and none touches the index: two test_auto_research_handlers_coverage_campaign_routes cases (cp950 UnicodeDecodeError), test_snapshot::TestSecurity::test_symlink_filtered_out (WinError 1314, no symlink privilege) and test_auto_research_watchdog's event_loop fixture error.
• black (the test file is baselined; the added lines are clean), isort, flake8, check_subprocess_encoding, check_black_formatting, check_comment_history, check_brand_name, check_file_line_cap, and test_flake_pattern_ratchet.py + test_windows_gap_list.py (65 passed) all pass.
• Trial merges with every open PR that edits test/windows-expected-failures.txt (test(windows): burn down six path-separator xfails #23777, test(windows): burn down the gateway-run xfails by patching the rlimit seam #23781, test(windows): pin USERPROFILE in the two subagent cwd tilde tests #23786, test(windows): write the materialize fixtures with LF, skip the symlink case #23788, test(windows): run the stdio probe's stderr stubs as Python scripts #23799, test(crew): give weak crew, agent and subagent tests real checks and merge twins #23887) are clean.

Manual verification

N/A — unit coverage sufficient: the tests drive the real _get_db self-heal on a real corrupt file, and on Windows that is the reported failure itself.

Related Issues

Fixes #24024

Sibling sweep

Swept: git grep -nE "_is_corruption_error|is_damaged_database_error|quarantine_sqlite_file" -- src/ and git grep -nE '"-wal"' -- src/ for code that deletes or moves a damaged SQLite file aside
Sibling: src/kiro_crew/memory.py:1187 -- changed here
Sibling: src/kiro_crew/knowledge/store.py:379 -- not affected -- closes the failed connection before open_recovering moves the file aside
Sibling: src/kiro_crew/taskq/store.py:548 -- not affected -- closes the connection before _CorruptStore reaches the quarantine

Pattern harvest

Rule candidate: review-prompt
Pattern: a SQLite connection whose setup raises must be closed before the error leaves when a caller deletes or moves the file on that error, because Windows refuses while a handle is open

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

🤖 Generated with Claude Code

[github-actions]

UX Review (Opus 5.5, fork) — ⏭️ skipped

Revision 714ca5243310e407d349bb341b163370acd009b2 touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Does not block merge.

[github-actions]

Design Review (Opus 5.5, fork) — ✅ PASS

Design-level review of 714ca5243310e407d349bb341b163370acd009b2 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

Nothing blocks. The fix closes the connection in the same way the knowledge and task stores already do, and the test that was a strict xfail on Windows now runs in the Windows CI lane.

Notes

• Once the test_memory_selfheal line is removed, the leftover "Needs symlink-creation privilege" comment sits directly above test_pip_deps_consistency, so it reads as if it explains that entry. It is misleading until the follow-up removes it.

[DESIGN-REVIEWED] 714ca52

Description read

[DESCRIPTION-READ] a09c2f96d6d753d1271c43d1974a8772c58599266334b260bb151c50432e93d9

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

First Principles Review (Opus 5.5, fork) — ✅ PASS

Premise-level review of 714ca5243310e407d349bb341b163370acd009b2 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

Nothing blocks. The change fixes the mechanism behind a defect that a strict xfail on base reproduces.

What this change ships

Inventory (3 items) — 3 justified

Intent: on Windows, a corrupt memory search index is rebuilt on its own instead of leaving memory search empty for good. FIX.

1. On Windows, a corrupt memory index is deleted and rebuilt, so search returns results again — justified
2. A new test checks that each connection that hit the corruption is closed before the self-heal deletes the file — justified
3. The genuine-corruption self-heal test now runs as a normal test on Windows instead of an expected failure — justified

[FIRST-PRINCIPLES-REVIEWED] 714ca52

Description read

[DESCRIPTION-READ] 18f4ab0a45254d7c792365439ac4fc0e1be8f6c975b1d4d21387f4ef775fd8ef

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

[github-actions]

GPT 6.1 Review (fork) — ✅ no blocking findings

Reviewed 714ca5243310e407d349bb341b163370acd009b2 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 714ca52

[github-actions]

Opus 5.5 Review (fork) — ✅ no blocking findings

Reviewed 714ca5243310e407d349bb341b163370acd009b2 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 714ca52

[leonlaiyc]

Re the Design note on 714ca52433 (the leftover "Needs symlink-creation privilege" comment now sits above test_pip_deps_consistency): agreed that it reads wrong, and it is left on purpose for now. Those two lines belonged to test_md_notebook.py::test_trashing_a_symlink_leaves_its_target_alone, whose entry #4467 removed. #23799 deletes the two list lines directly above them, and a trial merge with this PR's branch (git merge-tree --write-tree) conflicts when this PR deletes them as well, but merges cleanly when it leaves them. They go in a one-line follow-up once #23799 merges.

[KiroCrewCaptain]

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, security checklist all-NO, no reviewer-lane findings, readiness signal fresh for this head SHA, declared category matches the diff. Category: bug fix that closes the index connection when its setup raises, so the self-heal can delete a corrupt memory index on Windows; adds a regression test and drops the test from the Windows expected-failures list.