fix: drop legacy multipart listing overload

[TylerHillery]

What kind of change does this PR introduce?

small fix to follow up on #1346

[Copilot]

🟢 Approval recommended

The change is narrowly scoped to removing an obsolete Postgres function overload and does not introduce behavioral risk beyond the intended function-resolution cleanup.

Pull request overview

This PR removes the legacy 6-argument overload of storage.list_multipart_uploads_with_delimiter from the tenant migration so that callers using 6 arguments resolve to the newer function signature via the defaulted trailing parameter.

Changes:

• Drops the old storage.list_multipart_uploads_with_delimiter(text, text, text, integer, text, text) overload to avoid function-resolution ambiguity.
• Keeps the newer list_multipart_uploads_with_delimiter definition as the canonical implementation in the migration.

File summaries

File	Description
migrations/tenant/0070-list-objects-with-versions.sql	Drops the legacy multipart listing overload so 6-arg calls bind to the newer signature.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coveralls]

Coverage Report for CI Build 34506765857

Coverage decreased (-0.009%) to 82.675%

Details

• Coverage decreased (-0.009%) from the base build.
• Patch coverage: No coverable lines changed in this PR.
• 1 coverage regression across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
src/http/routes/s3/index.ts	1	87.42%

---

Coverage Stats

Relevant Lines:	14076
Covered Lines:	12096
Line Coverage:	85.93%
Relevant Branches:	8591
Covered Branches:	6644
Branch Coverage:	77.34%
Branches in Coverage %:	Yes
Coverage Strength:	678.85 hits per line

---

💛 - Coveralls

[claude]

I reviewed this PR and didn't find any bugs. Because this modifies an already-merged migration file (0070) instead of adding a new one, and that pattern can be unsafe if 0070 has already applied against any tenant database, a human look would still be worthwhile.

What was reviewed:

• Confirmed the DROP is needed because the new CREATE signature adds a 7th parameter (raw_prefix_param) — Postgres can't change a function's parameter list via CREATE OR REPLACE FUNCTION, matching the existing DROP pattern used for list_objects_with_delimiter later in the same file.
• Checked the migration runner (src/internal/database/migrations/migrate.ts): migrations already recorded by id are never re-run, and a changed hash on an already-applied migration either throws (hash mismatch) or is silently accepted without re-executing the SQL — so if 0070 already ran on any tenant, this added DROP/CREATE would never take effect there.
• The rest of the migration (the versioning/pagination functions) is unchanged from the base commit.

Extended reasoning...

Overview

The diff is a one-line addition to migrations/tenant/0070-list-objects-with-versions.sql, inserting a DROP FUNCTION IF EXISTS storage.list_multipart_uploads_with_delimiter(text, text, text, integer, text, text) immediately before the CREATE OR REPLACE FUNCTION for the same name whose new signature has 7 parameters. This is a legitimate fix: Postgres cannot alter a function's parameter list via CREATE OR REPLACE, so an explicit DROP is required when the arity changes — the file already does this same DROP+CREATE dance for storage.list_objects_with_delimiter further down, so the pattern is consistent with existing code in the same file.

Security risks

None from the SQL itself — it is a DDL statement with no user input, and it mirrors an existing pattern in the file.

Level of scrutiny

The line itself is trivial, but it edits a migration file (0070) that was merged into main only ~41 minutes before this fix commit (f6493bf at 11:30, 4e34a1c at 12:11, same day). I traced how this repo's migration runner (src/internal/database/migrations/migrate.ts) tracks applied migrations: filterMigrations skips any migration whose id is already recorded in the migrations table, and validateMigrationHashes/refreshMigrationHash either throw on a hash mismatch or silently rewrite the stored hash without re-running the SQL. That means if migration 0070 already executed against any tenant database (multitenant fleet, progressive rollout, or a self-hosted deployment built from main) before this fix landed, the newly added DROP FUNCTION statement in 0070 will never run for that database — either the migration run for that tenant will error out (hash mismatch, if dbRefreshMigrationHashesOnMismatch is off) or it will silently accept the new hash and leave the stale 6-arg overload in place (if that flag is on). The conventional-and-safer fix would be a new migration file (e.g. 0072-...sql) rather than editing an already-shipped one. Whether this is actually a problem depends entirely on deployment timing that isn't visible from the repo — I can't confirm from the code alone whether 0070 has already been applied anywhere, which is exactly the kind of operational fact a human with deploy visibility should confirm before merging.

Other factors

This is a narrow, single-line diff with no test changes, so the bug-hunting pass reported no findings. My defer note above is limited to raising the migration-timing question for human judgment; I'm not asserting it is definitely broken, since I lack visibility into whether 0070 has been deployed to any real tenant yet.