fix(memory): back up the default store in the scheduled sweep - #11494
Conversation
The daily backup pass called back_up_all_stores(private_only=True), so the default Global V1 store was enumerated and then skipped on every run. An install with no crew member had no automatic memory backup at all, and one with members backed up only the small member silos while the largest store, the one every install has and the one that cannot be rebuilt from anywhere else, had no copy. Drop the private_only knob: the heartbeat and the CLI now share one sweep over the default store, declared named V1 stores and active V2 stores. The fail-soft loop, backup_dir_for and the sandbox mask of <home>/backups already handled the default store; only the caller excluded it. The setting's description, the memory spec and the generated config baseline now say what the sweep covers. sim: https://t.corp.amazon.com/P514524379
|
Intent: The daily scheduled memory backup copies the default Global V1 store (and named V1 stores), not only member V2 silos, so every install has an automatic copy of the one store that cannot be rebuilt. |
github-actions
Bot
added
the
readiness: checking
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Verify disk headroom: the daily sweep now retains up to [DESIGN-REVIEWED] 21cfcfa |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of First-Principles-Verdict: PASS Verify the one unverifiable claim: that the Memory V2 change's history truly records no reason for excluding V1 from the sweep — the reversed spec pin "V1 backups remain manual" rests on it. What this change shipsInventory (5 items) — 5 justifiedIntent: give every install a daily restorable copy of its memory, including the default store the sweep enumerated and skipped — a FIX.
[FIRST-PRINCIPLES-REVIEWED] 21cfcfa |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
github-actions
Bot
added
readiness: passed
github-actions
Bot
removed
the
readiness: passed
2 participants
Problem / Motivation
The daily memory backup skips the default store.
heartbeat.pycalledback_up_all_stores(..., private_only=True), andmemory_backup.pyskipped every store that is not a member V2 silo. The default Global V1 store, the one every install has, was enumerated and then skipped on every run. An install with no crew member had no automatic memory backup at all. One with members backed up only the small member silos.On the reporting host: nine member stores each had a fresh backup of about 4 KB, and the 11.8 MB default store carrying every memory the assistant uses had none. Its
~/.kiro/crew/backupsdirectory did not exist.Why it matters
Memory is the one thing here that cannot be rebuilt from anywhere else. The module's own header says so and names the incident it exists for: a 36 MB store that became 29 bytes with nothing to restore from. That incident was the default store. The sweep protected everything except the store it was written for.
What changed (motivation → approach → change)
The scheduled sweep now copies every active store: the default store first, then declared named V1 stores, then active member V2 stores.
🟩 added · 🟨 changed · 🟥 removed · 🟦 unchanged
The
private_onlyknob is removed rather than flipped. It arrived with the Memory V2 change and nothing in that history states a reason for excluding V1; the spec asserted the exclusion without one, and the module docstring argued the opposite. After the fix no caller wants a member-only sweep: the heartbeat andkirocrew memory backupgo through the same loop. Everything downstream already handled the default store.backup_dir_forplaces its copies under<home>/backups/, the sandbox already masks that directory, the fail-soft loop was written so one broken silo cannot cost the default store its copy, and the 20-hour freshness guard still stops a restart loop from taking a copy every boot.A config knob (
memory.backup_private_only) was considered and rejected: there is no known reason to want the default store left out, so a switch would only make the gap opt-in again.Text now matches behaviour: the
memory.backup_enabledhelp inconfig/sections.py, its row inconfiguration.md, the regeneratedconfig-baseline.json, and the backup sections ofmemory-skills-hooks.mdandslack-gateway.md.Tests
test/test_memory_resources.py:test_scheduled_backup_pass_copies_the_default_store— runsHeartbeatService._back_up_memory()against a real home and asserts a restorable copy of the default store lands inbackup_dir_for(default), with the written semantic row readable from it. This is the regression pin;prove.pyconfirms it fails with the old call restored.test_automatic_backups_cover_default_and_named_v1_and_preserve_archived_backups— the sweep with the heartbeat's call shape copies default, named V1 and active member stores (backed_up: 3), prunes only their own directories, and leaves an archived member's backups byte-identical.test_first_eligible_heartbeat_schedules_one_backup_without_blocking_ticks— now pins that the scheduled call passes onlyshould_stop, no narrowing keyword.Related backend set: 21,150 passed. Config baseline test passes on the regenerated file.
Manual verification
N/A — unit coverage sufficient: the heartbeat test exercises the real executor path and the real SQLite backup API against a temporary home.
Related Issues
no linked issue: internal ticket P514524379.
Pattern harvest
Rule candidate: review-prompt
Pattern: "a filter parameter added to a shared loop is hardcoded by its only production caller, so the loop's fail-soft branch for the excluded case is unreachable"
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)