172 lines
9.4 KiB
Markdown
172 lines
9.4 KiB
Markdown
---
|
|
phase: "12.1"
|
|
plan: "02"
|
|
subsystem: cloud-storage-backends
|
|
status: complete
|
|
tags: [cloud, freshness, tdd, celery, reconciliation, security]
|
|
completed: "2026-06-22"
|
|
duration_seconds: 2100
|
|
task_count: 3
|
|
file_count: 13
|
|
|
|
requires:
|
|
- "12.1-01"
|
|
provides:
|
|
- "apply_listing_and_finalize — single shared freshness gate for API and worker"
|
|
- "ListingResult dataclass — structured result for complete vs incomplete listings"
|
|
- "Truthful refresh_state: complete=False can never produce refresh_state=fresh"
|
|
- "Preserved last_refreshed_at on incomplete/error refresh"
|
|
affects: [cloud-browse-api, cloud-tasks, cloud-items-service, security-gate]
|
|
|
|
tech_stack:
|
|
added: []
|
|
patterns:
|
|
- "TDD RED/GREEN — 9 failing tests written first; implementation makes them pass"
|
|
- "Single shared service gate (apply_listing_and_finalize) replaces two independent transitions"
|
|
- "ListingResult dataclass carries is_fresh, warning_code, warning_message — controlled API surface"
|
|
|
|
key_files:
|
|
created: []
|
|
modified:
|
|
- backend/services/cloud_items.py
|
|
- backend/api/cloud/browse.py
|
|
- backend/tasks/cloud_tasks.py
|
|
- backend/tests/test_cloud_items.py
|
|
- backend/tests/test_cloud.py
|
|
- backend/tests/test_cloud_security.py
|
|
- backend/main.py
|
|
- frontend/package.json
|
|
- CLAUDE.md
|
|
- README.md
|
|
- RUNBOOK.md
|
|
- SECURITY.md
|
|
|
|
decisions:
|
|
- "apply_listing_and_finalize is the canonical gate — browse.py and cloud_tasks.py may not call update_folder_state('fresh') independently after list_folder"
|
|
- "ListingResult.warning_message must never contain traceback, URLs, or credential strings"
|
|
- "SQLite test timezone: strip tzinfo before comparing naive vs aware datetime (test-only issue, not production)"
|
|
|
|
requirements:
|
|
- CLOUD-01
|
|
- CACHE-01
|
|
- SYNC-01
|
|
---
|
|
|
|
# Phase 12.1 Plan 02: Truthful Cloud Freshness Gate — Summary
|
|
|
|
**One-liner:** Added `apply_listing_and_finalize` as the single shared gate: `CloudListing.complete=False` now produces a controlled `warning` state instead of false `fresh`, preserving cached items and prior `last_refreshed_at`.
|
|
|
|
## What Was Built
|
|
|
|
### Task 1 — TDD RED: Failing tests for incomplete listing semantics
|
|
|
|
Added 9 new failing tests across three test files before touching production code:
|
|
|
|
**`backend/tests/test_cloud_items.py` (5 new tests):**
|
|
- `test_incomplete_listing_never_marks_folder_fresh` — asserts `apply_listing_and_finalize` (import error = RED) never sets `refresh_state=fresh` on `complete=False`
|
|
- `test_incomplete_listing_retains_cached_rows_and_last_success` — seeds prior timestamp, asserts both cached items and `last_refreshed_at` survive incomplete refresh
|
|
- `test_complete_empty_listing_is_authoritative_and_fresh` — asserts `complete=True, items=[]` soft-deletes unseen children AND marks fresh
|
|
- `test_partial_items_upsert_without_deleting_unseen_children` — asserts seen items upsert but unseen survive when `complete=False`
|
|
- `test_apply_listing_returns_warning_for_complete_false` — asserts `ListingResult(is_fresh=False, warning_code="incomplete_listing")` returned; tests warning_message for forbidden patterns (traceback, URL, credentials)
|
|
|
|
**`backend/tests/test_cloud.py` (2 new tests):**
|
|
- `test_sync_browse_returns_warning_for_complete_false` — first-visit browse with `complete=False` adapter must not produce `refresh_state=fresh`
|
|
- `test_worker_returns_warning_for_complete_false` — Celery `_run()` must not call `update_folder_state('fresh')` when listing is incomplete
|
|
|
|
**`backend/tests/test_cloud_security.py` (3 new tests):**
|
|
- `test_browse_complete_false_never_sets_fresh` — T-12.1-06 API-level assertion
|
|
- `test_browse_complete_false_no_raw_provider_error_in_response` — T-12.1-09 no traceback/exception/token in response body
|
|
- `test_browse_cached_items_remain_owner_scoped_on_incomplete` — T-12.1-07 IDOR holds even during incomplete refresh path
|
|
|
|
**Result:** 6 tests immediately failed with `ImportError: cannot import name 'apply_listing_and_finalize'` (RED confirmed).
|
|
|
|
### Task 2 — GREEN: Centralize listing application and freshness finalization
|
|
|
|
**`backend/services/cloud_items.py`:**
|
|
- Added `ListingResult` dataclass (`is_fresh: bool`, `warning_code: Optional[str]`, `warning_message: Optional[str]`)
|
|
- Added `apply_listing_and_finalize(session, *, user_id, connection_id, parent_ref, listing) -> ListingResult`:
|
|
- Calls `reconcile_cloud_listing` (existing, correct guard: deletion only when `complete=True`)
|
|
- `listing.complete=True`: calls `update_folder_state("fresh")`, returns `ListingResult(is_fresh=True)`
|
|
- `listing.complete=False`: sets `refresh_state="warning"`, `error_code="incomplete_listing"`, controlled message; does NOT touch `last_refreshed_at`; returns `ListingResult(is_fresh=False, warning_code="incomplete_listing", ...)`
|
|
|
|
**`backend/api/cloud/browse.py`:**
|
|
- Added `apply_listing_and_finalize` to imports
|
|
- First-visit synchronous refresh: replaced `reconcile_cloud_listing(...)` + `update_folder_state("fresh")` pair with a single `apply_listing_and_finalize(...)` call
|
|
|
|
**`backend/tasks/cloud_tasks.py`:**
|
|
- Added `apply_listing_and_finalize` to `_run()` imports
|
|
- Replaced `reconcile_cloud_listing(...)` + `update_folder_state("fresh")` pair with `apply_listing_and_finalize(...)`
|
|
- Task return value: `{"status": "ok", ...}` when `is_fresh=True`; `{"status": "warning", "warning_code": ..., ...}` when `is_fresh=False` (no credentials, no provider item names in broker result)
|
|
|
|
**Verification search:**
|
|
```
|
|
rg 'refresh_state="fresh"' api/cloud/browse.py tasks/cloud_tasks.py
|
|
(no output — no unconditional fresh assignments remain)
|
|
```
|
|
|
|
### Task 3 — Documentation, security review, version bump
|
|
|
|
- Version bumped `0.2.3 → 0.2.4` in `backend/main.py` and `frontend/package.json`
|
|
- `CLAUDE.md`: updated current state line; added `apply_listing_and_finalize` and `ListingResult` to module map; added "no independent fresh" rule
|
|
- `README.md`: version bump
|
|
- `RUNBOOK.md`: updated freshness state descriptions; documented `complete=True` gate requirement; `last_refreshed_at` advancement semantics
|
|
- `SECURITY.md`: added T-12.1-06 through T-12.1-10 threat closure table and security gate evidence
|
|
- Bandit over modified files: 0 HIGH, 0 MEDIUM findings
|
|
- Full backend suite: 595 passed, 1 pre-existing failure (`test_extract_docx` — missing `python-docx`, unrelated)
|
|
- Pushed to remote
|
|
|
|
## Deviations from Plan
|
|
|
|
### Auto-fixed Issues
|
|
|
|
**1. [Rule 1 - Bug] SQLite naive vs. aware datetime comparison in test**
|
|
- **Found during:** Task 2 (test_incomplete_listing_retains_cached_rows_and_last_success)
|
|
- **Issue:** SQLite strips timezone info when storing datetime objects. The test seeded `prior_ts = datetime(2026, 6, 1, 12, 0, 0, tzinfo=timezone.utc)` and retrieved it back as naive `datetime(2026, 6, 1, 12, 0)`. Direct `==` comparison failed with `AssertionError`.
|
|
- **Fix:** Normalize `stored_ts` by attaching `tzinfo=timezone.utc` when the stored value is naive, before comparing with `prior_ts`.
|
|
- **Files modified:** `backend/tests/test_cloud_items.py`
|
|
- **Scope:** Test only — not a production defect (PostgreSQL preserves timezone; SQLite is used for unit tests only).
|
|
|
|
## Test Evidence
|
|
|
|
| Suite | Tests | Status |
|
|
|-------|-------|--------|
|
|
| `test_cloud_items.py` | 45 | All pass (40 pre-existing + 5 new) |
|
|
| `test_cloud.py` | 25 | All pass (23 pre-existing + 2 new) |
|
|
| `test_cloud_security.py` | 22 | All pass (19 pre-existing + 3 new) |
|
|
| Full backend suite | 595 | 1 pre-existing failure (`test_extract_docx`) |
|
|
|
|
**RED/GREEN gate compliance:**
|
|
- RED commit `fe08afd` — 6 tests fail with ImportError before implementation
|
|
- GREEN commit `bfa4dc5` — all 12 new tests pass; existing 79 still pass
|
|
|
|
## Security Verification
|
|
|
|
Bandit over `services/cloud_items.py`, `api/cloud/browse.py`, `tasks/cloud_tasks.py`: 0 HIGH/MEDIUM findings.
|
|
|
|
Threat model coverage:
|
|
|
|
| Threat ID | Mitigation | Evidence |
|
|
|-----------|-----------|---------|
|
|
| T-12.1-06 | False fresh state on provider failure | `apply_listing_and_finalize` single gate; `test_incomplete_listing_never_marks_folder_fresh`, `test_browse_complete_false_never_sets_fresh` |
|
|
| T-12.1-07 | Incomplete listing deletes cached metadata | `reconcile_cloud_listing` still guarded by `complete=True`; `test_incomplete_listing_retains_cached_rows_and_last_success`, `test_partial_items_upsert_without_deleting_unseen_children` |
|
|
| T-12.1-08 | Error overwrites last known-good timestamp | `apply_listing_and_finalize` does not touch `last_refreshed_at` on `complete=False`; timestamp preservation test |
|
|
| T-12.1-09 | Provider errors leak secrets or URLs | Controlled `warning_message` only; `test_apply_listing_returns_warning_for_complete_false` checks forbidden patterns; `test_browse_complete_false_no_raw_provider_error_in_response` |
|
|
| T-12.1-10 | Refresh changes bytes/quota | No byte IO in `apply_listing_and_finalize`; existing no-byte/no-quota assertions still pass |
|
|
|
|
## Known Stubs
|
|
|
|
None — all warning messages, codes, and state transitions are fully wired.
|
|
|
|
## Self-Check: PASSED
|
|
|
|
| Check | Result |
|
|
|-------|--------|
|
|
| `backend/services/cloud_items.py` (apply_listing_and_finalize) | FOUND |
|
|
| `backend/api/cloud/browse.py` (apply_listing_and_finalize import) | FOUND |
|
|
| `backend/tasks/cloud_tasks.py` (apply_listing_and_finalize import) | FOUND |
|
|
| Commit fe08afd (RED tests) | FOUND |
|
|
| Commit bfa4dc5 (GREEN implementation) | FOUND |
|
|
| Commit 70a543a (docs + version) | FOUND |
|
|
| No unconditional refresh_state="fresh" in browse.py or cloud_tasks.py | CONFIRMED |
|
|
| Full test suite: 595 passed | CONFIRMED |
|