Cordy/cairn-desktop

Fork 0

P2-2: PROPFIND — implement Remote.List and Stat #22

New issue

Closed

opened 2026-09-10 17:40:58 +00:00 by Cordy · 2 comments

Cordy commented

2026-09-10 17:40:58 +00:00

Owner

Depends on P2-1.

Goal

Enumerate the remote tree over WebDAV and satisfy the Remote.List / Remote.Stat half of the interface from Task 6.

Files

Create: internal/remote/propfind.go, internal/remote/propfind_test.go

What to request

PROPFIND with Depth: 1, descending recursively. Request these properties:

<?xml version="1.0" encoding="utf-8"?>
<d:propfind xmlns:d="DAV:">
  <d:prop>
    <d:resourcetype/>
    <d:getcontentlength/>
    <d:getlastmodified/>
    <d:getetag/>
    <d:quota-available-bytes/>
    <d:quota-used-bytes/>
  </d:prop>
</d:propfind>

The two quota-* properties are RFC 4331 and are what let the UI show free space (Cairn already serves them).

Parsing gotchas — each gets a test

Response is 207 Multi-Status, not 200. Treat any other 2xx as an error.
href values are URL-encoded and may be absolute or relative. Decode, strip the base path, then pass through sync.Normalise.
ETags are frequently quoted ("abc123") and may carry a W/ weak prefix. Strip both, consistently, or every comparison against stored state will mismatch and you will re-download the world.
getlastmodified is RFC 1123 (Mon, 02 Jan 2006 15:04:05 GMT), not RFC 3339.
A collection has <d:collection/> inside <d:resourcetype> and no content length. Map to Entry.IsDir = true.
The request path itself is included in the response — exclude it from List output, or every directory contains itself.

Steps

Write failing tests against httptest serving a captured real 207 body — generate one from the .249 dogfood and paste it into a testdata file. Do not invent the XML; real servers differ from the spec in small ways.
Include a case with a quoted ETag, a W/ weak ETag, a path with a space, and a non-ASCII filename.
Run them; confirm they fail.
Implement using encoding/xml.
Run tests; confirm they pass.
Commit: git commit -s -m "feat(remote): PROPFIND-backed List and Stat"

Acceptance criteria

ETags are normalised identically on every path through the code (one helper, used everywhere).
Self-reference is excluded from List.
List output is sorted and canonical, matching the MemRemote contract so the engine cannot tell them apart.

Depends on P2-1. ## Goal Enumerate the remote tree over WebDAV and satisfy the `Remote.List` / `Remote.Stat` half of the interface from Task 6. ## Files - Create: `internal/remote/propfind.go`, `internal/remote/propfind_test.go` ## What to request `PROPFIND` with `Depth: 1`, descending recursively. Request these properties: ```xml <?xml version="1.0" encoding="utf-8"?> <d:propfind xmlns:d="DAV:"> <d:prop> <d:resourcetype/> <d:getcontentlength/> <d:getlastmodified/> <d:getetag/> <d:quota-available-bytes/> <d:quota-used-bytes/> </d:prop> </d:propfind> ``` The two `quota-*` properties are RFC 4331 and are what let the UI show free space (Cairn already serves them). ## Parsing gotchas — each gets a test - **Response is `207 Multi-Status`**, not 200. Treat any other 2xx as an error. - **`href` values are URL-encoded and may be absolute or relative.** Decode, strip the base path, then pass through `sync.Normalise`. - **ETags are frequently quoted** (`"abc123"`) and may carry a `W/` weak prefix. Strip both, consistently, or every comparison against stored state will mismatch and you will re-download the world. - **`getlastmodified` is RFC 1123** (`Mon, 02 Jan 2006 15:04:05 GMT`), not RFC 3339. - **A collection has `<d:collection/>` inside `<d:resourcetype>`** and no content length. Map to `Entry.IsDir = true`. - **The request path itself is included** in the response — exclude it from `List` output, or every directory contains itself. ## Steps - [ ] Write failing tests against `httptest` serving a **captured real 207 body** — generate one from the `.249` dogfood and paste it into a testdata file. Do not invent the XML; real servers differ from the spec in small ways. - [ ] Include a case with a quoted ETag, a `W/` weak ETag, a path with a space, and a non-ASCII filename. - [ ] Run them; confirm they fail. - [ ] Implement using `encoding/xml`. - [ ] Run tests; confirm they pass. - [ ] Commit: `git commit -s -m "feat(remote): PROPFIND-backed List and Stat"` ## Acceptance criteria - ETags are normalised identically on every path through the code (one helper, used everywhere). - Self-reference is excluded from `List`. - `List` output is sorted and canonical, matching the `MemRemote` contract so the engine cannot tell them apart.

Cordy added this to the phase-2-integration milestone

2026-09-10 17:40:58 +00:00

Cordy added the

remote

label

2026-09-10 17:40:58 +00:00

Cordy referenced this issue

2026-09-10 20:19:41 +00:00

Task 3: Path normalisation (NFC, separators) #3

Cordy commented

2026-09-11 01:50:09 +00:00

Author

Owner

Amendment — 2026-09-11: phase-1 hand-off (binding rulings Task 3 F1 and F5)

The phase-1 review made rulings that bind the adapters feeding names to the engine. They
amend this issue's gotcha "Decode, strip the base path, then pass through
sync.Normalise", which on its own lets distinct server names collapse into one engine path.

Task 3 F1 — names that cannot be normalised safely

For every URL-decoded href segment:

(a) A decoded name containing \ is never passed to Normalise. Normalise turns
\ into /, so notes\draft.txt would become the different path notes/draft.txt. Skip the
entry and report it per file, with a legible reason. It cannot exist on Windows either (spec §4:
"detect, skip, report — never fail the whole sync").
(b) Every listing checks that no two distinct native names normalise to the same engine
path. This covers backslash twins, NFC/NFD twins, and NFC compatibility singletons (U+F900
becomes U+8C48). Skip every member of such a group with a reason, as #14 does for case
collisions. Never let one entry shadow the other.
(c) A name refused under (a) or (b) must never count as missing, so it can never feed
a deletion. An entry that is simply left out of List while it has a state row reads as
"deleted on the server", and the engine would then delete the local copy.

Engine support, since commit e7cef2d (fix(sync): refuse non-canonical paths and their canonical twins):
SyncOnce refuses every listed path that is not in canonical form (Normalise(p) != p). It
also refuses the canonical path that shares its normal form, each as a Skip. An adapter can
therefore meet (c) by listing a refused entry under its raw decoded name. Omitting it does not
meet (c).

Task 3 F5 — keep the native name

Normalise also rewrites names that are not NFD (compatibility ideographs, the Stream-Safe
insertion after 31+ combining marks). A byte-preserving server can therefore hold names that
the NFC engine path does not reproduce.

Keep the native (decoded, un-normalised) name observed for every entry. Use a per-listing
engine-path → native-name map, or the equivalent in state.
Stat and, in #23, GET/PUT/DELETE/MOVE on an existing entry use that native name.
Percent-encoding the engine path is only for names the client itself creates. An NFC-rebuilt
URL for an NFD-stored name returns 404, which under R2 reads as a normal race one step away
from a deletion.

Note — paths outside the sync folder (final review X1)

Since commit 6a9a06b (fix(sync): refuse paths that could resolve outside the sync root), the engine
refuses on every platform any path that is empty, starts with /, or has an empty, . or ..
segment. It reports each one as a Skip.

The adapter should still never resolve .. itself: an href that decodes to something outside
the base path is not an entry of the sync folder. The same applies after %2e%2e and %2f
decoding.

## Amendment — 2026-09-11: phase-1 hand-off (binding rulings Task 3 F1 and F5) The phase-1 review made rulings that bind the adapters feeding names to the engine. They **amend this issue's gotcha "Decode, strip the base path, then pass through `sync.Normalise`"**, which on its own lets distinct server names collapse into one engine path. ### Task 3 F1 — names that cannot be normalised safely For every URL-decoded href segment: - **(a)** A decoded name containing `\` is **never passed to `Normalise`**. `Normalise` turns `\` into `/`, so `notes\draft.txt` would become the different path `notes/draft.txt`. Skip the entry and report it per file, with a legible reason. It cannot exist on Windows either (spec §4: "detect, skip, report — never fail the whole sync"). - **(b)** Every listing checks that **no two distinct native names normalise to the same engine path**. This covers backslash twins, NFC/NFD twins, and NFC compatibility singletons (U+F900 becomes U+8C48). Skip every member of such a group with a reason, as #14 does for case collisions. Never let one entry shadow the other. - **(c)** A name refused under (a) or (b) must **never count as missing**, so it can never feed a deletion. An entry that is simply left out of `List` while it has a state row reads as "deleted on the server", and the engine would then delete the local copy. Engine support, since commit e7cef2d (`fix(sync): refuse non-canonical paths and their canonical twins`): `SyncOnce` refuses every listed path that is not in canonical form (`Normalise(p) != p`). It also refuses the canonical path that shares its normal form, each as a Skip. An adapter can therefore meet (c) by listing a refused entry under its raw decoded name. Omitting it does not meet (c). ### Task 3 F5 — keep the native name `Normalise` also rewrites names that are not NFD (compatibility ideographs, the Stream-Safe insertion after 31+ combining marks). A byte-preserving server can therefore hold names that the NFC engine path does not reproduce. - Keep the **native (decoded, un-normalised) name** observed for every entry. Use a per-listing engine-path → native-name map, or the equivalent in state. - `Stat` and, in #23, `GET`/`PUT`/`DELETE`/`MOVE` on an **existing** entry use that native name. - Percent-encoding the engine path is only for names the client itself creates. An NFC-rebuilt URL for an NFD-stored name returns 404, which under R2 reads as a normal race one step away from a deletion. ### Note — paths outside the sync folder (final review X1) Since commit 6a9a06b (`fix(sync): refuse paths that could resolve outside the sync root`), the engine refuses on every platform any path that is empty, starts with `/`, or has an empty, `.` or `..` segment. It reports each one as a Skip. The adapter should still never resolve `..` itself: an href that decodes to something outside the base path is not an entry of the sync folder. The same applies after `%2e%2e` and `%2f` decoding.

Cordy referenced this issue

2026-09-11 01:50:21 +00:00

P2-3: GET, PUT, MKCOL, DELETE, MOVE — complete the Remote interface #23

Cordy referenced this issue from a commit

2026-09-11 01:55:32 +00:00

docs: CLAUDE.md lists the phase-1 hand-off amendments on #22, #23, #25, #26

Cordy commented

2026-09-11 04:12:19 +00:00

Author

Owner

Done

a7f4c36 — feat(remote): PROPFIND-backed List and Stat
4178669 — fix(remote): List fails on any answer that does not describe the folder asked for
9cef60d — fix(remote): a folder the server cannot list is a Skip, not a failed pass

What was built

Client.List/Client.Stat/Client.Quota over PROPFIND, matching the MemRemote contract (sorted, canonical, no self entry).
One normaliseETag helper for every entry; RFC1123 dates; <collection/> → IsDir; absolute/relative/encoded hrefs handled without ever resolving ...
Amendment F1/F5: names that cannot normalise safely (backslash, NFC/NFD/compat twins) are listed raw so the engine refuses them as Skips, never as deletes; Stat/descent use the native name via a per-listing map.
Entry.Unreadable + engine skip (P2-R14/P2-R15): a folder the server cannot address (400/404/"answer doesn't describe it") is Skipped with everything inside it, never emptied and never deleted from.
internal/sync/names.go (P2-R14): .cairn-meta.json, .cairn-state, .cairn-upload-* are refused before observe/Decide on both sides.

Tests

Real captures from a throwaway local cairnd (bd006ef, POSIX, cairnd-dev.sh, never .249) plus labelled edited copies for cases it can't produce (W/ ETags, hostile hrefs, twins).
Unit + engine e2e tests for every acceptance item and both fix rounds; 16 + 5 + 8 mutation runs killed; live smoke against a real cairnd re-run each round, server stopped after.
gofmt, go vet (host, windows/amd64, linux/arm64), go mod tidy -diff clean; full suite green locally each round.
CI: Forgejo Actions run #20 on commit 9cef60d, arm64, green — go vet ./... and go test ./... pass for remote/state/sync/vfs, total coverage 91.6%.

Acceptance criteria

ETags normalised identically on every path (one helper) — met: normaliseETag, reached only via entryFromProps for both List and Stat.
Self-reference excluded from List — met: only direct children counted; self falls out by depth and is required to be present (F1 fix).
List sorted and canonical, matching MemRemote — met: TestListMatchesMemRemote, TestListReturnsTheCapturedTreeSortedWithoutSelf.
Amendment F1(a) backslash never normalised — met, listed raw, engine refuses.
F1(b) NFC/NFD/compat twins all refused, none shadows another — met.
F1(c) a refused name never counts as missing — met, never feeds a delete.
F5 native name kept and used by Stat/descent — met via nativeNames map.
X1 never resolve ..; hostile-encoded hrefs (%2e%2e, %2f) excluded — met.

Rulings

P2-R1: push to main after clean review; close only once every Actions run for the pushed commit is green; comment + close, never edit issue bodies.
P2-R3: Client errors on missing paths satisfy both ErrNotFound and fs.ErrNotExist; MKCOL 405 → nil (kept in mind for #23).
P2-R10: real 207 bodies captured from a local cairnd (bd006ef, POSIX); owner's .249 credentials never touched; edited copies clearly labelled.
P2-R13: sync root is <server>/dav/home/; Config.BaseURL is that URL.
P2-R14: Cairn-reserved/server-internal names never sync; skipped before observe/Decide, so a skip never feeds a delete.
P2-R15: client tolerates cairnd deviations; names with % are double-decoded → a per-file/per-folder Skip, never a failed pass.
P2-R18: live-server testing uses cairnd-dev.sh only; never ~/Cairn, never .249, never owner's credentials.
Controller CV1–CV7 (progress.md): independently re-verified — CI-gate process, ext4/twin behaviour deferred to #29, live smoke reproducible, gofmt/vet/tidy/suite clean on all three targets, .249 ETag forms deferred to owner, Go 1.25.5 vs 1.26.5 static parity, RED/GREEN + mutation transcripts reproduced.

Deferred

Depth cap for server-side symlink loops (F4); e2e stub Get makes one assertion partly vacuous (F5); root-level errors read remote: PROPFIND : … (F6); exact-match native-name map handed off to #23 (F7); cairn-server-notes.md correction (ruling 7) still pending for the controller. None of these affect correctness of what shipped.

Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI.

**Done** - [a7f4c36](http://192.168.10.245/Cordy/cairn-desktop/commit/a7f4c3662216c9cbffc27cc145e0b6cef8079101) — feat(remote): PROPFIND-backed List and Stat - [4178669](http://192.168.10.245/Cordy/cairn-desktop/commit/4178669c14ca019c471f0a054b49a95da0bcd8f0) — fix(remote): List fails on any answer that does not describe the folder asked for - [9cef60d](http://192.168.10.245/Cordy/cairn-desktop/commit/9cef60def476c7d683948953eb9df16055100068) — fix(remote): a folder the server cannot list is a Skip, not a failed pass **What was built** - `Client.List`/`Client.Stat`/`Client.Quota` over PROPFIND, matching the `MemRemote` contract (sorted, canonical, no self entry). - One `normaliseETag` helper for every entry; RFC1123 dates; `<collection/>` → `IsDir`; absolute/relative/encoded hrefs handled without ever resolving `..`. - Amendment F1/F5: names that cannot normalise safely (backslash, NFC/NFD/compat twins) are listed raw so the engine refuses them as Skips, never as deletes; Stat/descent use the native name via a per-listing map. - `Entry.Unreadable` + engine skip (P2-R14/P2-R15): a folder the server cannot address (400/404/"answer doesn't describe it") is Skipped with everything inside it, never emptied and never deleted from. - `internal/sync/names.go` (P2-R14): `.cairn-meta.json`, `.cairn-state`, `.cairn-upload-*` are refused before observe/Decide on both sides. **Tests** - Real captures from a throwaway local cairnd (bd006ef, POSIX, `cairnd-dev.sh`, never .249) plus labelled edited copies for cases it can't produce (W/ ETags, hostile hrefs, twins). - Unit + engine e2e tests for every acceptance item and both fix rounds; 16 + 5 + 8 mutation runs killed; live smoke against a real cairnd re-run each round, server stopped after. - `gofmt`, `go vet` (host, windows/amd64, linux/arm64), `go mod tidy -diff` clean; full suite green locally each round. - CI: Forgejo Actions run [#20](http://192.168.10.245/Cordy/cairn-desktop/actions/runs/20) on commit 9cef60d, arm64, **green** — `go vet ./...` and `go test ./...` pass for remote/state/sync/vfs, total coverage 91.6%. **Acceptance criteria** - ETags normalised identically on every path (one helper) — met: `normaliseETag`, reached only via `entryFromProps` for both List and Stat. - Self-reference excluded from `List` — met: only direct children counted; self falls out by depth and is required to be present (F1 fix). - `List` sorted and canonical, matching `MemRemote` — met: `TestListMatchesMemRemote`, `TestListReturnsTheCapturedTreeSortedWithoutSelf`. - Amendment F1(a) backslash never normalised — met, listed raw, engine refuses. - F1(b) NFC/NFD/compat twins all refused, none shadows another — met. - F1(c) a refused name never counts as missing — met, never feeds a delete. - F5 native name kept and used by Stat/descent — met via `nativeNames` map. - X1 never resolve `..`; hostile-encoded hrefs (`%2e%2e`, `%2f`) excluded — met. **Rulings** - P2-R1: push to main after clean review; close only once every Actions run for the pushed commit is green; comment + close, never edit issue bodies. - P2-R3: Client errors on missing paths satisfy both `ErrNotFound` and `fs.ErrNotExist`; MKCOL 405 → nil (kept in mind for #23). - P2-R10: real 207 bodies captured from a local cairnd (bd006ef, POSIX); owner's .249 credentials never touched; edited copies clearly labelled. - P2-R13: sync root is `<server>/dav/home/`; `Config.BaseURL` is that URL. - P2-R14: Cairn-reserved/server-internal names never sync; skipped before observe/Decide, so a skip never feeds a delete. - P2-R15: client tolerates cairnd deviations; names with `%` are double-decoded → a per-file/per-folder Skip, never a failed pass. - P2-R18: live-server testing uses `cairnd-dev.sh` only; never `~/Cairn`, never .249, never owner's credentials. - Controller CV1–CV7 (progress.md): independently re-verified — CI-gate process, ext4/twin behaviour deferred to #29, live smoke reproducible, gofmt/vet/tidy/suite clean on all three targets, .249 ETag forms deferred to owner, Go 1.25.5 vs 1.26.5 static parity, RED/GREEN + mutation transcripts reproduced. **Deferred** - Depth cap for server-side symlink loops (F4); e2e stub `Get` makes one assertion partly vacuous (F5); root-level errors read `remote: PROPFIND : …` (F6); exact-match native-name map handed off to #23 (F7); cairn-server-notes.md correction (ruling 7) still pending for the controller. None of these affect correctness of what shipped. _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-11 04:12:24 +00:00

Cordy referenced this issue from Cordy/Cairn

2026-09-11 21:03:57 +00:00

WebDAV: in-flight upload temp files (.cairn-upload-*) appear in PROPFIND listings #495