docs(13-06): complete upload follow-through plan
This commit is contained in:
curo1305
@@ -0,0 +1,180 @@
|
||||
---
|
||||
phase: "13"
|
||||
plan: "06"
|
||||
subsystem: cloud-operations
|
||||
status: complete
|
||||
tags:
|
||||
- cloud
|
||||
- upload
|
||||
- reconciliation
|
||||
- audit
|
||||
- cloud-items
|
||||
- tdd
|
||||
|
||||
dependency_graph:
|
||||
requires:
|
||||
- "13-01 (RED mutation contract tests)"
|
||||
- "13-03 (cloud operations seam + mutable provider implementations)"
|
||||
- "13-04 (operations.py route layer + typed result vocabulary)"
|
||||
- "13-05 (keep_both_name + upload mechanics tests)"
|
||||
provides:
|
||||
- "Upload success routes through upsert_cloud_item for stable row identity"
|
||||
- "Parent folder state invalidated (warning/upload_mutated) after success"
|
||||
- "cloud.file_uploaded audit row written in same transaction as reconciliation"
|
||||
- "3 Task 1 reconciliation tests in test_cloud_mutations.py"
|
||||
- "1 Task 2 upload audit test promoted from xfail in test_cloud_audit.py"
|
||||
affects:
|
||||
- "13-07 through 13-11 (rename, move, delete, folder audit and reconciliation plans)"
|
||||
|
||||
tech_stack:
|
||||
added:
|
||||
- "upsert_cloud_item called from upload route on MUT_KIND_UPLOADED success"
|
||||
- "update_folder_state called with warning/upload_mutated on upload success"
|
||||
- "write_audit_log called with cloud.file_uploaded event on upload success"
|
||||
- "get_client_ip imported from deps.utils in operations.py"
|
||||
- "write_audit_log imported from services.audit in operations.py"
|
||||
patterns:
|
||||
- "Reconcile-before-return: upsert + folder invalidation complete before response"
|
||||
- "Audit-in-transaction: write_audit_log flushes in caller transaction, caller commits"
|
||||
- "Non-success bypass: conflict/offline/reauth paths never reach reconciliation or audit"
|
||||
- "upload_mutated folder state: controlled code signaling re-list without a full provider scan"
|
||||
|
||||
key_files:
|
||||
modified:
|
||||
- path: "backend/api/cloud/operations.py"
|
||||
change: "Upload success path now calls upsert_cloud_item, update_folder_state, write_audit_log before commit"
|
||||
- path: "backend/tests/test_cloud_mutations.py"
|
||||
change: "Added 3 reconciliation behavioral tests for Task 1 (RED then GREEN)"
|
||||
- path: "backend/tests/test_cloud_audit.py"
|
||||
change: "Promoted test_upload_success_writes_metadata_only_audit_row from xfail to real test (RED then GREEN)"
|
||||
|
||||
decisions:
|
||||
- "upload_mutated is the controlled error_code for folder invalidation after upload — avoids apply_listing_and_finalize which needs a full provider listing"
|
||||
- "CloudResource.id assigned uuid4() at route layer before upsert — cloud_items.upsert_cloud_item uses provider_item_id as the stable identity key, not this id"
|
||||
- "Audit write uses flush (not commit) per services.audit contract; one commit covers upsert + folder state + audit atomically"
|
||||
- "Non-success upload paths (conflict, offline, reauth_required) do not call write_audit_log, satisfying T-13-21 (no false success events)"
|
||||
|
||||
metrics:
|
||||
duration: "~5 minutes"
|
||||
completed: "2026-06-22"
|
||||
tasks_completed: 2
|
||||
tasks_planned: 2
|
||||
files_changed: 3
|
||||
files_created: 0
|
||||
tests_added: 4
|
||||
tests_passing: 745
|
||||
---
|
||||
|
||||
# Phase 13 Plan 06: Upload Follow-Through Slice Summary
|
||||
|
||||
**One-liner:** Upload success routes through `upsert_cloud_item` + folder state invalidation + metadata-only `cloud.file_uploaded` audit row before the response is returned.
|
||||
|
||||
## Tasks Completed
|
||||
|
||||
| Task | Name | Commit | Key Files |
|
||||
|------|------|--------|-----------|
|
||||
| 1 (RED) | Add failing upload reconciliation tests | 4cd6499 | test_cloud_mutations.py |
|
||||
| 1 (GREEN) | Route upload success through centralized reconciliation | 7ecbec7 | api/cloud/operations.py |
|
||||
| 2 (RED) | Promote upload audit test from xfail | 33f0498 | test_cloud_audit.py |
|
||||
| 2 (GREEN) | Emit metadata-only audit row on upload success | d959e0c | api/cloud/operations.py |
|
||||
|
||||
## What Was Built
|
||||
|
||||
### Task 1: Reconcile-before-return on upload success
|
||||
|
||||
**RED phase** added 3 tests to `test_cloud_mutations.py`:
|
||||
|
||||
- `test_upload_success_upserts_cloud_item_before_returning` — verifies that after a 200 upload response, a CloudItem row exists in the DB with the correct `provider_item_id` and `name`. This was the core reconcile-before-return behavioral test.
|
||||
- `test_upload_success_marks_folder_freshness_stale` — verifies that a `CloudFolderState` row exists for the parent folder after success (folder state created/updated to signal re-list needed).
|
||||
- `test_upload_failed_does_not_mutate_cloud_items` — verifies offline/error upload results do not create phantom CloudItem rows.
|
||||
|
||||
**GREEN phase** implemented in `backend/api/cloud/operations.py`:
|
||||
|
||||
Inside the `MUT_KIND_UPLOADED` success branch of `upload_cloud_file`:
|
||||
|
||||
```python
|
||||
resource = CloudResource(
|
||||
id=uuid.uuid4(),
|
||||
provider_item_id=provider_item_id,
|
||||
connection_id=connection_id,
|
||||
user_id=current_user.id,
|
||||
name=resolved_name,
|
||||
kind="file",
|
||||
parent_ref=resolved_parent_ref,
|
||||
content_type=content_type,
|
||||
size=resolved_size,
|
||||
)
|
||||
await upsert_cloud_item(session, user_id=str(current_user.id), resource=resource)
|
||||
|
||||
await update_folder_state(
|
||||
session,
|
||||
user_id=str(current_user.id),
|
||||
connection_id=str(connection_id),
|
||||
parent_ref=invalidate_parent,
|
||||
refresh_state="warning",
|
||||
error_code="upload_mutated",
|
||||
error_message="Folder contents changed by upload — re-listing required.",
|
||||
)
|
||||
```
|
||||
|
||||
The folder state is set to `warning` with `error_code="upload_mutated"` (not `apply_listing_and_finalize`) because we do not have a full provider listing — we only know one new item arrived. The browse endpoint will detect the non-fresh state and trigger a provider re-list on the next navigation.
|
||||
|
||||
### Task 2: Metadata-only upload audit row
|
||||
|
||||
**RED phase** promoted `test_upload_success_writes_metadata_only_audit_row` from `xfail` to a real test. The test now uses a mock adapter so it reliably triggers the 200 success path and checks for a `cloud.file_uploaded` audit row.
|
||||
|
||||
**GREEN phase** added audit write in `backend/api/cloud/operations.py`:
|
||||
|
||||
```python
|
||||
await write_audit_log(
|
||||
session,
|
||||
event_type="cloud.file_uploaded",
|
||||
user_id=current_user.id,
|
||||
actor_id=current_user.id,
|
||||
resource_id=None,
|
||||
ip_address=get_client_ip(request),
|
||||
metadata_={
|
||||
"connection_id": str(connection_id),
|
||||
"provider_item_id": provider_item_id,
|
||||
"filename": resolved_name,
|
||||
"size_bytes": resolved_size,
|
||||
"parent_ref": resolved_parent_ref,
|
||||
},
|
||||
)
|
||||
await session.commit()
|
||||
```
|
||||
|
||||
The audit payload is metadata-only: no provider URLs, access tokens, refresh tokens, document content, or raw bytes. The `session.commit()` is called once after upsert + folder state + audit — all three writes land in a single atomic transaction.
|
||||
|
||||
Non-success paths (`MUT_KIND_CONFLICT`, `MUT_KIND_OFFLINE`, `MUT_KIND_REAUTH`) branch before this code is reached, so they cannot produce false `cloud.file_uploaded` events (T-13-21).
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
None. The plan was executed exactly as written.
|
||||
|
||||
**Scope note:** The plan listed `backend/api/cloud/operations.py`, `backend/services/cloud_operations.py`, `backend/services/cloud_items.py`, `backend/tests/test_cloud_mutations.py`, and `backend/tests/test_cloud_audit.py` as files modified. In practice, `cloud_operations.py` and `cloud_items.py` were not modified — the existing `upsert_cloud_item`, `update_folder_state`, and `write_audit_log` helpers were sufficient. The route layer in `operations.py` consumed them directly per the CLAUDE.md shared module map.
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None. Upload reconciliation and audit are fully functional. Remaining xfail tests in `test_cloud_audit.py` cover rename, move, delete, and folder operations which are deferred to Plans 07–11.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
No new security surfaces introduced. T-13-19, T-13-20, and T-13-21 are mitigated:
|
||||
|
||||
- **T-13-19** (upload reconciliation bypass): Tests verify reconcile-before-return; non-success paths bypass the upsert/folder-state path.
|
||||
- **T-13-20** (audit payload secrecy): Audit metadata contains only controlled metadata fields; no provider token, URL, or content.
|
||||
- **T-13-21** (false success audit events): Conflict, offline, and reauth_required branches never reach the audit write call.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- FOUND: `backend/api/cloud/operations.py` with `upsert_cloud_item`, `update_folder_state`, `write_audit_log` calls in MUT_KIND_UPLOADED branch
|
||||
- FOUND: `backend/tests/test_cloud_mutations.py` with 3 new reconciliation tests
|
||||
- FOUND: `backend/tests/test_cloud_audit.py` with `test_upload_success_writes_metadata_only_audit_row` promoted from xfail
|
||||
- FOUND commit: `4cd6499` (Task 1 RED)
|
||||
- FOUND commit: `7ecbec7` (Task 1 GREEN)
|
||||
- FOUND commit: `33f0498` (Task 2 RED)
|
||||
- FOUND commit: `d959e0c` (Task 2 GREEN)
|
||||
- Full suite: 745 passed, 17 skipped, 4 deselected, 12 xfailed
|
||||
Reference in New Issue
Block a user