fix: number multipart parts starting at 1 in uploadStream

[maximilize]

Problem

Fixes #1483 (and its duplicate #1482). In uploadStream (src/internal/client.ts) the multipart part counter is incremented before the part is uploaded, so the number sent to the server and pushed to eTags is one higher than the chunk's actual position:

let partNumber = 1
for await (const chunk of chunkier) {
  const oldPart = oldParts[partNumber]          // resume check uses the current number
  if (oldPart) {
    if (oldPart.etag === md5.toString('hex')) {
      eTags.push({ part: partNumber, etag: oldPart.etag })   // skip: current number
      partNumber++
      continue
    }
  }
  partNumber++                                   // upload: number bumped first
  const options = { query: qs.stringify({ partNumber, uploadId }), ... }
  ...
  eTags.push({ part: partNumber, etag })
}

Two consequences:

1. A fresh upload numbers its parts 2, 3, 4, … instead of 1, 2, 3, …. S3 tolerates a non-1 start as long as it is consistent, so this stays latent.
2. The skip branch records a chunk under partNumber while the upload branch records it under partNumber + 1. On a resumed upload that mixes skipped and re-uploaded chunks, the two branches disagree, producing gaps or duplicate part numbers in the eTags passed to completeMultipartUpload — a corrupted completion.

Fix

Use the current partNumber for the upload and increment only after a successful part, so the skip and upload branches agree and numbering is 1-based.

Tests

tests/unit/upload-stream-partnumber-test.js stubs the network-touching internals and drives uploadStream:

• a fresh 3-part upload now sends parts [1, 2, 3] (was [2, 3, 4]);
• a resume that already has parts 1 and 2 uploads only the third chunk, as part 3 (was 4), so skipped and uploaded parts stay consistent.

Both fail before the change and pass after; the full tests/unit suite (176 tests) stays green.

Note

This overlaps the stream loop touched by #1480 and #1481. #1481 (about unknown stream size) also happens to correct the counter but has been in CHANGES_REQUESTED for a while, and #1480 refactors the same loop while preserving the off-by-one. This PR is a minimal, standalone fix for the part-numbering bug with dedicated regression tests, independent of those changes.

Summary by CodeRabbit

• Bug Fixes
  • Fixed resumed multipart uploads to keep correct part numbering when some earlier parts are already present.
  • Ensured multipart completion reliably includes all parts in the correct order after resume scenarios.
• Tests
  • Added unit coverage for multipart resume behavior, including matching and mismatching previously uploaded parts, to prevent regressions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: eeb21ed8-7454-4d30-9fa9-c436373f23c5

📥 Commits

Reviewing files that changed from the base of the PR and between 968ebf7 and b588ebb.

📒 Files selected for processing (1)

• tests/unit/upload-stream-partnumber-test.js

---

📝 Walkthrough

Walkthrough

Corrects multipart upload part-number advancement during resume operations and adds unit tests for fresh uploads and resumed uploads with matching or mismatched existing parts.

Changes

Multipart upload numbering

Layer / File(s)	Summary
Correct numbering and regression coverage src/internal/client.ts, tests/unit/upload-stream-partnumber-test.js	uploadStream increments partNumber after recording a newly uploaded part. Tests cover fresh uploads, skipped matching parts, and re-uploaded mismatched parts.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• Issue 1482 — Directly tracks the multipart part-number resume bug fixed here.
• Issue 1483 — Addresses related partNumber handling in TypedClient.uploadStream.

Poem

A rabbit hops where numbers ran,
Now each chunk knows its proper span.
Etags guide the parts just right,
Resume hops avoid a flight.
“Hop hooray!” the tests unite.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: fixing multipart part numbering in uploadStream.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Warning

CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.

Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.

👉 Steps to fix this

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/upload-stream-partnumber-test.js`:
- Around line 85-101: Add a test alongside “numbers re-uploaded parts
consistently with skipped parts when resuming” that seeds an existing part with
a different etag/MD5, resumes the upload, and verifies the mismatched chunk is
re-uploaded using the same part number rather than an incremented number. Assert
the recorded sent and completed part numbers reflect that reused part number.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a559a141-fc79-42e4-9ea8-ffcc18f5857f

📥 Commits

Reviewing files that changed from the base of the PR and between f871afd and 968ebf7.

📒 Files selected for processing (2)

• src/internal/client.ts
• tests/unit/upload-stream-partnumber-test.js

[prakashsvmx]

@maximilize please address the review comments

[maximilize]

Added the missing case in b588ebb — the resume path where a stored part's etag is stale, so the chunk is re-uploaded at its own part number (2) instead of being shifted to 3. It asserts sentPartNumbers == [2, 3] and completedParts == [1, 2, 3].

Verified in a container: green with the fix, and red ([3, 4] instead of [2, 3]) when I reintroduce the pre-increment, so it actually guards the regression.