190 lines
9.0 KiB
Markdown
190 lines
9.0 KiB
Markdown
---
|
|
phase: "13"
|
|
plan: "09"
|
|
subsystem: cloud-operations
|
|
status: complete
|
|
tags:
|
|
- cloud
|
|
- move
|
|
- delete
|
|
- stale-guard
|
|
- descendant-safety
|
|
- reconciliation
|
|
- audit
|
|
- disclosure
|
|
- tdd
|
|
|
|
dependency_graph:
|
|
requires:
|
|
- "13-08 (create-folder and rename collision, stale guard, reconciliation)"
|
|
- "13-06 (upload reconcile-before-return pattern)"
|
|
- "13-04 (operations.py route layer + typed result vocabulary)"
|
|
provides:
|
|
- "Move with same-connection validation, descendant safety, stale guard, reconciliation (D-07, D-08, D-09, T-13-27, T-13-28)"
|
|
- "Delete with folder disclosure, parent folder invalidation, metadata-only audit (D-10, D-11, T-13-29)"
|
|
- "Metadata-only audit rows for move (cloud.item_moved) and delete (cloud.item_deleted)"
|
|
- "11 new behavioral tests in test_cloud_mutations.py"
|
|
- "2 xfail promotions in test_cloud_audit.py (move and delete audit rows)"
|
|
affects:
|
|
- "13-10 through 13-11 (remaining plans)"
|
|
|
|
tech_stack:
|
|
added:
|
|
- "_is_descendant_destination() helper in operations.py — ancestor-chain walk via cloud_items"
|
|
- "Descendant destination check in move_cloud_item before adapter call (D-09)"
|
|
- "Stale result handler in move_cloud_item: update_folder_state + typed stale body (D-07)"
|
|
- "Move success: upsert_cloud_item with new parent_ref (reconcile-before-return)"
|
|
- "Move success: update_folder_state for both source and destination folders"
|
|
- "Move success: write_audit_log 'cloud.item_moved' with metadata-only payload"
|
|
- "Delete: pre-fetch item kind and parent_ref for D-10 disclosure"
|
|
- "Delete success: update_folder_state for parent folder"
|
|
- "Delete success: write_audit_log 'cloud.item_deleted' with delete_kind metadata"
|
|
- "Delete response carries is_folder=True / item_kind for frontend disclosure (D-10)"
|
|
patterns:
|
|
- "Reconcile-before-return: upsert + folder invalidation + audit before success response"
|
|
- "Descendant-safe move: ancestor-chain walk through cloud_items metadata (no provider call)"
|
|
- "Stale-guard: folder state invalidation on stale result before returning typed stale body"
|
|
- "Metadata-only audit: connection_id, provider_item_id, operational metadata — never tokens or bytes"
|
|
- "Folder disclosure: is_folder + item_kind in delete response (D-10)"
|
|
|
|
key_files:
|
|
modified:
|
|
- path: "backend/api/cloud/operations.py"
|
|
change: "move_cloud_item: descendant check, stale guard, reconcile-before-return, audit; delete_cloud_item: pre-fetch metadata, folder invalidation, disclosure, audit"
|
|
- path: "backend/tests/test_cloud_mutations.py"
|
|
change: "Added 11 behavioral tests (RED then GREEN) for Plan 09"
|
|
- path: "backend/tests/test_cloud_audit.py"
|
|
change: "Promoted xfail for test_move_success_writes_audit_row and test_delete_success_writes_audit_row; added mock adapters for deterministic audit verification"
|
|
|
|
decisions:
|
|
- "Descendant detection walks ancestor chain through cloud_items DB (metadata-only, no provider call)"
|
|
- "Move stale guard marks the source folder (where the item was) as non-fresh"
|
|
- "Move success invalidates both source and destination folders — both listings changed"
|
|
- "Delete response carries is_folder and item_kind for frontend D-10 folder vs file disclosure"
|
|
- "Delete pre-fetches item metadata before adapter call so disclosure is available regardless of provider result"
|
|
- "Audit xfail markers in test_cloud_audit.py promoted to passing tests with explicit mock adapters"
|
|
|
|
metrics:
|
|
duration: "~30 minutes"
|
|
completed: "2026-06-22"
|
|
tasks_completed: 2
|
|
tasks_planned: 2
|
|
files_changed: 3
|
|
files_created: 0
|
|
tests_added: 11
|
|
tests_passing: 766
|
|
---
|
|
|
|
# Phase 13 Plan 09: Move and Delete — Validation, Reconciliation, and Audit Summary
|
|
|
|
**One-liner:** Move descendant-safety, stale guard, and reconcile-before-return; delete folder disclosure and metadata-only audit rows for both move and delete.
|
|
|
|
## Tasks Completed
|
|
|
|
| Task | Name | Commit | Key Files |
|
|
|------|------|--------|-----------|
|
|
| 1 (RED) | Add failing tests for move stale-guard, descendant safety, reconciliation, and delete/audit | af7d45f | test_cloud_mutations.py |
|
|
| 1/2 (GREEN) | Implement move/delete with stale guards, reconciliation, disclosure, and audit | 508d276 | api/cloud/operations.py, test_cloud_audit.py |
|
|
|
|
## What Was Built
|
|
|
|
### Task 1: Move — Descendant Safety, Stale Guard, Centralized Reconciliation
|
|
|
|
**`_is_descendant_destination()` helper in `backend/api/cloud/operations.py`:**
|
|
|
|
New async helper that walks the ancestor chain of a proposed destination through `cloud_items` metadata:
|
|
|
|
```python
|
|
while current_ref is not None:
|
|
if current_ref == source_item_id:
|
|
return True # destination is a descendant of source
|
|
# look up parent of current_ref via CloudItem.parent_ref
|
|
current_ref = parent_item.parent_ref if parent_item else None
|
|
```
|
|
|
|
- D-09: Backend independently rejects descendant destinations without provider involvement
|
|
- Protection against cycles via a visited-set guard
|
|
|
|
**`move_cloud_item` in `backend/api/cloud/operations.py`:**
|
|
|
|
Comprehensive move implementation:
|
|
|
|
1. **D-08 Cross-connection rejection** — before any adapter call (unchanged)
|
|
2. **D-09 Self-destination rejection** — before any adapter call (unchanged)
|
|
3. **D-09 Descendant destination rejection** — metadata-only ancestor-chain walk before adapter call
|
|
4. **Pre-fetch item metadata** — resolves `source_parent_ref`, `item_kind`, `item_name` before adapter
|
|
5. **D-07 Stale guard** — stale adapter result marks source folder non-fresh, returns typed stale body
|
|
6. **Reconcile-before-return on success:**
|
|
- `upsert_cloud_item` with `parent_ref=resolved_dest_parent_ref`
|
|
- `update_folder_state` for source folder (item removed)
|
|
- `update_folder_state` for destination folder (item added)
|
|
- `write_audit_log` `cloud.item_moved` with metadata-only payload
|
|
- `session.commit()`
|
|
|
|
### Task 2: Delete — Folder Disclosure and Metadata-Only Audit
|
|
|
|
**`delete_cloud_item` in `backend/api/cloud/operations.py`:**
|
|
|
|
1. **Pre-fetch item metadata** — resolves `item_kind` and `item_parent_ref` before adapter call
|
|
2. **D-10 Folder disclosure** — response includes `is_folder` and `item_kind` for frontend to display stronger folder-delete warning
|
|
3. **Reconcile-before-return on success:**
|
|
- `update_folder_state` for parent folder (item removed)
|
|
- `write_audit_log` `cloud.item_deleted` with `delete_kind`, `item_kind`, `provider_item_id`
|
|
- `session.commit()`
|
|
4. **D-11 Trash vs permanent** — `delete_kind` in audit metadata and response `reason` distinguish trashed vs permanent delete
|
|
|
|
Success response now includes:
|
|
```python
|
|
{
|
|
"kind": "deleted",
|
|
"reason": "trashed", # or "permanent"
|
|
"provider_item_id": "...",
|
|
"item_kind": "folder", # D-10 disclosure
|
|
"is_folder": True, # D-10 stronger confirmation
|
|
}
|
|
```
|
|
|
|
## Test Suite Results
|
|
|
|
```
|
|
766 passed, 17 skipped, 4 deselected, 10 xfailed, 55 warnings
|
|
```
|
|
|
|
Previous baseline (Plan 08): 755 passed, 17 skipped, 4 deselected, 12 xfailed
|
|
|
|
- 11 new tests added in `test_cloud_mutations.py` (all pass)
|
|
- 2 xfail markers promoted in `test_cloud_audit.py` (move + delete audit tests now pass)
|
|
- Net: +11 passing, -2 xfailed = 766 total passing
|
|
|
|
## Deviations from Plan
|
|
|
|
### Auto-fixed Issues
|
|
|
|
**1. [Rule 1 - Bug] Audit tests in test_cloud_audit.py needed mock adapters**
|
|
- **Found during:** Task 2 (GREEN verification)
|
|
- **Issue:** `test_move_success_writes_audit_row` and `test_delete_success_writes_audit_row` were promoted from xfail but used real connections without mock adapters — provider calls returned 503 which was not in the accepted status codes
|
|
- **Fix:** Added explicit mock adapters with `build_mutable_cloud_adapter` patch so tests exercise the audit write path deterministically
|
|
- **Files modified:** `backend/tests/test_cloud_audit.py`
|
|
- **Commit:** 508d276 (included in GREEN commit)
|
|
|
|
## Known Stubs
|
|
|
|
None. Move and delete reconciliation are fully functional, including stale guards, descendant safety, and metadata-only audit rows.
|
|
|
|
## Threat Flags
|
|
|
|
No new security surfaces introduced. T-13-27, T-13-28, T-13-29 are mitigated:
|
|
|
|
- **T-13-27 (move destinations):** Descendant-chain walk + self-check + cross-connection check enforce D-08 and D-09 before and independently of provider submission.
|
|
- **T-13-28 (stale move safety):** Stale guard stops move, refreshes source folder, requires retry — never forcing a stale move.
|
|
- **T-13-29 (delete disclosure and audit):** Typed delete results carry `is_folder`/`item_kind`; audit tests verify no token or byte appears in `metadata_`.
|
|
|
|
## Self-Check: PASSED
|
|
|
|
- FOUND: `backend/api/cloud/operations.py` with `_is_descendant_destination`, move stale guard, move/delete reconcile-before-return, and move/delete audit writes
|
|
- FOUND: `backend/tests/test_cloud_mutations.py` with 11 new behavioral tests
|
|
- FOUND: `backend/tests/test_cloud_audit.py` with promoted move/delete xfail markers and mock adapters
|
|
- FOUND commit: `af7d45f` (RED tests)
|
|
- FOUND commit: `508d276` (GREEN implementation)
|
|
- Full suite: 766 passed, 17 skipped, 4 deselected, 10 xfailed
|