Skip to content

fix(memory): requeue files that fail indexing for retry - #644

Closed
s97472091-pixel wants to merge 1 commit into
use-agent-os:mainfrom
s97472091-pixel:fix/memorysync-retry-index-failure-638
Closed

fix(memory): requeue files that fail indexing for retry#644
s97472091-pixel wants to merge 1 commit into
use-agent-os:mainfrom
s97472091-pixel:fix/memorysync-retry-index-failure-638

Conversation

@s97472091-pixel

Copy link
Copy Markdown
Contributor

Fixes #638

Problem

MemorySyncManager._do_file_sync() records the mtime before calling store.index_file(). When index_file() raises a transient error, the path is logged and dropped — but its mtime was already captured, so later watcher ticks consider the file unchanged and the index is never retried until the file is modified again or the process restarts.

Change

_do_file_sync() now returns a (failed_deletes, failed_indexes) pair. Failed indexes are re-enqueued into _pending_changes (mirroring how failed deletes already re-enqueue into _pending_deletes), and the manager stays dirty while a failed path is pending. The next watcher sync retries the unchanged file and clears the pending state after success.

Acceptance criteria coverage

  • Requeues every path whose index_file() fails ✅
  • Keeps the manager dirty while a failed path is pending ✅
  • Retries on a later watcher sync without requiring an mtime change ✅
  • Preserves the existing delete-failure retry behavior ✅

Tests

4 new offline regression tests (fake store, no network/provider): transient failure then successful retry, persistent failure stays pending+dirty, clean sync after retry does not duplicate, and delete-failure retry preserved. Full memory suite: 389 passed; ruff + mypy clean.

_do_file_sync() recorded the mtime before calling store.index_file(),
so a transient index failure was silently dropped: the file's mtime was
already captured and later watcher ticks considered it unchanged, so the
index was never retried until the file changed again or the process
restarted.

Return (failed_deletes, failed_indexes) from _do_file_sync() and
re-enqueue failed indexes into _pending_changes (like failed deletes
into _pending_deletes) so the next watcher sync retries them without
requiring an mtime change, keeping the manager dirty while pending.

Fixes use-agent-os#638
@andreapn

andreapn commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded. Reviewed alongside the other two PRs on issue #638; #781 was picked.

Your fix is correct — I verified it in a worktree and it keeps the manager dirty with the failed path back on _pending_changes, then retries and clears on the next sync. #781 was picked because it also covers the start() path: _mtimes is empty during the initial sync, so a path that fails to index there is not merely delayed, it is never rediscovered by the watcher diff at all. #781 re-enqueues that case explicitly.

This PR also currently conflicts with main on tests/test_memory_sync_manager_architecture.py.

Thanks for the contribution — this was close.

@andreapn andreapn closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: MemorySyncManager never retries files after transient index failure

2 participants