🐛 - Bugbot flags intentional retained partial snapshot evidence despite the explicit recovery contract #832

Is there an existing issue for this?

• I have searched the existing issues.

The search returned unrelated release/extension issues, not this reported behavior. This is a review grounding problem separate from the Python 3.14 false positives in #831.

Which actions are affected?

Pull Request

Which platforms are affected?

Linux
macOS

Description

Bugbot flags deliberate preservation of an incomplete snapshot destination as a reliability defect and recommends removing the directory on failure. The same reviewed change explicitly requires preserving partial diagnostic/uncertain-publication evidence, never overwriting an existing destination and leaving deliberate owned cleanup to the operator. A dedicated real-file regression verifies this exact contract.

Expected: distinguish an intentional inspect-before-cleanup recovery policy from a missing required cleanup. Do not suggest destructive cleanup contrary to the explicit operator contract. A genuine leaked disposable artifact remains reportable when cleanup is actually required and its ownership/publication state is established.

Actual: the low-severity 94%-confidence finding calls retention of the partial destination a defect and recommends deletion without addressing the contrary contract or test. The runtime behaves as specified; its recovery contract is not an unhandled omission.

Reproducing the issue

1. Consumer PR: Implement HA identity, SQLite persistence and verified restoration domus#7; exact analyzed head d67475a92792a109eef67df1c1e98039a6bcd43f.
2. Review run/job: https://github.com/vypdev/domus/actions/runs/41228420104/job/124617336749; native check https://github.com/vypdev/domus/runs/124619181706.
3. Finding: Implement HA identity, SQLite persistence and verified restoration domus#7 (comment), file backend/src/domus/infrastructure/sqlite_snapshots.py:282, finding ID sqlite_snapshots.create.partial-destination.
4. Read docs/snapshots.md at that same head, lines 39–44. It specifies manifest-last publication, preservation of an incomplete failed destination, no overwrite and explicit inspection before deliberate owned cleanup. It also distinguishes output failure after publication from no effect.
5. Read tests/backend/test_sqlite_snapshots.py::test_failed_copy_has_no_valid_manifest_and_a_retry_cannot_replace_partial_evidence at the same head. It injects copy failure, asserts the directory remains without a success manifest, requires inspection to reject the incomplete bundle, requires retry not to replace it, and proves source household state is unchanged.
6. Independent exact-head CI Check passes: https://github.com/vypdev/domus/actions/runs/41228420124. This includes all real-image jobs and 1387 backend tests at 100% supported statement/branch coverage. Later legitimate origin/approval-replay fixes add 20 cases; snapshot retention behavior is unchanged.

This is an observed real-model false-positive candidate with explicit contrary requirements/tests. It is not a claim that every failed directory must always be retained or that all review findings are false.

copilot Version

vypdev/copilot@v3 resolved to 3e7ceea; codex-cli 0.166.0, reviewer model gpt-6-luna, smart effort.

Relevant Log Output

The published finding says: Failed snapshot creation leaves a permanent partial destination. It recommends removing the newly created destination on failure. Its evidence is the lack of exception cleanup and subsequent snapshot_destination_exists rejection; it does not discuss the explicit retained-evidence policy or matching test.

Review summary: open=8, fixed=1, dismissed=2; review partial, effort=smart, 413723ms. All diff sources were retained (205 files), but human-conversation context retained 6 and omitted 5. That partial context is reported honestly and is not accepted as complete review.

Additional context and comments

• Immutable contract: https://github.com/vypdev/domus/blob/d67475a92792a109eef67df1c1e98039a6bcd43f/docs/snapshots.md
• Exact regression: https://github.com/vypdev/domus/blob/d67475a92792a109eef67df1c1e98039a6bcd43f/tests/backend/test_sqlite_snapshots.py
• Preserve inspection and source/partial-output evidence; do not silently erase uncertain publication, recursively remove a path later modified by another actor, or weaken existing no-overwrite guarantees to clear the finding.
• Other valid findings from this review are being fixed normally: default-port origin normalization, receipt replay after recovery consumption, OpenAPI callback XOR documentation and stale operator status text. No Copilot implementation or installed resource is modified.
• Regression controls should contrast this explicit retain/inspect contract with an explicitly required, safely owned disposable-cleanup contract and a failure after successful publication. The model should trace the selected contract rather than assume one universal cleanup policy.
• No owner HA data, credentials, session routes or private identity payloads are included. PR remains unmerged pending full current-head coverage/review without unresolved findings.