Skip to content

fix(cache): keep Windows probes out of concurrently scanned trees - #1795

Merged
mldangelo merged 1 commit into
mainfrom
fix/windows-cache-probe-scan-tree-isolation
Aug 1, 2026
Merged

fix(cache): keep Windows probes out of concurrently scanned trees#1795
mldangelo merged 1 commit into
mainfrom
fix/windows-cache-probe-scan-tree-isolation

Conversation

@mldangelo

Copy link
Copy Markdown
Member

Why

Nightly CI failed on windows-latest / Python 3.11 for three consecutive runs (Jul 29, 30, 31), plus Jul 26 and Jul 22. The recurring failure is the assets-tree scan in tests/test_security_asset_integration.py:

FAILED tests/test_security_asset_integration.py::TestSecurityAssetIntegration::test_performance_with_organized_structure
  AssertionError: Performance test scan should not have operational errors

Root cause

On Windows a cache identity probe is a TemporaryFile whose name stays visible until close, so it is observable to any scan walking the directory that holds it. (On POSIX the file is unlinked immediately and never appears in a directory walk — which is why this is Windows-only.)

_get_change_clock_probe only reaches its ancestor fallback when the temp and cache directories are on a different volume from the scanned file — exactly the Windows CI layout, where the checkout is on D: while %TEMP% and ~/.modelaudit are on C:. Both preferred candidates fail the same-device check, and the code then walked protected_root.parents nearest-first, so the probe landed just above the scanned file — inside tests/assets.

A concurrent pytest-xdist worker scanning tests/assets then enumerated that probe, and os.path.getsize raised once the probe was released, yielding operational_error_reason: file_size_check_failedhas_errors=True → whole scan fails.

The Jul 26 nightly log captured the probe directly:

"D:\\a\\modelaudit\\modelaudit\\tests\\assets\\samples\\pickles\\.modelaudit-cache-clock-20tlhazm": {
  "scanner_dependency_ids": ["error"],
  "operational_error": true,
  "operational_error_reason": "file_size_check_failed"
}

This is a real product defect beyond CI: modelaudit could write temporary probe files into a user's model directory whenever the scan target is on a different volume from temp/cache (e.g. a mounted model volume on Windows).

Fix

Walk the ancestors outermost-first so the cross-volume fallback settles near the volume root instead of inside a deep tree an unrelated scan is enumerating. The change is confined to the os.name == "nt" branch; POSIX ordering is untouched and correct as-is.

Also made the asset-scan assertions self-diagnosing — they previously truncated to an unhelpful model repr that hid which file failed, which is why this took several nightlies to pin down.

Tests

test_windows_change_clock_probe_prefers_outermost_ancestor_over_scanned_tree reproduces the CI topology (off-device temp/cache, every ancestor on-device). It fails on the pre-fix code with the probe landing in .../tests/assets/samples — the exact CI signature — and passes after.

Validation

  • ruff format / ruff check: clean
  • mypy: clean (only 5 pre-existing macOS-only os.setxattr/getxattr errors, present on main)
  • Full lane pytest -n auto -m "not slow and not integration": 81 failures on this branch vs 84 on clean main in the same local macOS environment; 70 shared. The 11 branch-only entries all pass in isolation (xdist-load flakes in the cache-invalidation family, churning in both directions — 14 tests failed only on main). No regressions.

Known remaining nightly flakes (not addressed here)

Two other Windows-only flake classes appeared in the same nightlies and are separate root causes:

  1. packages/modelaudit-picklescan/tests/test_api.pyScanStatus.INCONCLUSIVE instead of COMPLETE from the shared call-graph source-stability guard (Jul 26, Jul 29). fix(cache): isolate Windows probes and stabilize nightly checks #1782 established a narrow tolerance helper (_assert_call_graph_source_stability_error) for this; extending it needs a real Windows reproduction to know which assertions still hold under INCONCLUSIVE, so it is deliberately not guessed at here.
  2. tests/utils/file/test_large_file_handler.py::test_large_handler_cache_preserves_private_metadata_for_internal_resultsassert 2 == 1, a cache-store decline under load (Jul 30). Same family as the locally-flaky cache tests above.

🤖 Generated with Claude Code

Nightly CI failed on windows-latest/3.11 for three consecutive runs with
`has_errors is True` from the assets-tree scans in
tests/test_security_asset_integration.py.

Root cause: on Windows a cache identity probe is a TemporaryFile whose name
stays visible until close, so it is observable to any scan walking the
directory that holds it. `_get_change_clock_probe` only reaches the ancestor
fallback when the temp and cache directories are on another volume — exactly
the Windows CI layout, where the checkout is on D: while %TEMP% and
~/.modelaudit are on C:. It then walked `protected_root.parents`
nearest-first, so the probe landed just above the scanned file, inside
tests/assets. A concurrent xdist worker scanning tests/assets enumerated the
probe, and `os.path.getsize` raised once the probe was released, producing
`operational_error_reason: file_size_check_failed` and failing the scan. The
7/26 nightly log captured the probe under tests/assets/samples/pickles/.

Walk the ancestors outermost-first so the fallback settles near the volume
root rather than inside a deep tree an unrelated scan is enumerating. The
ordering change is confined to the Windows branch; POSIX TemporaryFile
unlinks immediately and is never visible to a directory walk.

Also report the offending paths and reasons in the asset-scan assertions,
which previously truncated to an unhelpful model repr and hid which file
actually failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 31, 2026 08:46
@github-actions

Copy link
Copy Markdown
Contributor

Workflow run and artifacts

Performance Benchmarks

Compared 13 shared benchmarks with a regression threshold of 15%.
Status: 0 regressions, 0 improved, 13 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 2.781s -> 2.803s (+0.8%).

Workload Benchmark Target Size Files Baseline Current Change Status
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 420.40ms 448.59ms +6.7% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 99.41ms 93.93ms -5.5% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 169.8us 178.3us +5.0% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 107.19ms 103.48ms -3.5% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 189.4us 193.4us +2.1% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 209.5us 212.1us +1.2% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 404.57ms 400.67ms -1.0% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 183.4us 184.9us +0.8% stable
rejected-basic-auth-candidates tests/benchmarks/test_scan_benchmarks.py::test_rejected_basic_auth_candidates_scan_linearly - 371.1 KiB 1 1.515s 1.522s +0.5% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 83.47ms 83.10ms -0.5% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 69.74ms 69.55ms -0.3% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 141.7us 142.0us +0.3% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 80.90ms 80.70ms -0.2% stable

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a Windows-specific cache/probe placement bug where cross-volume cache identity probes could be created inside a concurrently scanned directory tree, causing intermittent operational errors in parallel test runs. It also improves integration-test assertion diagnostics and adds a regression test to reproduce the CI topology.

Changes:

  • Adjust Windows _get_change_clock_probe candidate ordering to prefer outermost ancestors when falling back to scanned-path ancestors.
  • Add a targeted regression test covering cross-volume fallback probe placement outside the scanned assets tree.
  • Improve asset integration test assertions by including per-file operational error details in failure messages; update changelog.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
modelaudit/cache/scan_results_cache.py Changes Windows probe candidate ordering to avoid placing temp probes inside deep scanned trees.
tests/cache/test_cache_correctness.py Adds regression coverage for cross-volume Windows probe placement behavior.
tests/test_security_asset_integration.py Makes has_errors assertions self-diagnosing by summarizing per-file operational error metadata.
CHANGELOG.md Documents the Windows probe placement behavior change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1139 to +1144
# Windows keeps TemporaryFile names visible and locked until close, so a
# probe is observable to every concurrent scan that walks the directory
# holding it. Ancestors are only reached when the temp and cache
# directories live on another volume; walk them outermost-first so the
# fallback settles near the volume root instead of inside a deep tree
# that an unrelated scan is enumerating.
@mldangelo
mldangelo merged commit c29586b into main Aug 1, 2026
32 checks passed
@mldangelo
mldangelo deleted the fix/windows-cache-probe-scan-tree-isolation branch August 1, 2026 12:54
@github-actions github-actions Bot mentioned this pull request Aug 1, 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.

2 participants