diff --git a/graphify/detect.py b/graphify/detect.py index 2c9eb01a4..7e03e419f 100644 --- a/graphify/detect.py +++ b/graphify/detect.py @@ -2166,6 +2166,45 @@ def _to_absolute_from_storage(key: str, root: Path) -> str: return _nfc(str(Path(root).resolve() / p)) +def _collapse_manifest_duplicates(items, key_fn) -> dict: + """Canonicalize each raw key via ``key_fn``, keeping the more recently + observed entry when two distinct raw keys collapse to the same + canonical one (#1964). + + A manifest written across a mix of graphify versions/call sites — some + passing ``root`` to relativize keys, some not (an outdated installed + skill runbook, for one) — can end up with both an absolute and a + relative key for the same file, each carrying different data. A plain + dict comprehension over such a manifest silently keeps whichever raw + key happens to iterate last, an artifact of on-disk JSON key order that + has nothing to do with which entry is actually current — so a fresher + hash could be discarded in favor of a stale one purely because of how + the keys happened to accumulate, letting detect_incremental() call a + genuinely changed file unchanged. + + Ties are broken by each entry's own ``seen`` timestamp (when present on + both sides and they disagree) rather than iteration order, since that + field exists specifically to record when an entry was last confirmed + current. Legacy scalar/partial entries (no ``seen``, or either side not + a dict) fall back to the historical last-wins behavior, unchanged. + """ + result: dict = {} + for k, v in items: + canonical = key_fn(k) + if canonical in result: + prev = result[canonical] + prev_seen = prev.get("seen") if isinstance(prev, dict) else None + new_seen = v.get("seen") if isinstance(v, dict) else None + if ( + isinstance(prev_seen, (int, float)) + and isinstance(new_seen, (int, float)) + and new_seen < prev_seen + ): + continue + result[canonical] = v + return result + + def load_manifest( manifest_path: str = _MANIFEST_PATH, *, @@ -2189,8 +2228,10 @@ def load_manifest( if not isinstance(raw, dict): return raw if root is None: - return {_nfc(k): v for k, v in raw.items()} - return {_nfc(_to_absolute_from_storage(k, root)): v for k, v in raw.items()} + return _collapse_manifest_duplicates(raw.items(), _nfc) + return _collapse_manifest_duplicates( + raw.items(), lambda k: _nfc(_to_absolute_from_storage(k, root)) + ) def save_manifest( @@ -2397,10 +2438,15 @@ def save_manifest( # their absolute form so the manifest round-trips on the saving # machine even when not every entry can be portably encoded. # NFC after relativize so on-disk keys match what load_manifest - # re-anchors and compares against (#2221). - manifest = {_nfc(_to_relative_for_storage(k, root)): v for k, v in manifest.items()} + # re-anchors and compares against (#2221). A seeded absolute key and + # a seeded relative key for the same file collapse here too (#1964) + # -- _collapse_manifest_duplicates keeps whichever is more recently + # seen instead of whichever the dict comprehension iterates last. + manifest = _collapse_manifest_duplicates( + manifest.items(), lambda k: _nfc(_to_relative_for_storage(k, root)) + ) else: - manifest = {_nfc(k): v for k, v in manifest.items()} + manifest = _collapse_manifest_duplicates(manifest.items(), _nfc) # Avoid rewriting manifest.json when the serialized payload is identical (#2838). manifest_p = Path(manifest_path)