diff --git a/plugins/disk-hygiene/.claude-plugin/plugin.json b/plugins/disk-hygiene/.claude-plugin/plugin.json index 325da77b68..4f7b4158f8 100644 --- a/plugins/disk-hygiene/.claude-plugin/plugin.json +++ b/plugins/disk-hygiene/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "disk-hygiene", - "version": "0.41.3", + "version": "0.42.0", "description": "Context-aware disk hygiene for arbitrary directory trees: inventories orphaned and temporary artifacts, classifies evidence into review tiers, and offers exact-path cleanup only after a fresh safety preview and explicit per-tier approval. The target is read-only by default; OS-managed paths, links and mount points, VCS-tracked content without the complete checkout evidence bundle, changed entries, and live-handle uncertainty fail closed.", "author": { "name": "Melodic Software", diff --git a/plugins/disk-hygiene/CHANGELOG.md b/plugins/disk-hygiene/CHANGELOG.md index a58259e3cb..1daed17d7a 100644 --- a/plugins/disk-hygiene/CHANGELOG.md +++ b/plugins/disk-hygiene/CHANGELOG.md @@ -3,6 +3,32 @@ All notable changes to the `disk-hygiene` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.42.0] - 2026-09-30 + +### Added + +- **Catalog scope and an `uncataloged` report** + ([#4008](https://github.com/melodic-software/claude-code-plugins/issues/4008)). `catalog` now + accounts for every immediate child of the target, every hinted or empty entry at any depth, and, + at a user-home or `--root-children` target, every out-of-place immediate child. Entries with no + record and no owner-level ancestor are listed under `uncataloged`; a record marked + `owner_level` covers everything below its path while its identity holds. +- **Required ownership investigation** (`reference/ownership-investigation.md`). Each entry is + checked against nine local sources, each evidence item names its source, `/discovery:research` + is used only when no owner is found and only when it resolves, and the report ends with one + question per entry whose owner is unknown, which stays `keep` until answered. + +### Changed + +- **Operator answers follow the entry, not the scan target.** A `source: human` record whose identity + and descendant set still hold is reused when another scan target reaches the same entry, so it is + not asked again, even where this target holds an engine record with an open question. The scan + sets `target_prior_disposition` when the scan target itself was answered, and the `catalog` + report lists a reused entry under `unchanged` as `answered under `. A new answer replaces + older answers for the same entry recorded under other targets, an owner-level answer reached from + another target covers its descendants, and `catalog` refuses a `--sizes-only` snapshot, which has + no entries to account for. + ## [0.41.3] - 2026-09-30 ### Changed diff --git a/plugins/disk-hygiene/skills/clean/SKILL.md b/plugins/disk-hygiene/skills/clean/SKILL.md index 0db003d8cc..801d119b01 100644 --- a/plugins/disk-hygiene/skills/clean/SKILL.md +++ b/plugins/disk-hygiene/skills/clean/SKILL.md @@ -264,7 +264,8 @@ module name. The file's entry carries a `stdlib-module-shadow` advisory, and the advisory is not a hint and adds no tier. When a shadowing file has a `bytecode_cache`, recommend renaming or moving the source file, since deleting the cache alone is undone by the next import. -For each hinted or suspicious entry, inspect enough neighboring content and metadata to answer: +For each hinted or suspicious entry, and each entry the catalog lists as `uncataloged`, first run the required local procedure in [ownership investigation](reference/ownership-investigation.md) (its sources, how evidence is recorded, and the `/discovery:research` escalation when no owner is found). +Then inspect enough neighboring content and metadata to answer: 1. What created it? Prefer a manifest, log, documented naming contract, sibling structure, or owning tool over an age/name guess. @@ -339,7 +340,7 @@ was never inventoried, so `logical_size` is `null` rather than `0`, except on th partial walked sum alongside a `not-walked` qualifier, so read that number as a floor. Prefer the snapshot's `target_reclaimable_local_bytes` (and preview/apply `reclaimable_local_bytes*`) over summing `logical_size` yourself. Folding qualified or unknown sizes into a total claims space that deleting the path would never return. Never treat a -low or zero reclaimable-byte figure as a reason to skip a finding that otherwise clears the evidence bar. A `prior_disposition` or `prior_unresolved` is a hint, never approval; report and record answers per the [investigated catalog](reference/safety-model.md#investigated-catalog). +low or zero reclaimable-byte figure as a reason to skip a finding that otherwise clears the evidence bar. A `prior_disposition`, `target_prior_disposition` or `prior_unresolved` is a hint, never approval, and an operator answer recorded under another scan target is not asked again while the entry's identity holds; report and record answers per the [investigated catalog](reference/safety-model.md#investigated-catalog). ## 4. Build one exact-tier plan diff --git a/plugins/disk-hygiene/skills/clean/reference/ownership-investigation.md b/plugins/disk-hygiene/skills/clean/reference/ownership-investigation.md new file mode 100644 index 0000000000..972059df3c --- /dev/null +++ b/plugins/disk-hygiene/skills/clean/reference/ownership-investigation.md @@ -0,0 +1,52 @@ +# Ownership investigation + +Run this procedure for every entry the [investigated catalog](safety-model.md#investigated-catalog) +has to account for (hinted, suspicious, or listed under `uncataloged`) before classifying it. It +answers section 2 question 1 (what created it) and question 2 (is the owner active) from evidence on +this machine. Every command is read-only; none kills, pauses or modifies anything. + +## Local sources + +Check each source that applies to the platform. A source that does not apply or turns up nothing is +recorded as checked with no result, not skipped. + +1. **Manifests and READMEs.** `package.json`, `pyproject.toml`, `Cargo.toml`, `*.csproj`, `README*`, + `LICENSE` and similar files in the entry or its nearest parent directories. +2. **Config file contents.** Open the entry's own config files and the owning tool's config + (`~/.config/`, the tool's folder under `%APPDATA%`, `~/Library/Application Support/`). Look for the + entry's path or name. +3. **Command resolution.** Whether a command named like the entry resolves: `Get-Command ` on + Windows, `command -v ` or `which ` on Linux and macOS. +4. **Running processes.** A process whose command line, working directory or open files name the + entry (`Get-Process`, `ps`, `/proc//cwd`, `lsof`). +5. **Scheduled tasks.** Task Scheduler (`Get-ScheduledTask`), cron (`crontab -l`, `/etc/cron.*`) and + systemd timers (`systemctl list-timers --all`, `systemctl --user list-timers --all`) whose action + names the entry. +6. **PATH.** Whether the entry sits on, or is referenced from, the user `PATH` and the machine + `PATH` (on Windows read both scopes, not only the session value). +7. **Installed programs.** The package manager or installer inventory (`winget list`, the Uninstall + registry keys, `dpkg -S`, `rpm -qf`, `brew list`) for a program that owns the path. +8. **Git remotes and status.** When the entry is or contains a repository: `git remote -v`, + `git status --short`, `git log -1`. A remote names the owner; unpushed or dirty state makes the + entry real work. +9. **Dotfile and settings references.** The shell profiles, editor settings and dotfile manager + sources that mention the entry's path or name. + +## Recording evidence + +Each evidence item is one line with its source: the file path it was read from, the exact command that +produced it, or the URL. An item with no source is not evidence. The finding's `owner` and +`provenance` state the conclusion, and each evidence item's source goes into its `evidence` list as +`{"source": ""}`, so the catalog record keeps what the investigation found. The report shows +the same lines beside the conclusion. + +## When no owner is found + +Escalate to `/discovery:research` only when every source above found no owner, and only when that +skill resolves in this session. When it does not resolve, skip the escalation and go to the question +below. `/discovery:explore` applies only to a stray inside a repository, never to a home-directory or +volume-root entry, and only when that skill resolves in this session. When it does not resolve, read +the repository's own files with the local sources above. + +End the report with one question per entry whose owner is still unknown: name the entry, list the +sources checked, and ask who or what owns it. Until the operator answers, that entry stays `keep`. diff --git a/plugins/disk-hygiene/skills/clean/reference/safety-model.md b/plugins/disk-hygiene/skills/clean/reference/safety-model.md index c649c3d0bc..92bb12d920 100644 --- a/plugins/disk-hygiene/skills/clean/reference/safety-model.md +++ b/plugins/disk-hygiene/skills/clean/reference/safety-model.md @@ -874,15 +874,35 @@ file (`{"answers": [...]}`, `source: human`) into the catalog. Both take snapsho - A record is a hint. It records a conclusion, never an approval. Preview and apply do not read it, so a catalogued `remove` still needs the same preview, approval token, and revalidation as an entry that was never catalogued. -- A record belongs to one scan target: the same entry reached from another target is not annotated - and is asked again. -- A changed identity (device, inode, kind) or descendant set invalidates the record. It is replaced - by an unresolved `keep` with its question, and the next scan stops annotating it. +- An operator answer follows the entry, not the scan target. A record with `source: human` whose + identity (device, inode, kind) and descendant set still hold matches the same entry when another + scan target reaches it, whatever path that scan gives it, so the answer is not asked again. The + scan annotates the entry, or sets `target_prior_disposition` when the scan target itself is the + answered entry. A record under this target and path whose identity holds decides, unless it still + has an open question: then an answer recorded under another target replaces it. An engine record + is reused only under its own target and path. + A matched answer is not copied: the next answer recorded under this target becomes its own record + and wins here. The `catalog` report lists a matched entry under `unchanged` as ` | + | | answered under `. +- A changed identity (device, inode, kind) or descendant set invalidates the record. Under its own + target it is replaced by an unresolved `keep` with its question, and the next scan stops + annotating it. An entry reached from another target whose identity or descendant set differs + from the answer is treated as never answered and is asked. - An engine finding with no owner is not a conclusion: the record stays `keep` and the report asks who owns it. Unknown stays visibly unknown, and `prior_unresolved` marks it on the next scan. - An operator answer clears the question with or without an owner, so `{"path": "", "disposition": "keep"}` is a "keep, don't re-raise" answer. While identity holds, later engine findings do not overwrite it and the entry is not asked again. +- The catalog accounts for an entry when it is in scope: every immediate child of the target; every + entry at any depth that is hinted, or empty (`logical_size` 0 with empty `size_qualifiers`); and, + at a user-home or `--root-children` target, every immediate child with empty `protected_reasons` + and no hints (`out-of-place`, the section 2 positional read). Any other deeper entry is ordinary. + Give one record per owning tool or product instead of one per file: a finding or answer with an + `owner` and `"owner_level": true` covers every entry below its path while its identity holds. The + `catalog` output lists each in-scope entry with no record and no owner-level ancestor under + `uncataloged`, with its `reasons`; report every one, so nothing that looks out of place is skipped. + A `--sizes-only` snapshot has no entries, so `catalog` refuses it. The catalog reads only snapshot + fields and walks nothing. - The scan sets `prior_disposition` on an entry whose record still holds. Report new or changed entries first, one line for each unchanged entry, and end with the questions. Records for entries the snapshot did not inventory are kept unchanged. diff --git a/plugins/disk-hygiene/skills/clean/scripts/hygiene.py b/plugins/disk-hygiene/skills/clean/scripts/hygiene.py index c0d3b38de4..f2dbe886b9 100755 --- a/plugins/disk-hygiene/skills/clean/scripts/hygiene.py +++ b/plugins/disk-hygiene/skills/clean/scripts/hygiene.py @@ -341,6 +341,23 @@ def annotate_investigated_catalog(snapshot: dict[str, Any]) -> None: snapshot["catalog_unreadable"] = True +def catalog_target_is_positional(snapshot: dict[str, Any]) -> bool: + """True for a root-children scan or a scan of the user home directory. + + Only there does "no protection and no hint" mark a loose root-level entry + as out of place. + """ + if snapshot.get("root_children_mode"): + return True + home = user_home() + if home is None: + return False + try: + return os.path.samefile(snapshot["target"], home) + except OSError: + return False + + def write_text_atomic(path: Path, text: str) -> None: """Replace ``path`` whole or leave it as it was.""" temporary = path.with_name(f"{path.name}.{secrets.token_hex(4)}.tmp") @@ -5754,6 +5771,11 @@ def main(argv: list[str] | None = None) -> int: raise HygieneError( "catalog needs a scan snapshot whose entries each have a path" ) + if snapshot.get("inventory_mode") == "sizes-only": + raise HygieneError( + "sizes-only snapshot has no entries to catalog; scan without " + "--sizes-only" + ) json_path, markdown_path = catalog_paths() json_path.parent.mkdir(parents=True, exist_ok=True) findings = ( @@ -5774,6 +5796,7 @@ def main(argv: list[str] | None = None) -> int: findings, answers, args.run_id, + positional=catalog_target_is_positional(snapshot), ) write_text_atomic( json_path, json.dumps(merged, indent=2, sort_keys=True) + "\n" @@ -5791,8 +5814,8 @@ def main(argv: list[str] | None = None) -> int: "note": ( "A catalog record is a hint. It does not authorize deletion, " "skip a preview, or shorten approval. Report new_or_changed " - "first, then one line per unchanged entry, and end with the " - "questions." + "first, then one line per unchanged entry, then every " + "uncataloged in-scope entry, and end with the questions." ), } ) diff --git a/plugins/disk-hygiene/skills/clean/scripts/investigated_catalog.py b/plugins/disk-hygiene/skills/clean/scripts/investigated_catalog.py index 727fb4b92c..dfd1eceb99 100644 --- a/plugins/disk-hygiene/skills/clean/scripts/investigated_catalog.py +++ b/plugins/disk-hygiene/skills/clean/scripts/investigated_catalog.py @@ -31,13 +31,20 @@ "question", } ) +# Optional on a record: absent means the record describes its own entry only. +OWNER_LEVEL_KEY = "owner_level" DISPOSITIONS = frozenset({"keep", "remove", "review"}) SOURCES = frozenset({"engine", "human"}) +def _prefix(path: str) -> str: + """The text every descendant path starts with. The scan target is ``.``.""" + return "" if path == "." else f"{path}/" + + def descendant_set(path: str, entries: list[dict[str, Any]]) -> list[str]: """Inventoried paths strictly below ``path``, in stable order.""" - prefix = f"{path}/" + prefix = _prefix(path) return sorted( entry["path"] for entry in entries @@ -70,13 +77,142 @@ def identity_holds( A moved inode or a changed child set means the thing being described is not the thing that was described, so the record is not evidence for this entry. A set that was never walked, on either side, cannot be compared and is not - a change. + a change. Sets compare below their own entry, so a record made from one scan + target still holds for the same entry reached from another. """ if record.get("identity") != identity_of(entry): return False stored = record.get("descendant_set") current = descendants_of(entry, entries) - return stored is None or current is None or stored == current + if stored is None or current is None: + return True + return _below(record["path"], stored) == _below(entry["path"], current) + + +def _below(path: str, descendants: list[str]) -> list[str]: + prefix = _prefix(path) + return [item.removeprefix(prefix) for item in descendants] + + +def _human_by_identity( + records: dict[tuple[str, str], dict[str, Any]], +) -> dict[tuple[Any, Any, Any], list[dict[str, Any]]]: + """Operator answers indexed by the filesystem object they describe. + + A missing device or inode cannot tell one object from another, so such a + record is never reused outside its own target and path. + """ + index: dict[tuple[Any, Any, Any], list[dict[str, Any]]] = {} + for record in records.values(): + identity = record["identity"] + if record["source"] == "human" and identity["device"] and identity["inode"]: + key = (identity["device"], identity["inode"], identity["kind"]) + index.setdefault(key, []).append(record) + return index + + +def _supersede( + records: dict[tuple[str, str], dict[str, Any]], + target: str, + answered: dict[str, Any], +) -> None: + """Drop other targets' operator answers for the object just answered. + + The newest answer is the operator's decision, so an older one for the same + filesystem object must not outrank it on a later scan. + """ + identity = answered["identity"] + if not (identity["device"] and identity["inode"]): + return + for key, record in list(records.items()): + if ( + key[0] != target + and record["source"] == "human" + and record["identity"] == identity + ): + del records[key] + + +def _other_target_answer( + index: dict[tuple[Any, Any, Any], list[dict[str, Any]]], + target: str, + entry: dict[str, Any], + entries: list[dict[str, Any]], + local: dict[str, Any] | None = None, +) -> dict[str, Any] | None: + """An operator answer recorded under another scan target for this same entry. + + ``local`` is this target's record for the entry when its identity holds. It + settles the entry unless it still has an open question, which an answer from + another target retires. + """ + if local is not None and not local["question"]: + return None + identity = identity_of(entry) + for record in index.get( + (identity["device"], identity["inode"], identity["kind"]), [] + ): + if record["target"] != target and identity_holds(record, entry, entries): + return record + return None + + +def catalog_scope(snapshot: dict[str, Any], positional: bool) -> dict[str, list[str]]: + """The entries a catalog must account for, each with why it is in scope. + + Every immediate child; every hinted or genuinely empty entry at any depth; + and, at a user-home or root-children target (``positional``), every + immediate child with no protection and no hint, the loose entry that + belongs to no recognizable convention. Any other deeper entry is ordinary + and is covered at owner level instead. Only snapshot fields are read. + """ + scope: dict[str, list[str]] = {} + for entry in snapshot.get("entries") or []: + path = entry["path"] + hinted = bool(entry.get("hints")) + reasons = [] + if "/" not in path: + reasons.append("immediate-child") + if positional and not hinted and not entry.get("protected_reasons"): + reasons.append("out-of-place") + if hinted: + reasons.append("hinted") + if _size(entry) == 0 and not entry.get("size_qualifiers"): + reasons.append("empty") + if reasons: + scope[path] = reasons + return scope + + +def _uncataloged( + target: str, + scope: dict[str, list[str]], + records: dict[tuple[str, str], dict[str, Any]], + reused: dict[str, dict[str, Any]], + target_owned: bool = False, +) -> list[dict[str, Any]]: + """In-scope entries with no record of their own and no owner-level ancestor. + + ``target_owned`` is an owner-level answer for the scan target itself, which + covers every entry below it. + """ + if target_owned: + return [] + owners = { + path + for (record_target, path), record in records.items() + if record_target == target and record.get(OWNER_LEVEL_KEY) + } | {path for path, answer in reused.items() if answer.get(OWNER_LEVEL_KEY)} + + def covered(path: str) -> bool: + parts = path.split("/") + return any("/".join(parts[:end]) in owners for end in range(1, len(parts))) + + return [ + {"path": path, "reasons": reasons} + for path, reasons in sorted(scope.items()) + if (target, path) not in records and path not in reused and not covered(path) + ] def _question(path: str) -> str: @@ -141,9 +277,14 @@ def _apply(record: dict[str, Any], conclusion: dict[str, Any], source: str) -> N An engine conclusion without an owner is not a conclusion: the record stays ``keep`` and keeps its question. An operator answer always retires the question, with or without an owner, and so is not asked again while identity - holds. + holds. ``owner_level: true`` with an owner makes the record cover every + entry below its path: one record per owning tool, not one per file. """ owner = _text(conclusion.get("owner")) + if owner and conclusion.get(OWNER_LEVEL_KEY) is True: + record[OWNER_LEVEL_KEY] = True + else: + record.pop(OWNER_LEVEL_KEY, None) unresolved = source == "engine" and owner is None disposition = conclusion.get("disposition") if unresolved or disposition not in DISPOSITIONS: @@ -185,6 +326,10 @@ def _well_formed(record: Any) -> bool: and isinstance(record["path"], str) and isinstance(record["identity"], dict) and record["identity"].keys() == {"device", "inode", "kind"} + and all( + value is None or isinstance(value, (str, int)) + for value in record["identity"].values() + ) and ( record["descendant_set"] is None or ( @@ -192,6 +337,7 @@ def _well_formed(record: Any) -> bool: and all(isinstance(item, str) for item in record["descendant_set"]) ) ) + and isinstance(record.get(OWNER_LEVEL_KEY, False), bool) and record["evidence"] == _evidence(record["evidence"]) and record["disposition"] in DISPOSITIONS and (record["tier"] is None or record["tier"] in TIERS) @@ -213,24 +359,32 @@ def sync_catalog( findings: list[dict[str, Any]], answers: list[dict[str, Any]], run_id: str, + positional: bool = False, ) -> tuple[dict[str, Any], dict[str, Any]]: """Merge findings and operator answers into the catalog. Return it and the report. Records for entries this snapshot did not inventory are kept as they are: not seen is not changed. A record for an entry that was inventoried, and whose identity or descendant set changed, is replaced by an unresolved one, - so its question returns. + so its question returns. The report lists every in-scope entry that has no + record, so nothing that looks out of place is skipped without a trace. """ target = str(snapshot["target"]) entries = snapshot.get("entries") or [] by_path = {entry["path"]: entry for entry in entries} records = _records_by_key(existing) + answers_elsewhere = _human_by_identity(records) state: dict[str, str] = {} + reused: dict[str, dict[str, Any]] = {} for path, entry in by_path.items(): previous = records.get((target, path)) - if previous is None: - continue - if identity_holds(previous, entry, entries): + holds = previous is not None and identity_holds(previous, entry, entries) + if answer := _other_target_answer( + answers_elsewhere, target, entry, entries, previous if holds else None + ): + records.pop((target, path), None) + reused[path] = answer + elif holds: stored = previous["descendant_set"] records[(target, path)] = { **previous, @@ -242,7 +396,7 @@ def sync_catalog( "last_verified": run_id, } state[path] = "unchanged" - else: + elif previous is not None: records[(target, path)] = _unresolved_record( target, entry, entries, run_id, previous ) @@ -254,6 +408,8 @@ def sync_catalog( if not isinstance(path, str) or path not in by_path: unmatched.append(str(path)) continue + if source == "engine" and path in reused: + continue if (target, path) not in records: records[(target, path)] = _unresolved_record( target, by_path[path], entries, run_id, None @@ -268,31 +424,54 @@ def sync_catalog( continue before = dict(record) _apply(record, conclusion, source) + if source == "human": + _supersede(records, target, record) if state[path] == "unchanged" and record != before: state[path] = "changed" catalog = { "version": CATALOG_VERSION, "records": [records[key] for key in sorted(records)], } - return catalog, _report(target, records, state, unmatched) + identity = snapshot.get("target_identity") + target_answer = ( + _other_target_answer( + answers_elsewhere, target, {**identity, "path": "."}, entries + ) + if isinstance(identity, dict) + else None + ) + uncataloged = _uncataloged( + target, + catalog_scope(snapshot, positional), + records, + reused, + bool(target_answer and target_answer.get(OWNER_LEVEL_KEY)), + ) + return catalog, _report(target, records, state, reused, unmatched, uncataloged) def _report( target: str, records: dict[tuple[str, str], dict[str, Any]], state: dict[str, str], + reused: dict[str, dict[str, Any]], unmatched: list[str], + uncataloged: list[dict[str, Any]], ) -> dict[str, Any]: - """New or changed entries first, unchanged entries one line each.""" + """New or changed entries first, unchanged entries one line each. + + An entry matched to an operator answer recorded under another scan target + has no record of its own here. It is unchanged, and its line names the + target that holds the answer. + """ new_or_changed = [] unchanged = [] questions = [] for path in sorted(state): record = records[(target, path)] if state[path] == "unchanged": - unchanged.append( - f"{path} | {record['disposition']} | {record['owner'] or 'unknown'}" - ) + owner = record["owner"] or "unknown" + unchanged.append((path, f"{path} | {record['disposition']} | {owner}")) else: new_or_changed.append( { @@ -306,10 +485,21 @@ def _report( ) if record["question"]: questions.append({"path": path, "question": record["question"]}) + for path, answer in reused.items(): + if path not in state: + owner = answer["owner"] or "unknown" + unchanged.append( + ( + path, + f"{path} | {answer['disposition']} | {owner}" + f" | answered under {answer['target']}", + ) + ) return { "new_or_changed": new_or_changed, - "unchanged": unchanged, + "unchanged": [line for _, line in sorted(unchanged)], "questions": questions, + "uncataloged": uncataloged, "unmatched": unmatched, } @@ -317,6 +507,11 @@ def _report( def annotate_entries(snapshot: dict[str, Any], catalog: dict[str, Any]) -> None: """Set ``prior_disposition`` on entries whose record identity still holds. + The record under this target and path decides first, unless it still has an + open question. An operator answer recorded under another scan target + matches the same entry by identity and decides in its place, and the scan + target itself, which is not an entry, gets ``target_prior_disposition``. + The field is a report hint. It is not an approval and no plan copies it. ``prior_unresolved`` marks a record that still has an open question, so an unknown owner does not read as a settled keep. @@ -324,15 +519,30 @@ def annotate_entries(snapshot: dict[str, Any], catalog: dict[str, Any]) -> None: target = str(snapshot.get("target")) entries = snapshot.get("entries") or [] records = _records_by_key(catalog) + answers_elsewhere = _human_by_identity(records) for entry in entries: - record = records.get((target, entry["path"])) entry.pop("prior_disposition", None) entry.pop("prior_unresolved", None) - if record is None or not identity_holds(record, entry, entries): + record = records.get((target, entry["path"])) + if record is not None and not identity_holds(record, entry, entries): + record = None + record = ( + _other_target_answer(answers_elsewhere, target, entry, entries, record) + or record + ) + if record is None: continue entry["prior_disposition"] = record.get("disposition") if record.get("question"): entry["prior_unresolved"] = True + snapshot.pop("target_prior_disposition", None) + identity = snapshot.get("target_identity") + if isinstance(identity, dict): + answer = _other_target_answer( + answers_elsewhere, target, {**identity, "path": "."}, entries + ) + if answer is not None: + snapshot["target_prior_disposition"] = answer["disposition"] def render_markdown(catalog: dict[str, Any], report: dict[str, Any]) -> str: @@ -363,6 +573,11 @@ def render_markdown(catalog: dict[str, Any], report: dict[str, Any]) -> str: ] lines += ["### Unchanged", ""] lines += [f"- {line}" for line in report["unchanged"]] or ["None."] + lines += ["", "### Uncataloged", ""] + lines += [ + f"- `{item['path']}` ({', '.join(item['reasons'])})" + for item in report["uncataloged"] + ] or ["None."] lines += ["", "### Questions", ""] lines += [ f"{index}. `{item['path']}`: {item['question']}" @@ -376,6 +591,11 @@ def render_markdown(catalog: dict[str, Any], report: dict[str, Any]) -> str: "", f"- target: `{record['target']}`", f"- owner: {record['owner'] or 'unknown'}", + *( + ["- owner level: covers every entry below"] + if record.get(OWNER_LEVEL_KEY) + else [] + ), f"- disposition: {record['disposition']}", f"- tier: {record['tier']}", f"- size: {record['size']}", diff --git a/plugins/disk-hygiene/skills/clean/scripts/test_investigated_catalog.py b/plugins/disk-hygiene/skills/clean/scripts/test_investigated_catalog.py index 24287f7e5e..81682e468b 100644 --- a/plugins/disk-hygiene/skills/clean/scripts/test_investigated_catalog.py +++ b/plugins/disk-hygiene/skills/clean/scripts/test_investigated_catalog.py @@ -54,6 +54,7 @@ def test_identity_or_descendant_change_invalidates(self) -> None: _entry("cache/keep", inode=4, kind="file", logical_size=1), ] record = { + "path": "cache", "identity": catalog.identity_of(entries[0]), "descendant_set": catalog.descendant_set("cache", entries), } @@ -183,6 +184,139 @@ def test_records_outside_this_snapshot_survive(self) -> None: partial, _ = catalog.sync_catalog(_snapshot(), merged, [], [], "run-j") self.assertEqual(2, len(partial["records"])) + def test_operator_answer_is_reused_by_identity_from_another_target(self) -> None: + first = _snapshot( + _entry("esupport", inode=20), + _entry("esupport/a", inode=21, kind="file"), + target="/root", + ) + answered, _ = catalog.sync_catalog( + first, None, [], [{"path": "esupport", "disposition": "keep"}], "run-o" + ) + reached = _snapshot( + _entry("vendor/esupport", inode=20), + _entry("vendor/esupport/a", inode=21, kind="file"), + target="/other", + ) + catalog.annotate_entries(reached, answered) + self.assertEqual("keep", reached["entries"][0]["prior_disposition"]) + again, report = catalog.sync_catalog( + reached, + answered, + [_finding("vendor/esupport", owner=None, disposition="remove")], + [], + "run-p", + ) + self.assertEqual([], report["questions"]) + self.assertEqual(answered, again) + own = _snapshot(_entry("a", inode=21, kind="file"), target="/root/esupport") + own["target_identity"] = {**_entry("esupport", inode=20), "size_qualifiers": []} + catalog.annotate_entries(own, answered) + self.assertEqual("keep", own["target_prior_disposition"]) + own["target_identity"]["inode"] = 99 + catalog.annotate_entries(own, answered) + self.assertNotIn("target_prior_disposition", own) + + def test_an_answer_reused_from_another_target_is_reported_unchanged(self) -> None: + answered, _ = catalog.sync_catalog( + _snapshot(_entry("esupport", inode=20), target="/root"), + None, + [], + [{"path": "esupport", "owner": "eSupport", "disposition": "keep"}], + "run-t", + ) + reached = _snapshot(_entry("esupport", inode=20), target="/other") + again, report = catalog.sync_catalog(reached, answered, [], [], "run-u") + line = "esupport | keep | eSupport | answered under /root" + self.assertEqual([line], report["unchanged"]) + for key in ("new_or_changed", "questions", "uncataloged"): + self.assertEqual([], report[key]) + self.assertIn(f"- {line}", catalog.render_markdown(again, report)) + _, local = catalog.sync_catalog( + reached, + answered, + [], + [{"path": "esupport", "disposition": "remove"}], + "run-v", + ) + self.assertEqual([], local["unchanged"]) + self.assertEqual(["new"], [item["state"] for item in local["new_or_changed"]]) + + def test_an_open_question_yields_to_an_answer_from_another_target(self) -> None: + asked, first = catalog.sync_catalog( + _snapshot(_entry("esupport", inode=20), target="/a"), + None, + [_finding("esupport", owner=None)], + [], + "run-w", + ) + self.assertEqual(["esupport"], [item["path"] for item in first["questions"]]) + answered, _ = catalog.sync_catalog( + _snapshot(_entry("esupport", inode=20), target="/b"), + asked, + [], + [{"path": "esupport", "owner": "eSupport", "disposition": "keep"}], + "run-x", + ) + rescan = _snapshot(_entry("esupport", inode=20), target="/a") + catalog.annotate_entries(rescan, answered) + self.assertEqual("keep", rescan["entries"][0]["prior_disposition"]) + self.assertNotIn("prior_unresolved", rescan["entries"][0]) + again, report = catalog.sync_catalog( + rescan, answered, [_finding("esupport", owner=None)], [], "run-y" + ) + self.assertEqual([], report["questions"]) + self.assertEqual( + ["esupport | keep | eSupport | answered under /b"], report["unchanged"] + ) + self.assertEqual(["/b"], [record["target"] for record in again["records"]]) + + def test_an_open_question_stays_when_no_other_target_has_an_answer(self) -> None: + asked, _ = catalog.sync_catalog( + _snapshot(_entry("esupport", inode=20), target="/a"), + None, + [_finding("esupport", owner=None)], + [], + "run-z", + ) + rescan = _snapshot(_entry("esupport", inode=20), target="/a") + catalog.annotate_entries(rescan, asked) + self.assertTrue(rescan["entries"][0]["prior_unresolved"]) + _, report = catalog.sync_catalog(rescan, asked, [], [], "run-aa") + self.assertEqual(["esupport"], [item["path"] for item in report["questions"]]) + + def test_identity_change_under_another_target_invalidates_the_answer(self) -> None: + first = _snapshot(_entry("esupport", inode=20), target="/root") + answered, _ = catalog.sync_catalog( + first, None, [], [{"path": "esupport", "disposition": "keep"}], "run-q" + ) + moved = _snapshot(_entry("esupport", inode=77), target="/other") + catalog.annotate_entries(moved, answered) + self.assertNotIn("prior_disposition", moved["entries"][0]) + grown = _snapshot( + _entry("esupport", inode=20), + _entry("esupport/new", inode=5, kind="file"), + target="/other", + ) + catalog.annotate_entries(grown, answered) + self.assertNotIn("prior_disposition", grown["entries"][0]) + _, report = catalog.sync_catalog( + moved, answered, [_finding("esupport", owner=None)], [], "run-r" + ) + self.assertEqual(["esupport"], [item["path"] for item in report["questions"]]) + + def test_engine_records_are_not_reused_across_targets(self) -> None: + stored, _ = catalog.sync_catalog( + _snapshot(_entry("esupport", inode=20), target="/root"), + None, + [_finding("esupport")], + [], + "run-s", + ) + other = _snapshot(_entry("esupport", inode=20), target="/other") + catalog.annotate_entries(other, stored) + self.assertNotIn("prior_disposition", other["entries"][0]) + def test_malformed_records_are_ignored_not_fatal(self) -> None: snapshot = _snapshot(_entry("scratch")) good, _ = catalog.sync_catalog( @@ -263,6 +397,156 @@ def test_rendered_markdown_leads_with_changes_and_ends_with_questions(self) -> N self.assertIn("- owner: unknown", text) +class CatalogScopeTest(unittest.TestCase): + def snapshot(self) -> dict: + return _snapshot( + _entry("tool", inode=1), + _entry("tool/data", inode=2, logical_size=9), + _entry("tool/data/blob.bin", kind="file", inode=3, logical_size=9), + {**_entry("tool/data/cache", inode=4, logical_size=9), "hints": ["cache"]}, + _entry("tool/empty", inode=5, logical_size=0), + { + **_entry("tool/cut", inode=6, logical_size=0), + "size_qualifiers": ["not-walked"], + }, + {**_entry("Library", inode=7), "protected_reasons": ["shell-folder"]}, + { + **_entry("Library/x", kind="file", inode=8), + "protected_reasons": ["shell-folder"], + }, + ) + + def test_scope_names_each_entry_shape(self) -> None: + scope = catalog.catalog_scope(self.snapshot(), positional=True) + self.assertEqual( + { + "tool": ["immediate-child", "out-of-place"], + "tool/data/cache": ["hinted"], + "tool/empty": ["empty"], + "Library": ["immediate-child"], + }, + scope, + ) + + def test_out_of_place_needs_a_home_or_root_children_target(self) -> None: + scope = catalog.catalog_scope(self.snapshot(), positional=False) + self.assertEqual(["immediate-child"], scope["tool"]) + + def test_an_in_scope_entry_without_a_record_is_reported(self) -> None: + _, report = catalog.sync_catalog( + self.snapshot(), None, [_finding("tool")], [], "run-a", positional=True + ) + self.assertEqual( + ["Library", "tool/data/cache", "tool/empty"], + [item["path"] for item in report["uncataloged"]], + ) + self.assertEqual(["hinted"], report["uncataloged"][1]["reasons"]) + + def test_an_owner_level_record_covers_its_descendants(self) -> None: + stored, report = catalog.sync_catalog( + self.snapshot(), + None, + [_finding("tool", owner_level=True), _finding("Library")], + [], + "run-b", + ) + self.assertTrue(stored["records"][1]["owner_level"]) + self.assertEqual([], report["uncataloged"]) + self.assertIn("owner level", catalog.render_markdown(stored, report)) + + def test_an_owner_level_marker_needs_an_owner(self) -> None: + stored, report = catalog.sync_catalog( + self.snapshot(), + None, + [_finding("tool", owner=None, owner_level=True), _finding("Library")], + [], + "run-c", + ) + self.assertNotIn("owner_level", stored["records"][1]) + self.assertEqual( + ["tool/data/cache", "tool/empty"], + [item["path"] for item in report["uncataloged"]], + ) + + def test_a_record_that_lost_identity_stops_covering(self) -> None: + snapshot = self.snapshot() + stored, _ = catalog.sync_catalog( + snapshot, None, [_finding("tool", owner_level=True)], [], "run-d" + ) + snapshot["entries"][0]["inode"] = 99 + _, report = catalog.sync_catalog(snapshot, stored, [], [], "run-e") + self.assertIn("tool/empty", [item["path"] for item in report["uncataloged"]]) + + def test_a_human_answer_from_another_target_counts_as_a_record(self) -> None: + elsewhere = {**self.snapshot(), "target": "/other"} + stored, _ = catalog.sync_catalog( + elsewhere, None, [], [{"path": "tool", "disposition": "keep"}], "run-f" + ) + _, report = catalog.sync_catalog(self.snapshot(), stored, [], [], "run-g") + self.assertNotIn("tool", [item["path"] for item in report["uncataloged"]]) + + def owner_answer(self) -> dict: + elsewhere = {**self.snapshot(), "target": "/other"} + answer = { + "path": "tool", + "disposition": "keep", + "owner": "tool vendor", + "owner_level": True, + } + stored, _ = catalog.sync_catalog(elsewhere, None, [], [answer], "run-h") + return stored + + def test_a_reused_owner_level_answer_covers_its_descendants(self) -> None: + reached = _snapshot( + *( + {**entry, "path": f"vendor/{entry['path']}"} + for entry in self.snapshot()["entries"] + if entry["path"].startswith("tool") + ), + ) + _, report = catalog.sync_catalog(reached, self.owner_answer(), [], [], "run-i") + self.assertEqual([], report["uncataloged"]) + + def test_a_reused_owner_level_answer_for_the_target_covers_every_entry( + self, + ) -> None: + inside = _snapshot( + *( + {**entry, "path": entry["path"].removeprefix("tool/")} + for entry in self.snapshot()["entries"] + if entry["path"].startswith("tool/") + ), + ) + inside["target_identity"] = _entry("tool", inode=1) + _, report = catalog.sync_catalog(inside, self.owner_answer(), [], [], "run-j") + self.assertEqual([], report["uncataloged"]) + + def test_the_latest_answer_for_an_object_supersedes_older_ones(self) -> None: + first = _snapshot(_entry("loose", inode=30), target="/a-first") + second = _snapshot(_entry("loose", inode=30), target="/z-second") + stored, _ = catalog.sync_catalog( + first, None, [], [{"path": "loose", "disposition": "keep"}], "run-k" + ) + stored, _ = catalog.sync_catalog( + second, stored, [], [{"path": "loose", "disposition": "remove"}], "run-l" + ) + third = _snapshot(_entry("loose", inode=30), target="/m-third") + catalog.annotate_entries(third, stored) + self.assertEqual("remove", third["entries"][0]["prior_disposition"]) + + def test_an_identity_with_a_non_scalar_value_is_ignored(self) -> None: + snapshot = _snapshot(_entry("loose")) + good, _ = catalog.sync_catalog( + snapshot, None, [], [{"path": "loose", "disposition": "keep"}], "run-m" + ) + record = good["records"][0] + record["identity"] = {**record["identity"], "inode": [1]} + catalog.annotate_entries(snapshot, good) + self.assertNotIn("prior_disposition", snapshot["entries"][0]) + _, report = catalog.sync_catalog(snapshot, good, [], [], "run-n") + self.assertEqual("loose", report["uncataloged"][0]["path"]) + + class CatalogCommandTest(unittest.TestCase): def setUp(self) -> None: self.tmp = tempfile.TemporaryDirectory() @@ -330,6 +614,56 @@ def test_catalog_writes_both_files_and_scan_annotates(self) -> None: self.assertEqual("remove", annotated["prior_disposition"]) self.assertTrue(self.item.is_file()) + def test_catalog_output_lists_the_uncataloged_in_scope_entries(self) -> None: + snapshot = self.scan("snapshot.json") + code, result = self.run_main( + "catalog", "--snapshot", str(snapshot), "--run-id", "run-1" + ) + self.assertEqual(0, code) + self.assertEqual( + [{"path": "loose.txt", "reasons": ["immediate-child"]}], + result["uncataloged"], + ) + self.assertIn( + "### Uncataloged\n\n- `loose.txt`", + (self.data / "CATALOG.md").read_text(encoding="utf-8"), + ) + + def test_a_home_target_marks_loose_root_entries_out_of_place(self) -> None: + snapshot = self.scan("snapshot.json") + with mock.patch.object(hygiene, "user_home", return_value=self.target): + code, result = self.run_main( + "catalog", "--snapshot", str(snapshot), "--run-id", "run-1" + ) + self.assertEqual(0, code) + self.assertEqual( + ["immediate-child", "out-of-place"], result["uncataloged"][0]["reasons"] + ) + + def test_a_home_target_spelled_through_a_link_is_still_the_home(self) -> None: + link = Path(self.tmp.name) / "home-link" + link.symlink_to(self.target, target_is_directory=True) + snapshot = json.loads(self.scan("snapshot.json").read_text(encoding="utf-8")) + snapshot["target"] = str(link) + path = self.write("linked.json", snapshot) + with mock.patch.object(hygiene, "user_home", return_value=self.target): + code, result = self.run_main( + "catalog", "--snapshot", str(path), "--run-id", "run-1" + ) + self.assertEqual(0, code) + self.assertIn("out-of-place", result["uncataloged"][0]["reasons"]) + + def test_catalog_refuses_a_sizes_only_snapshot(self) -> None: + snapshot = self.write( + "sizes.json", + {"target": str(self.target), "entries": [], "inventory_mode": "sizes-only"}, + ) + code, _ = self.run_main( + "catalog", "--snapshot", str(snapshot), "--run-id", "run-1" + ) + self.assertNotEqual(0, code) + self.assertFalse((self.data / "catalog.json").exists()) + def test_operator_answer_file_is_persisted_as_human_source(self) -> None: snapshot = self.catalog_remove() answers = self.write(