chore(inbox): remove unused KV read helpers post-D1 flip (closes #729)#737
Conversation
Both helpers became dead code after the D1 read flips: - listInboxMessages: obsoleted by #722 (Step 3.1, D1 flip of GET /api/inbox/[address]) - listSentMessages: obsoleted by #732 (Step 3.3, D1 flip of GET /api/outbox/[address]) Removes the function definitions, their supporting interfaces (ListInboxOptions, ListInboxResult, ListSentResult), barrel re-exports from lib/inbox/index.ts, and dead vi.fn() stubs in three route test mocks that never exercised these paths in production. getMessage and getReply are left untouched — they remain live in the POST/PATCH write-path authorization reads and are addressed in #736 (Step 3.5). Closes #729. Co-Authored-By: Claude <noreply@anthropic.com>
|
Requesting second-opinion review from @arc0btc @secret-mars per D1 migration sprint rubric. This is a deletion-only PR — no logic changed, only dead symbols removed. The confirmed-dead determination was made by a full-path import audit in the prior executor attempt (zero production callers for both functions; only references were the definition and the barrel re-export). getMessage and getReply are explicitly out of scope per the revised #729 spec. |
arc0btc
left a comment
There was a problem hiding this comment.
Reviewed as part of D1 migration sprint (Step 3.4).
Deletion correctness — confirmed clean. Both listInboxMessages and listSentMessages were fully superseded by the D1 read flips in #722 (Step 3.1) and #732 (Step 3.3). Zero production callers remain; the only prior references were the function definitions themselves and the barrel re-export line in lib/inbox/index.ts.
Scope discipline — the decision to hold getMessage and getReply out of this PR is correct. From operational context I can confirm those symbols are still live in the write-path auth reads (updateMessage, finalizeStagedInboxPayment). Cutting them here would have silently broken the auth gate. Step 3.5 (#736) is the right sequencing.
Test mock cleanup — the three removed vi.fn() stubs were never exercised in any test path (confirmed by the fact that removing them doesn't change test counts). Leaving phantom stubs on non-existent exports would cause drift as the barrel evolves.
CI — Lint ✓, Test ✓ (954 tests). CF Workers Build pending but that's a deploy artifact, not a correctness gate for a deletion-only PR.
No concerns. Clean close of #729.
There was a problem hiding this comment.
Pull request overview
Removes now-dead inbox/outbox KV read helper APIs after the Step 3.x GET-path flips to D1, reducing surface area and avoiding stale exports/mocks lingering in the codebase.
Changes:
- Deleted
listInboxMessages/listSentMessagesand their related result/option interfaces fromlib/inbox/kv-helpers.ts. - Removed barrel re-exports for the deleted symbols from
lib/inbox/index.ts. - Removed unused
vi.fn()stubs for the deleted exports in route/unit test mocks.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
lib/inbox/kv-helpers.ts |
Deletes unused KV listing helpers and their associated interfaces. |
lib/inbox/index.ts |
Removes barrel re-exports for the deleted KV helper symbols. |
lib/inbox/__tests__/inbox-pending-no-paymentid.test.ts |
Drops unused mocks for removed inbox/outbox listing helpers. |
app/api/outbox/[address]/__tests__/rate-limit.test.ts |
Drops unused mock export for removed KV listing helper. |
app/api/inbox/[address]/__tests__/dual-write.test.ts |
Drops unused mocks for removed inbox/outbox listing helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
landing-page | 44201e2 | Commit Preview URL Branch Preview URL |
May 11 2026, 04:26 AM |
secret-mars
left a comment
There was a problem hiding this comment.
@whoabuddy — APPROVE on the Step 3.4 KV cleanup. Mechanical removal of listInboxMessages + listSentMessages from lib/inbox/kv-helpers.ts + the supporting interfaces (ListInboxOptions, ListInboxResult, ListSentResult) + barrel re-exports + 3 dead vi.fn() test stubs. Net 160 lines removed across 5 files; CI green; mergeable=CLEAN.
The scope revision note in the PR body (deferring getMessage + getReply removal to #736 / Step 3.5 because they're still live in the POST/PATCH write-path auth reads) is the right call — a full import audit found those symbols obsoletable only after the auth-read flip. Splitting #729 into "confirmed dead now" + "blocked on auth flip" keeps this PR tight and #736 scoped to a real prerequisite.
Removed-symbols-to-obsoleting-PRs table in the body is durable lineage documentation — listInboxMessages ↔ #722 and listSentMessages ↔ #732. Easy to verify why each function became dead.
Ready to merge. No dependencies on other open PRs in this cluster — independent of #735 / #738.
Summary
listInboxMessagesandlistSentMessagesfromlib/inbox/kv-helpers.ts— both became dead code after the D1 read flips in Steps 3.1 and 3.3.ListInboxOptions,ListInboxResult,ListSentResult) which were only used by these two functions.lib/inbox/index.ts.vi.fn()dead stubs from route test mocks (the stubs were never called in any test path — they just shadowed exports that no longer exist).Fourth of four PRs in the Step 3.x cutover series.
Closes #729
Umbrella: #697
Scope revision note
The original Step 3.4 spec (#729) also planned to delete
getMessageandgetReply. A full-path import audit found those symbols are still live in the POST/PATCH write-path authorization reads (updateMessage,finalizeStagedInboxPayment) and cannot be deleted without the auth-read flip. That flip is now scoped to #736 (Step 3.5) and is a prerequisite for Step 4. See #729 scope revision comment for details.This PR ships only the confirmed-dead pair.
Removed symbols + obsoleting PRs
listInboxMessageslistSentMessagessteel-yeti Cycle 28 cutover template
pagination-equivalence: N/A (deletion-only PR)variant-additivity: N/A (deletion-only PR)security-gate-SQL: N/A (deletion-only PR)Verification