Implement LFS multipart upload support with S3 by Claude · Pull Request #163 · matrixhub-ai/hfd

Claude · 2026-03-31T09:39:05Z

Implements the Git LFS multipart transfer mode proposal for efficient large file uploads to S3 storage.

Changes

Storage interface: Added SignMultipartPutter interface with methods for initiating, completing, and aborting multipart uploads. Added MultipartPart and MultipartUpload structs.
S3 implementation: Implemented multipart operations in s3Storage:
- SignMultipartPut() - Initiates upload, splits files into 100MB parts, generates presigned URLs
- CompleteMultipartUpload() - Finalizes upload with part ETags from client
- AbortMultipartUpload() - Cancels upload and cleans up parts
Transfer negotiation: Added negotiateTransfer() to select between "basic" and "multipart" protocols based on:
- Client-requested transfers in batch API
- Storage backend capabilities
- File size threshold (≥100MB)
- Operation type (upload only)
Batch API: Updated lfsRepresent() to generate multipart-specific actions:
- parts[] - Array of presigned part URLs with position/size
- verify - Completion endpoint with upload_id and part_etags in params
- abort - Cancellation endpoint
Endpoints: Added /objects/{oid}/multipart/verify and /objects/{oid}/multipart/abort handlers

Protocol Flow

// Client batch request
{
  "transfers": ["multipart", "basic"],
  "operation": "upload",
  "objects": [{"oid": "abc...", "size": 200000000}]
}

// Server response
{
  "transfer": "multipart",
  "objects": [{
    "oid": "abc...",
    "actions": {
      "parts": [
        {"href": "https://s3.../part1", "pos": 0, "size": 100000000},
        {"href": "https://s3.../part2", "pos": 100000000, "size": 100000000}
      ],
      "verify": {
        "href": "https://server/objects/abc.../multipart/verify",
        "params": {"upload_id": "xyz", "part_count": 2}
      }
    }
  }]
}

Files <100MB continue using basic transfer. Falls back to basic if storage doesn't support multipart.

- Add SignMultipartPutter interface for multipart uploads - Implement S3 multipart upload operations (initiate, complete, abort) - Add transfer protocol negotiation logic for multipart - Update batch API to support multipart transfer mode - Add multipart-specific action types (parts, verify, abort) - Implement verify endpoint with params support for ETags - Add abort endpoint for multipart uploads - Files >= 100MB use multipart upload with 100MB parts - Follows Git LFS multipart transfer proposal spec Agent-Logs-Url: https://github.com/matrixhub-ai/hfd/sessions/5bcefe7e-2ca2-403b-8902-7272ea90f018 Co-authored-by: wzshiming <6565744+wzshiming@users.noreply.github.com>

Copilot

Pull request overview

Adds Git LFS multipart upload support for S3-backed storage, exposing multipart-specific batch actions and new verify/abort endpoints to complete or cancel multipart uploads.

Changes:

Introduces multipart-capable storage abstractions (SignMultipartPutter, multipart structs) and implements them for S3.
Extends the LFS batch API to negotiate a transfer protocol and emit multipart actions (parts, verify, abort) for uploads.
Adds HTTP endpoints to verify (complete) and abort multipart uploads.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
pkg/lfs/storage.go	Adds multipart upload interfaces/types for storage backends.
pkg/lfs/s3_storage.go	Implements S3 multipart initiation, completion, and abort using presigned URLs.
pkg/backend/lfs/handler.go	Registers new multipart verify/abort routes.
pkg/backend/lfs/handler_git_lfs.go	Negotiates transfer type; emits multipart actions; adds multipart verify/abort handlers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T10:10:06Z

+					partNumbers := make([]int, len(multipartUpload.Parts))
+					for i, part := range multipartUpload.Parts {
+						partNumbers[i] = part.PartNumber
+					}
+


partNumbers is computed but never used. This will fail to compile in Go due to an unused local variable. Either remove it or include it in the response (e.g., if clients need explicit part numbers).

Suggested change

partNumbers := make([]int, len(multipartUpload.Parts))

for i, part := range multipartUpload.Parts {

partNumbers[i] = part.PartNumber

}

Copilot · 2026-03-31T10:10:07Z

+				multipartUpload, err := multipartSigner.SignMultipartPut(rv.Oid, rv.Size)
+				if err != nil {
+					slog.ErrorContext(ctx, "failed to initiate multipart upload", "oid", rv.Oid, "error", err)
+					// Fallback to basic transfer
+					rep.Actions["upload"] = h.createBasicUploadAction(ctx, rv, user)
+				} else {


When multipart initiation fails, the code falls back to adding only the basic upload action but does not add the corresponding verify action. LFS clients expect both for basic uploads; returning only upload will break the upload flow.

Copilot · 2026-03-31T10:10:07Z

+				}
+			} else {
+				// Storage doesn't support multipart, fallback to basic
+				rep.Actions["upload"] = h.createBasicUploadAction(ctx, rv, user)


If the storage does not support multipart, this branch falls back to adding only the basic upload action but never adds the basic verify action. That leaves the object without a verify endpoint and will break basic uploads for large objects when transfer was negotiated as multipart.

Suggested change

rep.Actions["upload"] = h.createBasicUploadAction(ctx, rv, user)

rep.Actions["upload"] = h.createBasicUploadAction(ctx, rv, user)

// Also provide a verify action, same as for basic transfer

verifyHeader := make(map[string]string)

verifyLink := rv.verifyLink()

if h.tokenSignValidator != nil {

if token, err := h.tokenSignValidator.Sign(ctx, http.MethodPost, verifyLink, user.User, tokenExpiration); err != nil {

slog.WarnContext(ctx, "failed to sign token for LFS verify link", "oid", rv.Oid, "error", err)

} else if token != "" {

verifyHeader["Authorization"] = "Bearer " + token

}

} else if len(rv.Authorization) > 0 {

verifyHeader["Authorization"] = rv.Authorization

}

rep.Actions["verify"] = &lfsLink{Href: verifyLink, Header: verifyHeader}

Copilot · 2026-03-31T10:10:07Z

+// negotiateTransfer determines which transfer protocol to use based on client request and server capabilities.
+func negotiateTransfer(requestedTransfers []string, operation string, storage lfs.Storage) string {
+	// Default to basic if no transfers requested
+	if len(requestedTransfers) == 0 {
+		return "basic"
+	}
+
+	// Check if multipart is supported by storage
+	_, supportsMultipart := storage.(lfs.SignMultipartPutter)
+
+	// Only use multipart for uploads
+	if operation == "upload" && supportsMultipart {
+		// Check if client requested multipart
+		for _, t := range requestedTransfers {
+			if t == "multipart" {
+				return "multipart"
+			}
+		}
+	}
+
+	// Fallback to basic if supported
+	for _, t := range requestedTransfers {
+		if t == "basic" {
+			return "basic"
+		}
+	}
+
+	// Default to basic
+	return "basic"
+}


negotiateTransfer() selects a single batch-level transfer without considering per-object behavior (size threshold and multipart initiation failures). As written, the batch response can advertise transfer: multipart while some objects return only basic upload/verify actions, which can break clients because the transfer adapter is chosen from the batch-level transfer field. Consider negotiating multipart only when it can be used consistently for all objects in the batch (or always return multipart-style actions when transfer is multipart, even for smaller objects).

Copilot · 2026-03-31T10:10:07Z

+	var req struct {
+		Oid    string         `json:"oid"`
+		Size   int64          `json:"size"`
+		Params map[string]any `json:"params"`
+	}


The multipart verify endpoint expects oid and size as top-level JSON fields, while upload_id/part_etags are nested under params. However, the batch response’s multipart verify action publishes params that include oid and size, which may lead clients to send them under params instead of top-level and trigger the OID/size validation failure. Align the advertised actions.verify.params shape with what handleMultipartVerify actually decodes (or make the handler accept oid/size from params).

Copilot · 2026-03-31T10:10:08Z

+	result, err := s.s3.CreateMultipartUpload(createInput)
+	if err != nil {
+		return nil, fmt.Errorf("failed to initiate multipart upload: %w", err)
+	}


uploadID := *result.UploadId dereferences a pointer returned by the AWS SDK without a nil check. If the SDK ever returns a response missing UploadId, this will panic. Prefer validating result/result.UploadId and returning a descriptive error instead of panicking.

Suggested change

}

}

if result == nil || result.UploadId == nil {

return nil, fmt.Errorf("failed to initiate multipart upload: missing UploadId in response")

}

Copilot · 2026-03-31T10:10:08Z

+	// Sort by part number (S3 requires this)
+	// Using a simple bubble sort since parts are usually small
+	for i := 0; i < len(completedParts); i++ {
+		for j := i + 1; j < len(completedParts); j++ {
+			if *completedParts[i].PartNumber > *completedParts[j].PartNumber {
+				completedParts[i], completedParts[j] = completedParts[j], completedParts[i]
+			}
+		}
+	}


Sorting completedParts with a nested loop is O(n²) and can become very expensive with large multipart uploads (up to 10,000 parts). Use sort.Slice (or similar) to sort by PartNumber in O(n log n).

Initial plan

e338bc6

Claude AI assigned Claude and wzshiming Mar 31, 2026

Claude started work on behalf of wzshiming March 31, 2026 09:39 View session

Claude AI linked an issue Mar 31, 2026 that may be closed by this pull request

Support lfs multipart with s3 #160

Closed

Claude AI changed the title ~~[WIP] Copilot Request~~ Implement LFS multipart upload support with S3 Mar 31, 2026

Claude finished work on behalf of wzshiming March 31, 2026 09:58

Claude AI requested a review from wzshiming March 31, 2026 09:58

wzshiming marked this pull request as ready for review March 31, 2026 10:02

Copilot AI review requested due to automatic review settings March 31, 2026 10:02

Copilot started reviewing on behalf of wzshiming March 31, 2026 10:03 View session

wzshiming marked this pull request as draft March 31, 2026 10:05

Copilot AI reviewed Mar 31, 2026

View reviewed changes

wzshiming force-pushed the master branch from 79b025e to 25548b3 Compare April 25, 2026 04:41

wzshiming force-pushed the master branch 2 times, most recently from 7ce99ae to 73bc92d Compare May 22, 2026 10:31

wzshiming closed this Aug 14, 2026

Navigation Menu

Uh oh!

Implement LFS multipart upload support with S3 - #163

Implement LFS multipart upload support with S3#163
wzshiming with Claude wants to merge 2 commits into
masterfrom
claude/fix-253666302-1127486954-3280851a-22cf-4dbb-bc53-9375d961c7ab

Claude AI commented Mar 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Copilot AI Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-				rep.Actions["upload"] = h.createBasicUploadAction(ctx, rv, user)
+				rep.Actions["upload"] = h.createBasicUploadAction(ctx, rv, user)
+				// Also provide a verify action, same as for basic transfer
+				verifyHeader := make(map[string]string)
+				verifyLink := rv.verifyLink()
+				if h.tokenSignValidator != nil {
+					if token, err := h.tokenSignValidator.Sign(ctx, http.MethodPost, verifyLink, user.User, tokenExpiration); err != nil {
+						slog.WarnContext(ctx, "failed to sign token for LFS verify link", "oid", rv.Oid, "error", err)
+					} else if token != "" {
+						verifyHeader["Authorization"] = "Bearer " + token
+					}
+				} else if len(rv.Authorization) > 0 {
+					verifyHeader["Authorization"] = rv.Authorization
+				}
+				rep.Actions["verify"] = &lfsLink{Href: verifyLink, Header: verifyHeader}

-	}
+	}
+	if result == nil || result.UploadId == nil {
+		return nil, fmt.Errorf("failed to initiate multipart upload: missing UploadId in response")
+	}

Uh oh!

Conversation

Claude AI commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Protocol Flow

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Claude AI commented Mar 31, 2026 •

edited

Loading