fix: restore ServiceStatus and ServiceDiscoveryMessage to archipelago.proto - #453
Merged
Merged
Conversation
Test this pull request
|
LautaroPetaccio
approved these changes
Jul 28, 2026
This was referenced Jul 28, 2026
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.
Restores
ServiceStatusandServiceDiscoveryMessagetokernel/comms/v3/archipelago.proto. These types are already in production use — they just never made it ontomain.This is a cherry-pick of the two original commits, authorship preserved, not a rewrite.
Where it was dropped
Both messages were authored in September 2023 on
feat/default-realm:10343e9feat: add new archipelago message— added both messages,current_timeasuint32861829bfix: timestamp— widenedcurrent_timetouint64That branch was never merged. Verified against current
main:A sweep of every remote branch finds
ServiceDiscoveryMessageon exactly one:feat/default-realm. So for ~3 yearsmainhas been missing types that production depends on.Why it still works
archipelago-workersdoesn't install a released@dcl/protocol. All three services pin a CDN branch build — note the/branch/path segment:That tarball was built from the unmerged branch and does contain both messages, so
archipelago-coreandarchipelago-statshave compiled and run fine the whole time. The gap is invisible to anyone consuming the published artifact and only shows up when generating from the repository.Its source commit
3946d15is no longer present in this repo at all — the branch it was built from appears to have been deleted after publishing. Production therefore depends on types that exist only in an orphaned build artifact, with no reachable source. That is worth closing on its own merits, independently of the change below.Why now
Pulse is taking over island/cluster authorship from
archipelago-coreand must publish the same NATS feeds soarchipelago-statskeeps working after core is decommissioned:engine.islands→IslandStatusMessage— already onmain✅engine.discovery→ServiceDiscoveryMessage— missing ❌Pulse generates C# directly from this repository rather than from an npm tarball, so
protochas nothing to emit forengine.discoveryand the build fails on the missing type.Consumer for reference —
archipelago-statsdecodesengine.discoveryinto itscoreStatuscomponent, which servesGET /core-status:Verification
The resulting
archipelago.protois byte-identical (line endings aside) to the one inside the production tarball, so nothing on the wire changes:Note on
current_time: it must beuint64. Epoch milliseconds (~1.75e12) overflowuint32, and the health check above is a delta againstDate.now()— a truncated timestamp would make the service read as permanently unhealthy.861829bis included precisely for this reason; cherry-picking only10343e9would land the broken width.Additive change, new messages only. No existing message, field, or tag number is touched.