Honor context cancellation while enumerating file storage

[jonyoder]

Description

enumerate() in the file storage server only checked its context on a channel send. A cancelled walk that was traversing subtrees producing no matching items therefore never reached a check: it ran the walk to completion and returned an empty list with a nil error, so a caller that had already given up was told the store held nothing. On a large tree it also waited out the whole walk to hear it.

The reader loop did not select on ctx.Done() at all, so a caller could sit for the full walkTimeout (five minutes by default) after asking to stop.

Two checks, and both are needed:

• ctx.Err() at the top of the WalkDir callback bounds the work: it stops the walker reading directories after the reader has returned. On the oversized stores where this matters, that wasted I/O is the reason to cancel in the first place.
• case <-ctx.Done() in the reader bounds the wait, and is the only one that can. WalkDir invokes the callback per directory entry, so a filesystem that has stopped answering ReadDir blocks inside WalkDir and the callback never runs again.

A cancellation is now a returned error, so Enumerate and EnumeratePrefix no longer log one at Error. A caller that cancelled got what it asked for, and reporting a fault every time a process shuts down mid-listing is a false operational signal.

Found by an unresolved RoboRev review on the prefix-enumeration branch; the finding survived to main.

Testing

Two new cases in FileEnumerationSuite:

• TestEnumerateCancelledWithNoMatchesReportsCancellation is the regression test. A cancelled context and a prefix matching nothing means no send is ever attempted, which is exactly the case the old send-side check could not see. Neutering both new checks makes it fail with obtained = nil against context canceled, reproducing the original bug precisely.
• TestEnumerateCancelledDoesNotReturnPartialResults pins that cancellation wins over buffered items, so a caller never gets a partial listing that reads as complete. Its comment states plainly that it passes on the pre-fix code too, since the pre-existing send-side check already covered it; it is there to hold that behaviour, not as coverage for this change.

On coverage, stated rather than glossed: the callback check has no test of its own, and cannot easily have one. Once the context is cancelled the reader's ctx.Done() case is ready on the first pass through the select, so it decides the returned value regardless; the callback check only changes how much of the tree gets read. Isolating that would need either an injected walk counter or a timing assertion, and a deliberately timing-sensitive test is not worth it here. Both code comments say so at the point a reader would wonder.

go test ./pkg/rsstorage/servers/file/ passes, including leaktest on both new cases.

go test ./pkg/rsstorage/... fails in servers/postgres and internal/integration_test on my host. Those are the Docker integration suites (failed to connect to user=admin database=postgres: lookup postgres: no such host) and I confirmed they fail identically with the change stashed, so they pre-date it.

Risks

Enumerate and EnumeratePrefix can now return context.Canceled where they previously returned an empty list and nil. Callers that ignore the error and read the slice see the same empty slice as before. Callers that check the error now see the cancellation they caused, which is the point.

[jonyoder]

Follow-up in 9051cb5, from RoboRev on the first commit: enumerate returns ctx.Err(), which is DeadlineExceeded for a context.WithTimeout and Canceled only for a context.WithCancel. My log guard matched Canceled alone, so a caller that set a deadline still got "Error enumerating storage" — exactly the noise the guard was meant to remove. Both forms are now covered by a named isContextEnded helper rather than the condition being repeated at the two call sites.

TestEnumerateDoesNotLogAnEndedContextAsAFault covers the deadline and cancel cases, and also asserts that a walk timeout is still logged, since that one is not the caller's doing. Mutation tested in both directions: narrowing the helper back to Canceled fails the deadline case, and widening it to always suppress fails the walk-timeout case.