Push-driven team state via team_changed + consumer-loop resilience - #65
Merged
Conversation
Three faults observed live tonight, all the same class: an exception escaping
into a consumer's await-foreach ends that subscription for the rest of the
process, so the feature it drives goes silently dead until the bot restarts.
1. "Map connection-status loop faulted. InvalidOperationException: GetMap
returned no data." — ConnectionSupervisor.GetMonumentsAsync forwarded the
connection-level call (documented "Throws on failure") straight through a
query seam whose own contract promised "or an empty list". It was the sole
query in RustPlusSocketSource without a degrade-to-null/empty catch. It now
catches, logs and returns [], and the failure message names the Rust+ error
code instead of a bare "returned no data".
2. "ConnectionStatusChanged consumer faulted. TimeoutException" from a Discord
REST edit — the Workspace consumer died, leaving the info channels stale.
3. The same shape existed, unguarded, in 21 consumer loops across 11 services.
Add EventBusConsumption.ConsumeAsync<TEvent>(handler, onHandlerFailure, ct):
it subscribes, guards each handler call, reports failures and keeps consuming;
only cancellation of the loop token ends the subscription. Abstractions stays
dependency-free (the callback is Action<Exception>, so each service keeps its
own logger). Every vulnerable consumer now routes through it; each service's
outer try/catch stays as a backstop. ClansHostedService and
CommandsHostedService already isolate per event inline and are left alone —
migrating them would lose the guild/server ids in their failure logs.
Also fixes the unrelated log line that started this: Rust+ sends the token
"apartmentcomplex", which no entry in MonumentTokenMap mapped, so apartment
complexes rendered with no icon ("No monument icon for token ...; mapped type:
null"). MonumentType.ApartmentsComplex and Apartments_Complex.svg were already
in RustMapsApi.Assets — only the token table had not caught up.
Tests: EventBusConsumptionTests (a failing handler costs its own event only;
cancellation still ends the subscription), WorkspaceConsumerResilienceTests and
MapHostedServiceTests (both written failing first, reproducing the live
exceptions), plus a monuments-seam and a token-map case.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot review: GetMonumentsAsync (the query seam used by map rendering) reused LogMonumentsFetchFailed, whose message says "rig detection disabled for this connection" — misleading for a non-rig caller. Give the seam its own generic LogMonumentsQueryFailed and keep the rig-specific message on the rig path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AdvancingClock jumped 1 minute per read while MapRefreshInterval is 30 minutes, so MapRefreshThrottle gated every refresh after the first: the counter only re-incremented once ~30 clock reads accumulated. Fast enough locally, but on Windows CI fewer than 30 events were consumed inside the 5s deadline, so calls stuck at 1 and the assert failed. Advance an hour per read (past the interval) so the throttle passes every refresh and each event reaches the counter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Design for moving team-state detection (connect/disconnect/death/respawn/ AFK) from the 2-5s poll onto RustPlusApi beta.6's OnTeamChanged event, keeping a 30s team poll as an AFK safety tick. Marker/rig polling stays. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…Changed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Rust+ connection layer to (1) drive team/presence/death/AFK transitions primarily from RustPlusApi beta.6’s team_changed push event (with a low-frequency poll as an AFK safety tick), and (2) harden multiple hosted-service consumer loops so a single handler failure no longer permanently kills the subscription/event-processing loop.
Changes:
- Upgrade RustPlusApi to
2.0.0-beta.6and surfaceteam_changedasIRustServerConnection.TeamChanged, with sharedTeamInfo→snapshot mapping. - Move team-state transition publishing out of the fast marker poll into push + a dedicated
PollTeamAsyncslow tick (ConnectionOptions.TeamPollInterval). - Introduce
EventBusConsumption.ConsumeAsyncand migrate multiple hosted services to resilient consumption that skips only the failing event/refresh.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Packages.props | Bumps RustPlusApi/RustPlusApi.Fcm to beta.6 to enable team_changed dispatch. |
| src/RustPlusBot.Abstractions/Connections/IRustServerQuery.cs | Tightens contract: monuments query degrades to empty list on failures (no throws). |
| src/RustPlusBot.Abstractions/Events/EventBusConsumption.cs | Adds resilient event-consumption helper to prevent handler exceptions from killing subscriptions. |
| src/RustPlusBot.Features.Alarms/Hosting/AlarmsHostedService.cs | Switches alarm consumers to ConsumeAsync so handler failures don’t end loops. |
| src/RustPlusBot.Features.Chat/Hosting/ChatHostedService.cs | Switches chat consumers to ConsumeAsync; isolates clan name recording inside handler. |
| src/RustPlusBot.Features.Connections/ConnectionOptions.cs | Adds TeamPollInterval option (default 30s) for AFK/self-heal safety tick. |
| src/RustPlusBot.Features.Connections/Hosting/ConnectionHostedService.cs | Uses ConsumeAsync for connection-driving subscriptions to survive transient failures. |
| src/RustPlusBot.Features.Connections/Listening/IRustServerConnection.cs | Adds TeamChanged event to connection abstraction. |
| src/RustPlusBot.Features.Connections/Listening/RustPlusSocketSource.cs | Wires RustPlusApi OnTeamChanged into TeamChanged; refactors team mapping; improves GetMap error message. |
| src/RustPlusBot.Features.Connections/Listening/TeamInfoMapping.cs | Centralizes RustPlusApi TeamInfo → TeamInfoSnapshot mapping for both poll and push paths. |
| src/RustPlusBot.Features.Connections/Supervisor/ConnectionSupervisor.cs | Implements push-driven team state publishing + slow team poll; makes monuments query resilient. |
| src/RustPlusBot.Features.Events/Hosting/EventsHostedService.cs | Uses ConsumeAsync so relay consumers continue after individual handler failures. |
| src/RustPlusBot.Features.Map/Assets/MonumentTokenMap.cs | Adds "apartmentcomplex" token mapping to ApartmentsComplex. |
| src/RustPlusBot.Features.Map/Hosting/InfoMapHostedService.cs | Uses ConsumeAsync for status events so failures don’t end the loop. |
| src/RustPlusBot.Features.Map/Hosting/MapHostedService.cs | Uses ConsumeAsync for marker/settings/status; guards periodic refresh to avoid loop death on a single refresh failure. |
| src/RustPlusBot.Features.Players/Hosting/PlayersHostedService.cs | Uses ConsumeAsync so player relay loop survives transient relay failures. |
| src/RustPlusBot.Features.StorageMonitors/Hosting/StorageMonitorsHostedService.cs | Uses ConsumeAsync so storage monitor consumers survive handler failures. |
| src/RustPlusBot.Features.Switches/Hosting/SwitchesHostedService.cs | Uses ConsumeAsync so switch-related consumers survive handler failures. |
| src/RustPlusBot.Features.Wipes/Hosting/WipesHostedService.cs | Uses ConsumeAsync so wipe consumers survive handler failures. |
| src/RustPlusBot.Features.Workspace/Hosting/WorkspaceHostedService.cs | Uses ConsumeAsync and extracts scoped reconcile helper to keep workspace reconciles resilient. |
| docs/superpowers/specs/2026-07-24-team-changed-push-design.md | Adds the design spec for push-driven team state and AFK safety tick behavior/trade-offs. |
| docs/superpowers/plans/2026-07-24-team-changed-push.md | Adds a detailed implementation plan for the push-driven team state work. |
| tests/RustPlusBot.Abstractions.Tests/Events/EventBusConsumptionTests.cs | Adds unit tests verifying per-event failure isolation and cancellation behavior. |
| tests/RustPlusBot.Features.Connections.Tests/ConnectionOptionsTests.cs | Verifies new default TeamPollInterval. |
| tests/RustPlusBot.Features.Connections.Tests/ConnectionSupervisorTests.cs | Adds tests for push transitions, slow tick transitions, AFK during silence, and verifies team info isn’t polled on marker cadence. |
| tests/RustPlusBot.Features.Connections.Tests/Fakes/FakeRustSocketSource.cs | Extends fake connection with TeamChanged, TeamInfoCallCount, monuments faulting, and setup hook. |
| tests/RustPlusBot.Features.Connections.Tests/RustPlusBot.Features.Connections.Tests.csproj | Adds RustPlusApi reference needed for mapping tests. |
| tests/RustPlusBot.Features.Connections.Tests/RustPlusSocketSourceTests.cs | Adds test for fake TeamChanged raise. |
| tests/RustPlusBot.Features.Connections.Tests/ServerQueryTests.cs | Adds regression test ensuring monuments query returns empty on endpoint failure. |
| tests/RustPlusBot.Features.Connections.Tests/TeamInfoMappingTests.cs | Adds unit tests for the shared TeamInfo→snapshot mapping. |
| tests/RustPlusBot.Features.Map.Tests/Hosting/MapHostedServiceTests.cs | Adds test ensuring map hosted service consumption survives a failing refresh and continues consuming events. |
| tests/RustPlusBot.Features.Map.Tests/MonumentTokenMapTests.cs | Adds test coverage for the new "apartmentcomplex" token mapping. |
| tests/RustPlusBot.Features.Workspace.Tests/Hosting/WorkspaceConsumerResilienceTests.cs | Adds test ensuring workspace consumer loop survives a faulting reconcile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
This branch carries two related connection-layer improvements:
1. Push-driven team state via
team_changed(RustPlusApi beta.6)Moves team-state detection (connect / disconnect / death / respawn / AFK) off the 2–5 s marker poll and onto beta.6's new
OnTeamChangedpush event. Marker/rig polling is unchanged (the Rust+ protocol has no broadcast for markers).team_changed, so the recurringWRN Unknown broadcast received: RustPlusContracts.AppBroadcast(which was every dropped team update) disappears.PollTeamAsync(newTeamPollInterval, default 30 s) still runsDiffto guarantee "BecameAfk" is detected (worst caseAfkThreshold + TeamPollInterval).PublishTeamStateAsync(Diff + publish);TeamStateTrackeris unchanged and its existing lock makes the now-two-threadedDiffcalls safe.Design & plan:
docs/superpowers/specs/2026-07-24-team-changed-push-design.md,docs/superpowers/plans/2026-07-24-team-changed-push.md.Accepted trade-off (documented in §4 of the design): because the slow poll captures its snapshot before a network round-trip, a concurrent push can commit a newer baseline first, so the poll can rarely emit a self-healing spurious presence transition (corrected on the next tick). AFK timing is unaffected.
2. Consumer-loop resilience (earlier commits on this branch)
Keeps event consumers alive across transient handler failures, and a query-specific log for the monuments seam.
Testing
Out of scope
The
GetMapMarkers returned no datawarning discards the server's actualresponse.Errorcode — a separate diagnostic fix, noted in the design's §7, not included here.🤖 Generated with Claude Code