Repository navigation
[Bug]: Windows ctime-only skew can falsely reject unchanged files in remaining identity checks #8604
Description
Reviewed against signed follow-up commit 6ac8672c9b2667426e5adbce5b728c2df8e11d7c, directly after the unchanged materials fix ad9025ed9fae29fd3c7627306005ab6e427f4d53. This is an ignored local issue draft; it has not been submitted.
Classification rule and coverage
SUSCEPTIBLE means a comparison mixes descriptor/path observations, or ctime alone can reject, reset, abort or invalidate an observation. Two calls using the same stat API do not establish safety. NOT SUSCEPTIBLE applies to recording alone, an actual stable-source/non-rejecting use, or a Windows bypass; it does not certify downstream consumers.
rg -n 'st_ctime(_ns)?|ctime_ns' loopx and the broader rg -n 'st_ctime|ctime_ns' loopx --hidden identify 16 direct references in 11 Python modules. Followed stored tuple/dictionary fields through their comparisons, including persisted cursor and recovery receipts. No graph exists here; none was built. Findings below are source evidence and synthetic regression evidence, not live-Harness observations.
Every direct reference and its consumers
| Source site | Classification and evidence | Disposition |
|---|---|---|
materials.py:228, read_bounded_material_file |
SUSCEPTIBLE before existing branch fix; fixed on Windows. Descriptor before/after are compared with resolved-path current; any field mismatch rejects. Windows now excludes only ctime, while POSIX includes it. |
Keep existing commit and its provider scan/exact-read regressions; no Task B edit. |
control_plane/coordination/cold_source_backup.py:91, _same_file, called at :337 |
SUSCEPTIBLE before Task B; fixed on Windows. _open_regular supplies descriptor before; final fstat supplies after. Despite the same descriptor/API, ctime-only mismatch raises cold_import_backup_changed_retry. |
Only new production fix. Keep dev/ino/size/mtime, POSIX ctime, archive digest/declared size, regular-file/no-follow guards, full member/link validation and manifest byte readback. |
control_plane/coordination/authority_source_capture.py:199, _source_lease_bytes |
SUSCEPTIBLE. Path before = lstat() versus descriptor after = fstat(); ctime-only difference raises source_changed_retry. |
Deferred. Authority migration/source admission needs its own maintained policy and regression coverage. |
Same module :200 |
SUSCEPTIBLE. Descriptor after versus path current = lstat(); independent ctime-only rejection. Opened/current dev/ino and regular-file checks are separate safeguards. |
Deferred with the preceding site; do not weaken source fencing incidentally. |
codex_app_thread_activity.py:249, _rollout_activity |
SUSCEPTIBLE. Initial/final fstat on one open rollout; same-size ctime-only change evicts the cursor and returns RECORD_UNRECOGNIZED. Path identity checks are separate. |
Deferred. Liveness semantics and unread append handling deserve their own regression. |
hosts/codex_rollout_cursor.py:224, reset |
NOT SUSCEPTIBLE at assignment: recording only. Stores descriptor ctime into self.ctime; see comparisons below. |
Unchanged. |
Same module :326, observe |
SUSCEPTIBLE. Current descriptor observation compared with retained self.ctime from a previous observation. Same size/mtime plus ctime-only difference calls reset. |
Deferred. Across-time mutation evidence, bounded scanning and anchors are a policy boundary, not interchangeable with a one-read check. |
Same module :340, incomplete-header observation |
NOT SUSCEPTIBLE at assignment: recording only. Saves ctime alongside size/mtime/anchors; later observe compares it. |
Unchanged; downstream susceptibility retained above. |
Same module :369, successful observation update |
NOT SUSCEPTIBLE at assignment: recording only. Saves current descriptor ctime. save at :412 serializes it into the durable cache body. |
Unchanged; persisted consumer assessed separately. |
Same module :471, load |
SUSCEPTIBLE. Durable body["ctime"] versus current descriptor stat.st_ctime_ns: same-size mismatch rejects the cached cursor even when mtime matches. :477 merely restores the accepted value. |
Deferred conservatively. Historical cursors can span sessions, metadata epochs and runtime versions; matching APIs are not proof of a stable source. Binding, checksum, identity, extents, anchors and retained-fact verification must remain. |
file_lock.py:889, _effect_file_identity_from_stat |
NOT SUSCEPTIBLE at tuple construction: recording only. Stores dev/ino/birthtime/ctime. _effect_file_identity_from_fd and _effect_file_identity(path) feed the same comparator. |
No blanket exclusion. Comparator branches below determine actual exposure. |
control_plane/effect_runtime.py:354, _scan_runtime_source_snapshot |
SUSCEPTIBLE through consumers, not merely a cache-recording site. Path metadata tuples feed _require_unchanged_snapshot at :467-469 and the full-hash recheck at :508-510; ctime-only differences raise _RuntimeSourceChanged. Two unstable attempts in _runtime_fingerprint become packaged_runtime_source_unstable. |
Deferred. Source-loader/fingerprint freshness policy, disk memo keys and cache settlement interact here. Ordinary cache invalidation may only rehash; intra-read checks can abort. |
control_plane/collaboration/delegation_preview_transport.py:50, _source_snapshot |
SUSCEPTIBLE through consumers. DirEntry.stat snapshots enter the bridge partition at :156 and are compared again at :206; ctime-only difference can retire reuse or raise delegation preview source changed during inspection. |
Deferred. Loaded-code freshness/reuse policy requires dedicated coverage. Same snapshot routine does not eliminate the abort. |
capabilities/decision_context/capture_recovery.py:55, _file_fence |
SUSCEPTIBLE through consumers. Path-stat ctime is hashed with profile/reviewed-cursor identity, mtime and digest. :157 and :306 reject profile changes; :228-230 reports state_changed; :315-319 includes the fence in the preview token; :355 rejects rollback against persisted receipt file fences; :456-460 aborts changed apply. Ctime alone changes these decisions. |
Deferred. Recovery preview/execute/rollback fences and historical receipts require a maintained compatibility decision; a recording dictionary is not automatically harmless. |
capabilities/native_chat/file_discovery.py:19, file_change_fingerprint |
NOT SUSCEPTIBLE on Windows. sys.platform == "win32" returns None before stat, disabling metadata reuse. POSIX keeps the tuple; rows re-reads when it differs and declines cache insertion if the second token changes, without rejecting the fresh result. |
Keep existing Windows fresh-reader policy. |
control_plane/testing/model_tool_behavior.py:723, snapshot_disposable_fixture |
SUSCEPTIBLE through consumers. lstat metadata plus content hashes are compared by terminal_settlement_tool_behavior.py:746 (unchanged rejection) and :1026 (failed command mutation guard), via _protected_terminal_fixture. Ctime-only change can reject unchanged-content classification or raise workspace_authoring_failed. |
Deferred. This is a disposable qualification oracle, not a live read helper. Metadata intentionally detects permission/replacement/same-byte rewrites; do not silently weaken it. |
Additional comparison branches reached through those references
| Consumer | Classification and evidence | Disposition |
|---|---|---|
file_lock.py:904-931, _same_effect_file_identity: device/inode and birth-marker branches |
NOT SUSCEPTIBLE to ctime-only skew. With usable dev/ino, return depends on dev/ino and optional birth marker, never ctime. With no dev/ino but a birth marker, only that marker matters. | Preserve replacement/inode-reuse protection. |
Same comparator, final ctime fallback (:931) |
SUSCEPTIBLE conditionally. When both device/inode pairs and birth markers are unavailable, nonzero ctimes are the only remaining identity; descriptor/path skew or across-time skew makes equality fail. Two zero identities fail closed. | Deferred maintainer policy. Removing this last marker is not the materials policy and could make unknown files indistinguishable. |
Lock cleanup/reclamation/release at file_lock.py:941, :962, :1072, :1111 |
SUSCEPTIBLE only through that fallback. Comparisons can decline cleanup, stale reclamation or release. Newly created claim identities originate from fstat (:992), later compared against path stat (:941); other lifecycle checks compare path observations across time. |
Keep token, PID/liveness, age, claim and replacement guards; no Task B change. |
effect_runtime.py:391, source digest key, and :439, settle-time cache write |
NOT SUSCEPTIBLE as a read rejection at these operations alone. Key incorporates the snapshot; a newer ctime can suppress best-effort memo publication. Neither alone rejects source bytes. Separate freshness rechecks above are susceptible. | Leave source-fingerprint/cache policy together for a later scoped fix. |
Cursor save :412 and accepted-cache restore :477; recovery receipt file-fence storage |
NOT SUSCEPTIBLE at serialization/assignment alone. Their later comparisons are explicitly classified susceptible above. | Do not describe persisted state as safe merely because the serialization is recording-only. |
Selected fix and evidence
Only two Task B tracked files change: loopx/control_plane/coordination/cold_source_backup.py and tests/control_plane/test_cold_source_backup_scope.py. The new test calls the real archive-witness reader using a real synthetic tar/manifest and changes only one returned final-descriptor metadata field. Its 2-platform x 5-field cases exercise acceptance on Windows ctime alone, rejection on POSIX ctime, and rejection of dev/ino/size/mtime changes on both policies. The accepted witness retains exact member and archive hashes and archive size.
Before the production edit: uv run --extra test python -m pytest -q tests/control_plane/test_cold_source_backup_scope.py -k archive_identity_change_policy --basetemp=.local/pytest-task-b-red --tb=short -> 1 failed, 9 passed, 12 deselected; only [st_ctime_ns-nt] failed with cold_import_backup_changed_retry.
After the production edit: same selector with --basetemp=.local/pytest-task-b-green -> 10 passed, 12 deselected. Exact broader commands/results and prior materials evidence are retained in pr-decision-context-windows-ctime.md.
Deferred issue and limits
Eight modules retain susceptible or conditional/policy-sensitive consumers: authority source capture, thread activity, rollout cursor, file lock, Effect runtime, delegation preview transport, capture recovery and disposable fixture snapshots. This audit is the explicitly allowed local follow-up issue, not a claim that Windows ctime handling is globally fixed. No public issue was created.
The related run also exposes an unchanged Windows link-test portability failure: the cycle test expects link_unsafe, but _safe_link uses Windows os.path.normpath, producing backslashes that _member_name rejects as member_unsafe before _same_file is reached. A focused check restored the exact parent-commit _same_file implementation in memory and reproduced the same 1 failed, 21 deselected result; no source files were changed. The complete baseline command is in pr-decision-context-windows-ctime.md. The known open-file replacement test remains excluded; FIFO replacement remains POSIX-only. No real POSIX run, live-Harness acceptance or full repository suite is claimed.
Related: #8599 fixes the two sites with a clear descriptor-vs-path comparison (materials.py, cold_source_backup.py). The sites above are left for a maintainer decision because they compare persisted or across-time metadata, or reach the rejection only through a consumer chain.
Activity