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
Follow-up to #756 (global cross-project session index, rant 2026-08-13T16:42:22) — implements the stale-entry gap noted in my #756 review feedback (comment-5278191641, non-blocking note 1).
The reason will be displayed to describe this comment to others. Learn more.
✅ LGTM — cycle (1/3)
Follow-up to #756 implementing the stale-entry prune noted in review. Verified end-to-end on head 960e03a:
Prune logic (sessions_index.py rebuild_sessions_index): after the backfill upsert loop, walks the merged index and drops entries whose session directory no longer exists on disk (out-of-band deletes that bypass the Session.delete hook). Guards: Path.exists() wrapped in try/except (OSError/ValueError → treat as dead); manual entries pointing to a live directory are preserved (pure backfill for everything alive).
Tests (+1, 800 total): test_rebuild_prunes_stale_entries (out-of-band rmtree → entry dropped, live + scanned kept) + test_rebuild_preserves_manual_entries updated to point at a real live dir (asserts live manual entries survive). Discriminative: the prune loop is the only way s_dead leaves the index.
Agent.md doc count 799→800 synced.
Verification: uv run pytest tests/test_sessions_index.py → 21 passed; full suite → 800 passed; CI test + test-windows PASS (31685245241); MERGEABLE/CLEAN.
The reason will be displayed to describe this comment to others. Learn more.
✅ LGTM — cycle
Verified:
Prune logic (sessions_index.py): merge fresh scan into existing index, then drop entries whose Path(sdir).exists() is False (defensive OSError/ValueError catch). Correctly preserves manual entries pointing to live directories while pruning out-of-band-deleted sessions.
Test correctness: test_rebuild_preserves_manual_entries was properly updated from a nonexistent /tmp/manual (which the new prune would drop) to a real manual_dir; test_rebuild_prunes_stale_entries covers the dead-dir case. Both positive and negative states covered.
CI green: test + test-windows both pass (doc-count guard 799→800 validated).
The reason will be displayed to describe this comment to others. Learn more.
✅ LGTM — cycle (3/3)
Third independent-cycle approval. Confirmed on head 960e03a (unchanged since 1/3 + 2/3):
Prune loop drops only dead-path entries (Path.exists() False, OSError/ValueError-guarded); live manual entries preserved.
Test suite updated correctly: manual-entry test now uses a real dir (the old /tmp/manual would have been pruned); stale-entry test covers the out-of-band delete.
CI test + test-windows PASS (31685245241); MERGEABLE/CLEAN; full suite 800 passed locally (R1406).
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
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.
Follow-up to #756 (global cross-project session index, rant 2026-08-13T16:42:22) — implements the stale-entry gap noted in my #756 review feedback (comment-5278191641, non-blocking note 1).