feat(tsconnect/wasm): add hasShares filter to listDrivePeers #19

Open
codinget wants to merge 2 commits from feat/drive-peers-has-shares into webnet
Owner

Fork-side half of the hasShares option for listDrivePeers (webnet/webnet issue #143, "Beyond that").

PeerCapabilityTaildriveSharer means a peer is allowed to share with us, not that it currently exposes anything, so listDrivePeers is a superset of the peers that actually have shares. The narrower list is only knowable by asking each peer, which is why this is opt-in.

Shape

listDrivePeers now takes an optional JS options object. {hasShares: true} keeps only peers we positively confirmed are exposing at least one share to us; omitted, undefined, null, or a non-boolean hasShares all mean false and the call does no network I/O at all, exactly as before.

Probe

Depth-1 PROPFIND on {peerAPIURL}/v0/drive/ over Dialer().PeerAPITransport() (the transport the wasm taildrop path already uses). The peer filters its share list by our permissions before listing, so any non-root <response> href is a share we can actually reach. Only 207 Multi-Status counts; a 404 (taildrive off), 403 (no cap for us) or non-WebDAV answer excludes the peer.

  • probes run in parallel, bounded at 8 — Go under wasm is single-threaded, so a higher limit buys little
  • 5s per probe, so one peer that connects and then stalls cannot hold up the listing
  • response reads are bounded, and parsing is streaming: it stops at the first child, so a peer with many shares costs no more than one with a single share
  • results are indexed, so the returned order still follows the netmap
  • a probe that fails drops that peer and is logged; it never fails the whole call

That last point makes this a positive filter: an unreachable peer is excluded, which is not the same as knowing it has no shares. Documented as such on both sides.

Layout

The probe and its multistatus parsing are in a new cmd/tsconnect/driveprobe package rather than in cmd/tsconnect/wasm/drive.go, because that file is package main importing syscall/js and so cannot be reached by go test. drive.go keeps only the JS argument decode and the peer-list filtering.

Verification

  • go test ./cmd/tsconnect/driveprobe/ — 9 tests pass, covering root-only vs root+child listings, prefix-stripped and prefix-kept hrefs, absolute URLs, percent-encoded and unicode share names, non-207 statuses, malformed and truncated XML, cancellation, error exclusion with order preserved, and both the parallelism and its bound (8 probes of 100ms complete in 0.10s)
  • wasm builds clean via the consuming repo's build-go; the ts_omit_drive stub path builds too
  • gofmt clean

Not verified: that the probes overlap under wasm specifically. The test above proves the concurrency structure overlaps natively; the wasm scheduler is cooperative and single-threaded, and no available tailnet grants taildrive caps, so there is no way to exercise the probe path end-to-end there. The design consciously assumes PeerAPITransport yields while waiting on the network, as the taildrop streaming path does.

Review round

Reviewed autonomously by Claude Sonnet 5 (review on webnet/webnet#166). It verified the DAV: namespace handling against both webdav implementations in play, confirmed ipnlocal strips the /v0/drive prefix so real peers answer prefix-stripped, checked the closure capture and timeout semantics of the parallel driver, found no panic path in the JS argument decode, and ran the Go tests under -race -count=10 without flakes.

Its one finding is fixed: hasChild counted any href that was not the collection, so a peer answering about an unrelated collection looked like it had shares. It now follows RFC 4918 §9.1 — the collection comes first, anything after it is a member, and the first href counts only if it is itself below the root. Fixing that exposed a second defect the review had not caught: the XML decoder may split character data across tokens, which the previous token-at-a-time check miscounted, so href text is now accumulated per element.

Trade-off worth a second opinion: because the prefix is stripped, a peer that omits the collection from its Depth-1 listing has a lone member that cannot be told apart from the collection. That case now reports "no shares" rather than guessing. It violates RFC 4918 §9.1 and neither Go implementation does it, but a tsconnect peer's drive handler is arbitrary JS and could. Reporting no shares is consistent with the positive-filter contract; the alternative is to assume any single entry is a member, which reintroduces the false positives the flag exists to remove.

Also noted, not changed: filterPeersWithShares passes context.Background(), so the only bound on a whole {hasShares: true} call is ceil(N/8) x 5s. An overall deadline is a reasonable follow-up and the option bag has room for it.

Fork-side half of the `hasShares` option for `listDrivePeers` (webnet/webnet issue #143, "Beyond that"). `PeerCapabilityTaildriveSharer` means a peer is *allowed* to share with us, not that it currently exposes anything, so `listDrivePeers` is a superset of the peers that actually have shares. The narrower list is only knowable by asking each peer, which is why this is opt-in. ## Shape `listDrivePeers` now takes an optional JS options object. `{hasShares: true}` keeps only peers we positively confirmed are exposing at least one share to us; omitted, `undefined`, `null`, or a non-boolean `hasShares` all mean false and the call does no network I/O at all, exactly as before. ## Probe Depth-1 `PROPFIND` on `{peerAPIURL}/v0/drive/` over `Dialer().PeerAPITransport()` (the transport the wasm taildrop path already uses). The peer filters its share list by our permissions before listing, so any non-root `<response>` href is a share we can actually reach. Only `207 Multi-Status` counts; a 404 (taildrive off), 403 (no cap for us) or non-WebDAV answer excludes the peer. - probes run in parallel, bounded at 8 — Go under wasm is single-threaded, so a higher limit buys little - 5s per probe, so one peer that connects and then stalls cannot hold up the listing - response reads are bounded, and parsing is streaming: it stops at the first child, so a peer with many shares costs no more than one with a single share - results are indexed, so the returned order still follows the netmap - a probe that fails drops that peer and is logged; it never fails the whole call That last point makes this a **positive** filter: an unreachable peer is excluded, which is not the same as knowing it has no shares. Documented as such on both sides. ## Layout The probe and its multistatus parsing are in a new `cmd/tsconnect/driveprobe` package rather than in `cmd/tsconnect/wasm/drive.go`, because that file is `package main` importing `syscall/js` and so cannot be reached by `go test`. `drive.go` keeps only the JS argument decode and the peer-list filtering. ## Verification - `go test ./cmd/tsconnect/driveprobe/` — 9 tests pass, covering root-only vs root+child listings, prefix-stripped and prefix-kept hrefs, absolute URLs, percent-encoded and unicode share names, non-207 statuses, malformed and truncated XML, cancellation, error exclusion with order preserved, and both the parallelism and its bound (8 probes of 100ms complete in 0.10s) - wasm builds clean via the consuming repo's `build-go`; the `ts_omit_drive` stub path builds too - `gofmt` clean Not verified: that the probes overlap *under wasm* specifically. The test above proves the concurrency structure overlaps natively; the wasm scheduler is cooperative and single-threaded, and no available tailnet grants taildrive caps, so there is no way to exercise the probe path end-to-end there. The design consciously assumes `PeerAPITransport` yields while waiting on the network, as the taildrop streaming path does. ## Review round Reviewed autonomously by Claude Sonnet 5 (review on webnet/webnet#166). It verified the `DAV:` namespace handling against both webdav implementations in play, confirmed `ipnlocal` strips the `/v0/drive` prefix so real peers answer prefix-stripped, checked the closure capture and timeout semantics of the parallel driver, found no panic path in the JS argument decode, and ran the Go tests under `-race -count=10` without flakes. Its one finding is fixed: `hasChild` counted any href that was not the collection, so a peer answering about an unrelated collection looked like it had shares. It now follows RFC 4918 §9.1 — the collection comes first, anything after it is a member, and the first href counts only if it is itself below the root. Fixing that exposed a second defect the review had not caught: the XML decoder may split character data across tokens, which the previous token-at-a-time check miscounted, so href text is now accumulated per element. **Trade-off worth a second opinion:** because the prefix is stripped, a peer that omits the collection from its Depth-1 listing has a lone member that cannot be told apart from the collection. That case now reports "no shares" rather than guessing. It violates RFC 4918 §9.1 and neither Go implementation does it, but a tsconnect peer's drive handler is arbitrary JS and could. Reporting no shares is consistent with the positive-filter contract; the alternative is to assume any single entry is a member, which reintroduces the false positives the flag exists to remove. Also noted, not changed: `filterPeersWithShares` passes `context.Background()`, so the only bound on a whole `{hasShares: true}` call is `ceil(N/8) x 5s`. An overall deadline is a reasonable follow-up and the option bag has room for it.
codinget added 1 commit 2026-08-03 01:32:28 +02:00
PeerCapabilityTaildriveSharer says a peer may share with us, not that it
does, so listDrivePeers is a superset of the peers actually exposing
shares. listDrivePeers now takes an options object; with
{hasShares: true} each candidate's taildrive root is probed with a
Depth-1 PROPFIND and only peers listing at least one share are kept.

The probe and its multistatus parsing live in cmd/tsconnect/driveprobe
so they can be tested without syscall/js. Probes run in parallel with a
bounded worker count, a per-probe timeout and a bounded response read;
a probe that fails drops that peer and is logged rather than failing
the call, so the filter is positive-only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codinget added the
Agent
claude-opus-5
Agentic
labels 2026-08-03 01:32:28 +02:00
codinget added 1 commit 2026-08-03 01:52:00 +02:00
hasChild treated any href that was not the collection as a share, so a
peer answering about an unrelated collection looked like it had shares.
Follow RFC 4918 §9.1 instead: the collection comes first and anything
after it is a member, with the first href counted only if it is itself
below the root.

Also accumulate href text across tokens; the XML decoder may split
character data, which the previous token-at-a-time check miscounted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
codinget marked the pull request as ready for review 2026-08-03 02:14:41 +02:00
You are not authorized to merge this pull request.
This pull request can be merged automatically.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/drive-peers-has-shares:feat/drive-peers-has-shares
git checkout feat/drive-peers-has-shares
Sign in to join this conversation.