fs: replace Copy's == backend comparison with a SameBackend interface - #180
Merged
Conversation
Base automatically changed from
claude/github-issue-173-walk-nil-deref
to
main
August 26, 2026 13:32
fs.Copy compared dstFS == srcFS to decide whether to use a CopyFS's server-side Copy method. That produced false negatives (two distinct *BucketFS values over the same bucket/client never matched) and could panic outright when both values shared a non-comparable dynamic type. Add an optional SameBackend interface FS implementations can use to report backend identity, and have Copy consult it on dstFS instead of comparing interface values directly. Implement it for s3.BucketFS (bucket name plus a reflect-guarded client comparison, so a non-comparable client can't reproduce the panic one level down) and, as a one-line bonus, for local.FS (same absolute root path). Fixes #174
srerickson
force-pushed
the
claude/github-issue-174-b6u0mq
branch
from
August 26, 2026 13:37
5a78ea0 to
d8c84e6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #179.
Summary
fs.CopycompareddstFS == srcFSto decide whether to use aCopyFS's server-sideCopymethod. That produced false negatives (two distinct*BucketFSvalues over the same bucket/client never matched) and could panic outright when both values shared a non-comparable dynamic type.SameBackendinterface FS implementations can use to report backend identity, and haveCopyconsult it ondstFSinstead of comparing interface values directly.SameBackendfors3.BucketFS: bucket name plus a reflect-guarded client comparison, so a non-comparable client can't reproduce the panic one level down.SameBackendforlocal.FSas a one-liner (same absolute root path).Fixes #174 (and settles #125, per that issue's note: whichever design wins between them, one issue closes).
Test plan
fs/fs_test.go(new):Copydispatch tests — same-backend, different-backend, no-SameBackend, and a reproduction of the original panic with a non-comparable dynamic type.fs/s3/fs_test.go:TestSameBackend_Mock(including the non-comparable-client guard) andTestCopyDispatch_Mock, which asserts the s3 fast path viamock.Calls()(oneCopyObject, zeroGetObject/PutObject).go build ./...,go vet ./...,go test ./... -count=5 -raceall pass.Generated by Claude Code