Skip to content

cutover endpoint#3118

Merged
insipx merged 2 commits intomainfrom
push-rmpzuuomqxrs
Feb 25, 2026
Merged

cutover endpoint#3118
insipx merged 2 commits intomainfrom
push-rmpzuuomqxrs

Conversation

@insipx
Copy link
Contributor

@insipx insipx commented Feb 4, 2026

create an endpoint for the d14n cutover timestamp. closes #3031

@insipx insipx requested a review from a team as a code owner February 4, 2026 20:28
@insipx insipx mentioned this pull request Feb 4, 2026
@insipx
Copy link
Contributor Author

insipx commented Feb 4, 2026

This PR is part of a stack of 8 bookmarks:

  1. trunk()
  2. push-qqtpmktputny
  3. push-rmpzuuomqxrs ← this PR
  4. push-zwqsrxnyywwx
  5. push-tlyurqorvnrq
  6. push-wvmtovzklxpl
  7. push-szzlusvwrmur
  8. push-xutprvmtzsru
  9. push-tlzvqnqvukmt

Created with jj-stack

@claude
Copy link

claude bot commented Feb 4, 2026

Claude finished @insipx's task —— View job


Code Review - PR #3118

Reviewed on: February 25, 2026 at 15:00 UTC

Todo List

  • Read changed files to understand implementation
  • Analyze code quality and potential issues
  • Check test coverage
  • Provide concise feedback

Critical Issue

Type mismatch in Endpoint implementation (fetch_d14n_cutover.rs:11)

impl Endpoint for FetchD14nCutover {
    type Output = FetchD14nCutover;  // ❌ Should be FetchD14nCutoverResponse

This is a blocker. The Output type should be FetchD14nCutoverResponse, not FetchD14nCutover. This pattern is inconsistent with all other endpoints in the codebase (see GetInboxIds, HealthCheck, PublishClientEnvelopes, etc., which all use their respective response types). This will cause deserialization failures when the endpoint attempts to decode the gRPC response.

Fix:

type Output = FetchD14nCutoverResponse;

Minor Issues

  1. Proto cache collision potential (proto_cache.rs:33-46): The current implementation inserts both input and output types into METHOD_LOOKUP, which could overwrite entries if type names collide within the same package. While this may be intentional, consider adding a comment explaining the precedence or checking for existing entries before inserting.

  2. Test coverage: The actual integration test is marked #[ignore] (line 39), leaving only a path verification test. While understandable until the service is implemented, this contributes to the 59% patch coverage reported by codecov.

What's Good

  • Clean, minimal implementation following existing patterns
  • Proper use of pbjson_types::Empty for empty request body
  • Test structure is in place for when service is ready
  • Proto module generation looks correct

@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Feb 4, 2026

Add xmtp_api_d14n::endpoints::d14n::FetchD14nCutover to call /xmtp.migration.api.v1.D14nMigrationApi/FetchD14nCutover with an empty body and index RPC methods by output types in xmtp_proto::proto_cache::METHOD_LOOKUP

Introduce the FetchD14nCutover endpoint with tests, export it via the d14n module, add generated xmtp.migration.api.v1 code, and update METHOD_LOOKUP to map by output types. See fetch_d14n_cutover.rs and proto_cache.rs.

📍Where to Start

Start with the endpoint definition and tests in fetch_d14n_cutover.rs, then review the method map changes in proto_cache.rs.

Macroscope summarized 5300943.

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 59.25926% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.77%. Comparing base (18b4334) to head (5300943).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
..._api_d14n/src/endpoints/d14n/fetch_d14n_cutover.rs 42.10% 11 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3118   +/-   ##
=======================================
  Coverage   73.76%   73.77%           
=======================================
  Files         450      451    +1     
  Lines       56355    56380   +25     
=======================================
+ Hits        41569    41592   +23     
- Misses      14786    14788    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@insipx insipx mentioned this pull request Feb 4, 2026
@insipx insipx force-pushed the push-qqtpmktputny branch from 268c3c8 to f014a8d Compare February 4, 2026 21:18
@insipx insipx force-pushed the push-rmpzuuomqxrs branch from 0ea43ea to 26d963d Compare February 4, 2026 21:18
@claude
Copy link

claude bot commented Feb 9, 2026

Claude Code is working…

I'll analyze this and get back to you.

View job run

@insipx insipx force-pushed the push-qqtpmktputny branch from d945b46 to bf2be70 Compare February 9, 2026 21:48
@insipx insipx force-pushed the push-rmpzuuomqxrs branch from 7e7a69b to 5b0bb0a Compare February 9, 2026 21:48
Base automatically changed from push-qqtpmktputny to main February 9, 2026 22:14
@insipx insipx force-pushed the push-rmpzuuomqxrs branch 2 times, most recently from 3b5bfe7 to a610341 Compare February 10, 2026 15:09
@insipx insipx requested a review from a team as a code owner February 10, 2026 15:09
@insipx insipx mentioned this pull request Feb 17, 2026
@insipx insipx mentioned this pull request Feb 21, 2026
@insipx insipx enabled auto-merge (squash) February 25, 2026 15:20
@insipx insipx merged commit 6cb257f into main Feb 25, 2026
34 of 36 checks passed
@insipx insipx deleted the push-rmpzuuomqxrs branch February 25, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add cutover endpoints to the client

2 participants