[FIX] Rectify: Filesystem Snapshot Modification Blind Spot#3336
Merged
Trecek merged 5 commits intoMay 30, 2026
Merged
Conversation
a5477a2 to
9fedb5c
Compare
Trecek
commented
May 30, 2026
Collaborator
Author
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: approved_with_comments
Trecek
commented
May 30, 2026
Collaborator
Author
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.
⚠️ Outside Diff Range
These findings target lines not in the diff and could not be posted as inline comments:
tests/execution/test_write_evidence.py
- L741 [warning/tests]: test_snapshot_empty_when_no_new_files asserts only that no new keys appear (post.keys() - pre.keys() == set()), but _stat_snapshot now also captures mtime+size. An existing file modified between pre and post would pass the key-diff assertion while the production comparison (_post != _pre) would detect it. Assert post == pre to match production semantics.
Adds tests proving _recursive_snapshot misses in-place modifications, plus TestStatSnapshot and TestSnapshotTypeContract for the _stat_snapshot fix target. Integration test covers run_headless_core modification path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_recursive_snapshot returned set[str] (path names only), making it structurally blind to in-place file modifications. _stat_snapshot returns dict[str, tuple[int, int]] (mtime_ns, size per file), so the _post != _pre dict comparison detects creation, modification, and deletion. Updates _headless_execute.py to use the new function and dict fallbacks. Updates __init__.py re-export and all test references accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hes, weak assertion
- Use None sentinel for failed scans instead of {} to distinguish
scan failure from empty directory; continue on post-scan OSError
to avoid false-positive _fs_writes_detected
- Rename TestRecursiveSnapshot to TestSubdirSnapshot and update docstring
- Remove dead branches in test_snapshot_return_type_is_dict_not_set
(from __future__ annotations makes sig.return_annotation always str)
- Strengthen test_snapshot_empty_when_no_new_files to assert post == pre
(detects modifications, not just new file creation)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Combine the None guard and inequality check into one condition to stay within the 595-line budget for _headless_execute.py. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- _stat_snapshot: log skipped paths at debug level instead of silently swallowing OSError, improving diagnostic visibility - test_stat_snapshot_detects_modified_file: remove time.sleep(0.01) that implied mtime reliance when detection actually relies on size difference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b78f91d to
809841d
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
_recursive_snapshot()returnsset[str](path names only), making the post-session set-difference comparison (_post - _pre) structurally blind to in-place file modifications. This is a type-level defect: the snapshot's return type cannot represent file state, only file presence. The fix upgrades the snapshot to capture(mtime_ns, size)per file — a stat-based comparison that detects creation, modification, deletion, and truncation. The approach mirrors the proven_LoadCacheEntrypattern already in production atrecipe/_api_cache.py.Closes #3309
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260530-130303-981480/.autoskillit/temp/rectify/rectify_fs_writes_snapshot_blind_spot_2026-05-30_131500.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary
* Step used a non-Anthropic provider; caching behavior may differ.
Token Efficiency
Model Usage Breakdown