Files
kite/.planning/phases/12.1-fix-nextcloud-root-listing-and-sync-visibility/12.1-RESEARCH.md
T
curo1305andClaude Sonnet 4.6 73328eee0b docs(12.1): add phase 12.1 planning artifacts — Nextcloud root listing fix
Plans 01–04, CONTEXT, PATTERNS, RESEARCH, UAT, and .gitkeep for
the fix-nextcloud-root-listing-and-sync-visibility sub-phase.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-23 20:34:47 +02:00

334 lines
23 KiB
Markdown

# Phase 12.1: Fix Nextcloud Root Listing and Sync Visibility — Research
**Researched:** 2026-06-22
**Status:** Complete
**Scope:** Secure live diagnosis, provider-neutral browse contracts, reconciliation/freshness correctness, and cross-provider validation design
## Research Question
Why can a healthy Nextcloud connection report a successful sync while its root appears empty, and what contract, integration, live-smoke, and UI tests are required to make browse correctness observable across Nextcloud, generic WebDAV, Google Drive, and OneDrive without downloading provider bytes or mutating provider content?
## Executive Summary
The live Nextcloud account is reachable and its root is not empty. A credential-redacted, read-only `PROPFIND` with `Depth: 1` returned HTTP `207 Multi-Status` and 10 direct children. Seven names exactly matched the supplied expectation and three returned names differed from the supplied expectation; no unexpected names were printed or persisted. This proves that the configured account and WebDAV endpoint can return root metadata and that the DocuVault empty-root behavior is primarily an application defect, not an empty account or basic connectivity failure. The three name differences should be treated as test-fixture drift until confirmed in the Nextcloud UI; they do not explain DocuVault returning no items.
The immediate backend root cause is deterministic: `NextcloudBackend` overrides the provider-neutral `WebDAVBackend.list_folder(connection_id, user_id, parent_ref=None, page_token=None) -> CloudListing` with a legacy `list_folder(folder_path="") -> list[dict]`. The canonical browse endpoint invokes the four-argument contract, so a Nextcloud adapter raises `TypeError` before performing a listing. Existing tests only assert that Nextcloud is a `CloudResourceAdapter` subclass and that `list_folder` is async; they never invoke the Nextcloud instance through the canonical contract. Python inheritance therefore made a structurally invalid adapter look valid.
Three additional defects create false-sync or invisible-navigation behavior across providers:
1. The synchronous browse path and Celery refresh path mark a folder `fresh` even when an adapter returns `CloudListing(complete=False)`. An incomplete or failed fetch is therefore eligible to look successfully synchronized.
2. `CloudFolderView.vue` ignores `response.freshness`, unconditionally sets freshness to `fresh`, and invents `lastRefreshedAt` from the browser clock after any HTTP 200. A safe HTTP response containing an empty incomplete/warning listing can therefore be shown as current.
3. The API returns `kind: "folder" | "file"`, `provider_item_id`, and a stable DocuVault `id`, but cloud views and tree components still filter on `is_dir` and navigate using `item.id`. Folders are consequently treated as files, and folder navigation sends the DocuVault metadata UUID where adapters require the provider folder reference/path.
Phase 12.1 should fix the inherited adapter contract first, then make listing completeness control freshness, then align the frontend with the normalized API. The verification architecture should apply one reusable provider contract suite to all four adapters and add opt-in, read-only live smoke tests whose credentials remain in ignored environment variables.
## Secure Live Probe Evidence
### Guardrails Used
- Read only the ignored `.env` variables `NEXTCLOUD_URL`, `NEXTCLOUD_USER`, and `NEXTCLOUD_APP_PASSWORD` in process memory.
- Performed only one-level `PROPFIND` metadata requests; no `GET`, download, upload, `MKCOL`, `PUT`, `MOVE`, `COPY`, or `DELETE` request was made.
- Did not edit `.env` or any environment file.
- Did not print, echo, log, quote, persist, or include credentials or the full URL in this artifact.
- Reported only response status, direct-child count, expected-match count, missing expected names, and unexpected-name count. Unexpected provider names were deliberately not emitted.
### Sanitized Result
| Check | Result |
|---|---|
| Endpoint reachable | Yes |
| Authentication accepted | Yes |
| Method | `PROPFIND` |
| Depth | `1` |
| HTTP status | `207 Multi-Status` |
| Direct root children returned | 10 |
| Exact supplied-name matches | 7 of 10 |
| Supplied names not exactly matched | `Manual Nextcloud.png`, `Nextcloud Intro.mp4`, `Template credits.md` |
| Additional returned-name count | 3; names withheld |
| Byte download or provider mutation | None |
The root-list response confirms that URL derivation to the user WebDAV root works when a base Nextcloud URL is normalized to the standard `/remote.php/dav/files/{encoded-user}/` endpoint. Repeated percent-decoding and plus-decoding did not turn the three differences into exact matches, so those differences are not explained by a simple href percent-encoding bug.
## Root-Cause Analysis
### P0 — Nextcloud Violates the Provider-Neutral Adapter Signature
Current runtime signatures:
```text
NextcloudBackend.list_folder(self, folder_path: str = "") -> list[dict]
WebDAVBackend.list_folder(self, connection_id, user_id, parent_ref=None, page_token=None) -> CloudListing
```
`build_cloud_resource_adapter("nextcloud", credentials)` passes the `isinstance(..., CloudResourceAdapter)` check because `NextcloudBackend` inherits from `WebDAVBackend`. However, the override replaces the abstract-contract implementation with the old pre-Phase-12 method. The canonical browse endpoint calls:
```python
await adapter.list_folder(connection_id, current_user.id, parent_ref=parent_ref)
```
For Nextcloud this raises before the provider request. The most direct fix is to delete the duplicate Nextcloud `list_folder` override and inherit the shared WebDAV implementation. If Nextcloud-specific href normalization is required, it should be a narrowly scoped protected hook used by the shared WebDAV listing algorithm, not a second public method with a different contract.
This is exactly the project rule “things that look the same to the user are the same in code”: Nextcloud is a WebDAV specialization and must not own a parallel browse API.
### P0 — Incomplete Listings Are Marked Fresh
Both first-visit browse reconciliation and `refresh_cloud_folder` do the following regardless of `listing.complete`:
1. reconcile the listing;
2. set folder state to `fresh`;
3. update `last_refreshed_at`.
`reconcile_cloud_listing` correctly avoids soft deletion for `complete=False`, but freshness semantics are not enforced by the service boundary. An incomplete empty result can therefore preserve cached rows yet still claim synchronization succeeded; on first visit it can claim an empty root is fresh.
The planner should introduce one service-level completion gate shared by synchronous browse and Celery refresh. A listing may transition to `fresh` only when `complete=True` and every required page/response was parsed successfully. `complete=False` must retain the previous `last_refreshed_at`, set a controlled warning/error code, and schedule bounded retry where appropriate. An empty `complete=True` listing remains a valid authoritative empty folder.
### P0 — Frontend Discards Server Freshness
`CloudFolderView.vue` currently changes state to:
```text
freshness = "fresh"
refreshedAt = new Date().toISOString()
```
after every successful HTTP response. It does not use `data.freshness.refresh_state`, `last_refreshed_at`, `error_code`, or `error_message`. This independently explains the user-visible “everything is synced” report even when backend refresh failed or was incomplete.
The view must map the server freshness object verbatim into the store and render controlled warning copy. The client clock must never manufacture provider refresh evidence. A 200 response means the cached browse request succeeded; it does not mean provider synchronization succeeded.
### P0 — API/UI Item Shape and Navigation Reference Disagree
The backend whitelisted response uses:
```text
kind: "folder" | "file"
id: stable DocuVault CloudItem UUID
provider_item_id: provider folder/file reference
```
The frontend still expects `is_dir` and uses `item.id` as the next `parent_ref`. Consequences:
- `items.filter(i => i.is_dir)` yields no folders because `is_dir` is absent;
- folders fall into the file collection;
- sidebar trees also filter on the absent field;
- clicking a folder, where possible, sends a DocuVault UUID to WebDAV/Drive/Graph instead of the provider reference.
Prefer one normalized frontend shape rather than adding duplicate backend fields. Views/components should use `item.kind === "folder"`, and provider navigation should use `item.provider_item_id`. The stable DocuVault `id` remains the row identity/key for persistence and future analysis. If the team chooses to expose an explicit `navigation_ref`, it must be populated centrally in `CloudItemOut`; do not reconstruct provider-specific navigation values in Vue.
### P1 — Nextcloud URL Normalization Is UI-Only
The credential modal constructs the standard Nextcloud WebDAV root from a base URL and encoded username, but the backend accepts and stores an arbitrary `server_url` without provider-specific normalization. This creates inconsistent behavior among UI-created connections, API clients, imported configuration, edited credentials, and live tests.
Add one pure shared helper for Nextcloud endpoint normalization, used by connection create/update and backend construction. Required cases:
- bare origin with or without trailing slash;
- origin with a deployment subpath;
- already canonical `/remote.php/dav/files/{user}/` endpoint;
- username requiring path-segment percent encoding;
- trailing-slash idempotence;
- rejection of query strings, fragments, userinfo, non-HTTPS, private/loopback/link-local targets, and malformed endpoints;
- no double append of the WebDAV suffix;
- no logging of the normalized URL when it contains a username path segment.
SSRF validation must run against the normalized URL before client construction and before every outbound request. Redirects must not bypass host/IP revalidation.
### P1 — WebDAV Listing Uses N+1 Requests and Weak Error Semantics
The shared implementation calls `client.list()` and then `client.info()` once per item. Per-item `info()` errors are swallowed and converted to defaults, potentially misclassifying a folder as a file. A standards-level `PROPFIND Depth: 1` can return the root self-response and all direct children with `resourcetype`, length, type, mtime, and etag in one response.
Phase 12.1 should preferably centralize a single Depth-1 parser for Nextcloud and generic WebDAV. It must:
- send explicit `Depth: 1`;
- request an allowlist of metadata properties only;
- identify and exclude exactly the collection self-response by normalized href, not by a display-name coincidence;
- decode each href path segment exactly once for display while retaining a canonical provider reference for subsequent requests;
- accept absolute and relative hrefs but reject a child href that escapes the configured root;
- distinguish folders from the DAV `collection` resource type, not size or trailing slash alone;
- treat missing optional properties as `None` without dropping the item;
- mark the entire listing incomplete on malformed multistatus, request failure, untrusted href, or interrupted pagination/response processing;
- never call byte-content methods.
## Provider-Neutral Correctness Contract
Every provider adapter must pass the same behavioral suite. Provider-specific fixtures are inputs; normalized `CloudListing` and `CloudResource` behavior is the contract.
### Adapter Contract
For each of Nextcloud, WebDAV, Google Drive, and OneDrive:
1. `list_folder` has the canonical callable signature and accepts root plus nested `parent_ref`.
2. It returns `CloudListing`, never a provider dictionary/list.
3. Every resource contains the requested `connection_id` and `user_id` supplied by the trusted caller.
4. `provider_item_id` is a usable opaque navigation reference; `id` is a generated/stable-in-DB DocuVault identity and is not sent back to the provider.
5. `kind`, `name`, parent, size, content type, modified time, and etag/version normalize consistently, with absent provider metadata represented as `None`.
6. All pages are consumed before `complete=True`; an error on any page yields `complete=False` and must not authorize deletion.
7. Authentication/scope failures map to typed, controlled domain reasons; raw provider bodies and URLs never reach API responses or audit logs.
8. Listing and capability discovery invoke no upload, create-folder, move, rename, copy, delete, media/content, or download method.
9. Listing changes neither `quotas.used_bytes` nor MinIO/object storage.
### Provider-Specific Fixtures
| Provider | Required fixture assertions |
|---|---|
| Nextcloud | Canonical and base URL normalization; `207` multistatus; explicit Depth 1; root self href removed; encoded spaces/unicode; DAV collection detection; nested path; malformed/foreign href incomplete; canonical four-argument method inherited or implemented exactly once |
| Generic WebDAV | Absolute/relative hrefs; missing optional props; servers with/without trailing slashes; 401/403 vs 5xx mapping; root and nested Depth-1 listings; no N+1 metadata calls if direct parser is adopted |
| Google Drive | Root query uses `root`; every `nextPageToken` followed; folder MIME type; native Google files have nullable size; trashed items excluded; insufficient `drive.file` visibility represented as scope limitation where detectable; no `get_media` |
| OneDrive | Root and item children endpoints; every `@odata.nextLink` followed; folder/file facets; parent reference and drive context retained; eTag/cTag normalization; expired token refresh failure controlled; no `/content` request |
## Test Design
### 1. Static and Runtime Contract Regression
Add a parametrized test over all factory providers that constructs the adapter, inspects `list_folder`, and invokes it with `(connection_id, user_id, parent_ref=None, page_token=None)` against mocked provider responses. This would have caught the current Nextcloud override immediately. Merely testing `issubclass` or `iscoroutinefunction` is insufficient.
Suggested location: `backend/tests/test_cloud_provider_contract.py`.
Core assertions:
```text
factory returns CloudResourceAdapter
canonical invocation does not raise TypeError
return type is CloudListing
item ownership IDs equal trusted call arguments
listing uses provider metadata only
no byte or mutation spy was called
```
### 2. WebDAV/Nextcloud Parser Tests
Use recorded synthetic XML fixtures with no real credentials or hostnames:
- root self + 4 folders + 6 files produces exactly 10 children;
- spaces, `%` characters, unicode, trailing slash, absolute href, and relative href;
- child folder recognized from `<d:collection/>` even with size absent;
- same basename as root is not incorrectly filtered when it is a genuine child;
- href outside the configured DAV root is rejected and listing is incomplete;
- missing property status blocks do not drop a child;
- malformed XML, non-207 response, timeout, and mid-parse failure return/raise a controlled incomplete result;
- request method is `PROPFIND`, `Depth` is exactly `1`, and body requests no content bytes.
### 3. Freshness/Reconciliation Tests
Add a shared service such as `apply_cloud_listing_result` or an equivalent explicit branch and test:
- `complete=True`, non-empty: upsert, delete unseen siblings, state fresh, advance `last_refreshed_at`;
- `complete=True`, empty: authoritative empty folder, delete unseen siblings, state fresh;
- `complete=False`, empty or partial: retain all unseen rows, state warning, preserve previous `last_refreshed_at`;
- provider exception: retain cache, controlled warning, bounded retry only for transient errors;
- first visit + incomplete empty listing: response is empty with warning, never fresh;
- repeated complete listing is idempotent;
- no branch changes quota or calls byte storage.
Exercise both the synchronous first-visit endpoint and the Celery `_run` path so they cannot drift.
### 4. API and Frontend Contract Tests
Backend API tests should return a mixed folder/file fixture and assert `kind`, stable `id`, provider navigation reference, freshness, credential exclusion, owner scoping, and admin denial.
Frontend tests should use the real response shape, not `{items: []}`:
- mixed items split correctly using `kind`;
- folder click navigates with `provider_item_id` or centralized `navigation_ref`, never stable metadata `id`;
- sidebar tree applies the same rule;
- warning/refreshing/fresh is copied from `data.freshness`;
- `last_refreshed_at` comes from the server and is not replaced with `new Date()`;
- HTTP 200 + warning + empty items renders a synchronization warning, not “This folder is empty” with a fresh indicator;
- cached items remain visible during warning;
- filenames render as text with no `v-html`.
Suggested files:
- `frontend/src/views/__tests__/CloudFolderView.test.js`
- `frontend/src/components/cloud/__tests__/CloudProviderTreeItem.test.js`
- `frontend/src/components/cloud/__tests__/CloudFolderTreeItem.test.js`
- `frontend/src/stores/__tests__/cloudConnections.test.js`
### 5. Opt-In Read-Only Live Tests
Live tests should be marked, excluded from the default deterministic suite, and enabled explicitly, for example with `RUN_CLOUD_LIVE_TESTS=1`. They must skip when provider-specific variables are absent. Test output must contain provider name, status category, counts, and sanitized mismatch summaries only—never credentials, tokens, usernames, response headers, response bodies, or full URLs.
Nextcloud variables already available:
```text
NEXTCLOUD_URL
NEXTCLOUD_USER
NEXTCLOUD_APP_PASSWORD
```
The Nextcloud live test should normalize the endpoint through production code, invoke `build_cloud_resource_adapter("nextcloud", ...)`, call the canonical root listing, and assert:
- `CloudListing.complete is True`;
- exactly 10 direct children for the controlled fixture account, once fixture drift is reconciled;
- the agreed expected-name set matches exactly;
- at least the expected folder/file kinds are correct;
- no content or mutation method is available to the test harness;
- a quota snapshot before/after is unchanged if exercised through the DocuVault API.
For Google Drive, OneDrive, and generic WebDAV, define equivalent optional environment contracts only when dedicated revocable test accounts are supplied. OAuth live tests should list a controlled root folder with least-privilege test credentials, follow pagination, and compare an agreed fixture set. Never make the live suite create its own marker file: provider mutations are outside Phase 12.1 research and are prohibited for these smoke tests.
### 6. Security-Negative Tests
- foreign user connection ID returns indistinguishable 404;
- admin cannot browse user cloud metadata;
- credentials and normalized full endpoint never appear in response, logs, task payload, exception strings, or snapshots;
- worker decrypts credentials only after owner-scoped connection resolution;
- WebDAV/Nextcloud root and every redirect/request pass SSRF checks;
- malicious href cannot escape configured root or cause a request to another origin;
- provider error text is mapped to controlled codes/messages;
- listing performs no `get_object`, Google `get_media`, OneDrive `/content`, MinIO operation, quota update, or mutation verb.
## Recommended Implementation Shape
### Plan 12.1-01 — Restore the Shared Adapter Contract
- Add failing cross-provider runtime contract tests first.
- Remove the legacy Nextcloud public `list_folder` override so Nextcloud inherits the shared canonical WebDAV implementation.
- Resolve the deprecated provider-folder helper: route it through the canonical connection-ID adapter or delete the dead compatibility route if no active route/import depends on it. Do not retain two public listing shapes.
- Add backend Nextcloud URL normalization in one shared helper and test idempotence/security cases.
### Plan 12.1-02 — Make WebDAV Metadata Listing Authoritative
- Implement or encapsulate a single Depth-1 PROPFIND parser shared by Nextcloud and generic WebDAV.
- Normalize root self filtering, href containment/decoding, kind, metadata, and controlled errors.
- Add fixture tests for the supplied 10-item root plus encoding, missing-property, malformed-response, and SSRF cases.
- Keep listing metadata-only and mutation-free.
### Plan 12.1-03 — Eliminate False Freshness
- Centralize complete/incomplete listing state transitions.
- Use the same transition in synchronous browse and Celery refresh.
- Ensure incomplete results preserve cache and last success, set warning, and retry only when transient.
- Add reconciliation, endpoint, worker, quota, and no-byte tests.
### Plan 12.1-04 — Align the Shared Browser Contract
- Update cloud views/trees to use normalized `kind` and provider navigation reference.
- Consume server freshness instead of manufacturing success.
- Add mixed-item navigation and warning-state frontend regressions while keeping `StorageBrowser.vue` as the sole browser.
- Run the read-only Nextcloud live smoke, then full backend/frontend/security gates and documentation/version closeout required by project protocol.
## Acceptance Evidence
Phase 12.1 is complete only when all of the following hold:
1. The production Nextcloud adapter, invoked through `build_cloud_resource_adapter`, returns the agreed root fixture through the canonical contract.
2. Nextcloud and generic WebDAV share one public listing implementation/signature.
3. All four providers pass one runtime adapter contract suite and provider-specific normalization/pagination tests.
4. `complete=False` cannot produce `fresh` or advance `last_refreshed_at` in either request or worker paths.
5. The frontend renders folders/files from the API's normalized shape, navigates with provider references, and displays backend freshness faithfully.
6. Browse/live tests use metadata requests only and prove zero byte downloads, provider mutations, MinIO writes, and quota changes.
7. Owner/admin/credential/SSRF/href-containment negative tests pass.
8. The controlled Nextcloud live smoke reports a complete listing and the final agreed exact-name fixture; any fixture drift is explicitly reconciled rather than hidden.
## Risks and Planner Notes
- Do not “fix” Nextcloud by adding an adapter-specific branch in `browse.py` or Vue. The public contract must be identical across providers.
- Do not interpret `HTTP 200` from DocuVault as provider freshness; cached browse success and provider refresh success are separate states.
- Do not use a root item count alone as general production correctness. Exact names are appropriate only for dedicated controlled live accounts; generic folders may legitimately be empty.
- Do not mark an empty listing incomplete merely because it is empty. Completeness comes from successful authoritative traversal, not item count.
- Google `drive.file` can produce a valid but intentionally restricted view. Provider-neutral tests should distinguish transport completeness from authorization scope and expose `insufficient_scope` where known.
- OneDrive continuation URLs are provider-supplied. Follow them only for the expected Graph origin and never accept arbitrary client-supplied pagination URLs.
- Keep unexpected live filenames out of CI logs; report hashed/set counts or controlled expected-name diffs only.
- The current worktree contains unrelated user changes and untracked files. Implementation plans must stage only Phase 12.1 files plus required documentation/version updates.
## RESEARCH COMPLETE