Conversation
ApprovabilityVerdict: Approved Mechanical Go version upgrade from 1.25 to 1.26 across Dockerfiles, go.mod, and documentation. No runtime behavior changes—just version bumps and a .gitignore addition. Author owns nearly all modified files. No code changes detected at You can customize Macroscope's approvability policy. Learn more. |
11 tasks
Collaborator
Author
Update Go version across the project: - go.mod: go 1.25 → 1.26 - Dockerfile: golang:1.25-alpine → golang:1.26-alpine - devcontainer.Dockerfile: golang:1.25 → golang:1.26 - README.md: Go 1.20 → Go 1.26 (was outdated) CI workflows already use go-version-file: go.mod, so they pick up the new version automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d1cc14b to
529e2f5
Compare
This was referenced Apr 5, 2026
fbac
approved these changes
Apr 6, 2026
Collaborator
Author
3 tasks
neekolas
added a commit
that referenced
this pull request
Apr 8, 2026
## Summary
Migrate the notification server from V3 string-based topic identifiers (e.g., `/xmtp/mls/1/g-{hex}/proto`) to V4 binary topics as defined by `xmtpd/pkg/topic`. This enables support for V4 clients while maintaining full backwards compatibility for existing V3 consumers.
> **Stack:** This PR is stacked on #93 (Go 1.26 upgrade), which is a prerequisite for the `xmtpd` dependency.
## What changed
### New: Topic conversion module (`pkg/topics/`)
- `ParseV3Topic()` — converts V3 string topics to `*topic.Topic`
- `TopicToString()` — converts `*topic.Topic` back to V3 string representation
- `GetMessageTypeFromTopic()` — extracts message type from binary topic kind
- Removed dead code: `GetMessageType()`, `messageTypeByPrefix`, `HmacUpdates`
### Proto: New binary topic fields
- `Subscription.topic_bytes` (field 4) — binary topic, takes precedence over string `topic`
- `SubscribeRequest.topics_bytes` (field 3) — merged with string `topics`
- `UnsubscribeRequest.topics_bytes` (field 3) — merged with string `topics`
- **Existing field numbers are unchanged** — fully wire-compatible
### Database: TEXT → BYTEA migration
- **Migration 00003**: Renames `topic` to `topic_legacy`, adds `topic BYTEA`, converts conforming V3 topics inline using `lower()` for hex normalization, deletes non-conforming rows, deduplicates collapsed case variants, recreates indexes
- **Migration 00004**: Drops `topic_legacy` column
- SQLC schema and queries updated for `BYTEA` parameters
### Core interfaces (`pkg/interfaces/`)
- `Subscription.Topic` changed from `string` to `*topic.Topic`
- Added `Subscription.TopicString` for backwards-compatible JSON serialization
- Custom `MarshalJSON` emits both `"topic"` (string) and `"topicBytesB64"` (base64)
- `Subscriptions` interface methods now accept `[]*topic.Topic`
### API layer (`pkg/api/`)
- `normalizeTopics()` merges string + binary topic inputs with deduplication
- `normalizeSubscriptionInputs()` handles `SubscribeWithMetadata` with bytes-takes-precedence logic
- Invalid topics return `InvalidArgument` with descriptive error messages
### Listener (`pkg/xmtp/`)
- `processEnvelope()` converts V3 string topics to binary before subscription lookup
- Fast-path `strings.HasPrefix` check before expensive parsing
- Non-convertible topics logged at Info level and dropped (not an error)
- `getContext()` now uses `GetMessageTypeFromTopic()` for message classification
### Delivery services (`pkg/delivery/`)
- APNS and FCM payloads include both `"topic"` (reconstructed V3 string) and `"topicBytesB64"` (base64-encoded binary)
- HTTP delivery gets both fields automatically via `Subscription.MarshalJSON()`
## Important design considerations
### Intentional compatibility break for non-conforming topics
Non-conforming topics (e.g., non-hex identifiers like `test_installation`) are **deleted from the database** during migration and **rejected by the API** going forward. This is acceptable because non-conforming topics cannot be delivered on the V4 network. Existing tests were updated to use valid hex identifiers.
### Hex case normalization
The migration applies `lower()` inline during conversion without modifying `topic_legacy` at rest. This means two rows that differ only by hex casing (e.g., `g-ABCD` vs `g-abcd`) collapse to the same binary topic. A deduplication step (keep newest by ID) runs before recreating the unique index.
### String topic fidelity in delivery payloads
The `"topic"` string in delivery payloads is now **reconstructed** from the binary topic via `TopicToString()`, rather than forwarded verbatim from `Envelope.ContentTopic`. This means the string will always be lowercase hex. Consumers should treat topic strings as case-insensitive and prefer `"topicBytesB64"` when available.
### Envelope passthrough
The raw `Envelope` (containing string `ContentTopic`) is still passed through in `SendRequest.Message` for payload delivery and idempotency key generation. Only subscription lookups and message classification use the binary topic.
### `TopicString` is stored, not computed
`Subscription.TopicString` is populated once when reading from DB via `TopicToString()` and stored on the struct, rather than computed on each access. This avoids repeated hex encoding in delivery hot paths where multiple services read the same subscription.
## Test plan
- [x] 70 tests pass (`go test -p 1 ./...`)
- [x] `golangci-lint run` — no issues
- [x] `go build ./...` and `./dev/build` — binary compiles
- [x] Topic conversion: 14 tests covering ParseV3Topic, TopicToString, GetMessageTypeFromTopic
- [x] Migration: data conversion test with conforming, non-conforming, and case-duplicate rows
- [x] SQLC: BYTEA round-trip test
- [x] Subscriptions: 9 tests including nil topic guard
- [x] API: 19 tests including string topics, bytes topics, merged/deduped, invalid, empty, SubscribeWithMetadata precedence
- [x] Listener: 4 tests with binary topic conversion and non-convertible topic drop
- [x] Delivery: APNS and FCM payload tests verifying dual topic fields
- [x] Interfaces: JSON marshaling tests for both populated and nil topic cases
🤖 Generated with [Claude Code](https://claude.ai/claude-code)
<!-- Macroscope's pull request summary starts here -->
<!-- Macroscope will only edit the content between these invisible markers, and the markers themselves will not be visible in the GitHub rendered markdown. -->
<!-- If you delete either of the start / end markers from your PR's description, Macroscope will append its summary at the bottom of the description. -->
> [!NOTE]
> ### Migrate subscription topics from text to binary V4 format
> - Adds `topic_bytes` / `topics_bytes` binary fields to the `Subscription`, `SubscribeRequest`, and `UnsubscribeRequest` proto messages, allowing clients to pass V4 binary topics alongside or instead of string topics.
> - Adds DB migration [00004_binary_topics.up.sql](https://github.com/xmtp/example-notification-server-go/pull/94/files#diff-0546aa9393838c02b607a9cbf781188a059c8c96dfabfaa6d64f5803c64dbf61) that converts `subscriptions.topic` from `TEXT` to `BYTEA`, parsing V3 topic strings into kind-prefixed bytes, deleting non-convertible rows, and deduplicating on `(installation_id, topic)`.
> - Updates the `Subscriptions` interface and all implementations so `Subscribe`, `Unsubscribe`, and `GetSubscriptions` accept `*topic.Topic` objects instead of strings; SQL queries are updated to use `bytea[]` parameters throughout.
> - Adds `topics.ParseV3Topic` and `topics.TopicToString` helpers for converting between string and structured topic representations; removes the old `GetMessageType(envelope)` function in favour of `GetMessageTypeFromTopic(*topic.Topic)`.
> - Updates APNS and FCM delivery to populate the notification `topic` field from `Subscription.Topic` rather than `envelope.ContentTopic`.
> - Risk: the up migration deletes rows whose topic cannot be parsed as a valid V3 topic string and collapses duplicates, which is irreversible without the down migration.
>
> <!-- Macroscope's review summary starts here -->
>
> <sup><a href="https://app.macroscope.com">Macroscope</a> summarized c7d7e68.</sup>
> <!-- Macroscope's review summary ends here -->
>
<!-- macroscope-ui-refresh -->
<!-- Macroscope's pull request summary ends here -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Upgrade Go toolchain from 1.25 to 1.26 across the project. This is a prerequisite for the V4 binary topics migration, which depends on
github.com/xmtp/xmtpd@v1.3.0— a module that declaresgo 1.26in itsgo.mod.Changes
go.mod:go 1.25→go 1.26Dockerfile:golang:1.25-alpine→golang:1.26-alpinedev/docker/devcontainer.Dockerfile:golang:1.25→golang:1.26README.md: Updated prerequisite fromGo 1.20(was outdated) toGo 1.26.gitignore: Added.omx/directoryDesign considerations
go-version-file: go.mod, so they automatically pick up the new version.Test plan
go build ./...passesgo test -p 1 ./...passes (39 tests)golangci-lint runreports no issues🤖 Generated with Claude Code
Note
Upgrade Go from 1.25 to 1.26
Updates the Go version to 1.26 across the module definition, production Dockerfile, dev container, and README prerequisites.
Macroscope summarized 529e2f5.