Skip to content

Add multi-gateway gRPC concurrency and pointlist isolation tests#115

Merged
takashikasuya merged 3 commits into
mainfrom
claude/gateway-grpc-multi-connection-o605xc
Jul 8, 2026
Merged

Add multi-gateway gRPC concurrency and pointlist isolation tests#115
takashikasuya merged 3 commits into
mainfrom
claude/gateway-grpc-multi-connection-o605xc

Conversation

@takashikasuya

Copy link
Copy Markdown
Contributor

Summary

Closes the test-coverage gaps identified in #114: BuildingOS previously had no test proving that concurrent gateway gRPC connections (GatewayIngress telemetry ingest, GatewayEgress control plane) and the per-gateway point-list export (#224) stay correctly isolated between gateways.

  • GatewayIngress (GatewayIngressServiceTest.cs): two gateway streams processed concurrently (forced onto separate thread-pool threads via Task.Run, since the test fakes complete synchronously and would otherwise never yield) never cross-contaminate telemetry, and an identity-spoofing attempt on one connection doesn't affect a legitimate concurrently-connected gateway.
  • GatewayEgress (GatewayEgressReplicaRoutingTest.cs, GatewayConnectionRegistryTest.cs): extended the existing 2-replica routing test to 3 concurrent gateways; added a point-list-push routing-isolation test (the shared bus's point-list subscription was previously a no-op stub); added a concurrent same-gateway-id connect-race test for GatewayConnectionRegistry.
  • Pointlist sync isolation (ListGatewayPointListTest.cs, GatewayProvisioningControllerTest.cs, new OxiGraphSeedHostedServicePointListPushTest.cs): added a fake OxiGraph endpoint that actually scopes SPARQL responses by the gatewayId literal in the query (previous tests always returned one canned response regardless of query content, so a dropped filter would have gone undetected); fixed the controller test's mock to be gatewayId-aware for the same reason; added coverage for the seed-time per-gateway push fan-out (OxiGraphSeedHostedService.PublishPointListUpdatesAsync), which had zero test coverage before.

Production fix (found while writing the push-isolation test): PublishPointListUpdatesAsync's try/catch wrapped the entire per-gateway loop, so one gateway's publish failure silently aborted the push for every gateway after it in that seed run. Moved the try/catch inside the loop so each gateway is published independently, and added a BuildingOsMetrics.PointListPushSignals counter (the codebase's established "logged + metered" convention for best-effort per-item loops) so a partial failure is visible in Prometheus/Grafana, not just logs.

nexus-gateway (the external BOWS agent) was intentionally left out of scope — it's architected as one-gateway-id-per-process and multi-gateway federation is explicitly out of scope there.

This PR was reviewed with a multi-angle automated code review (correctness, reuse, simplification, efficiency, altitude, CLAUDE.md conventions) before being opened; findings that surfaced real issues (tests not actually running concurrently, a metrics gap, some test-fake duplication) were fixed.

Test plan

  • dotnet test --filter "FullyQualifiedName!~IntegrationTest" (unit tests) — could not run in this sandboxed environment: the .NET SDK download hosts (dotnetcli.azureedge.net, builds.dotnet.microsoft.com) are blocked by the environment's egress policy, so nothing here has been compiled. All new/changed code was verified by careful manual reading (types, signatures, Moq/xUnit usage, SPARQL query text matched against production constants) and cross-checked by 7 independent review passes, but please run the real test suite (or the manual-trigger oss-ci GitHub Actions workflow) before merging.
  • Manually verified against the local OSS stack

Checklist

  • I read CONTRIBUTING.md and followed the coding conventions in CLAUDE.md
  • I added or updated tests for the change
  • I updated relevant docs (README.md, docs/) if behavior or setup changed — no doc changes needed (test-only + one internal metric addition, no behavior/setup change)

Generated by Claude Code

claude added 3 commits July 8, 2026 14:41
Adds test coverage for issue #114: concurrent GatewayIngress streams
from distinct gateway_ids, GatewayEgress point-list-push routing
isolation across replicas, 3+ concurrent gateway connections, a
same-gateway-id concurrent-connect race, and gateway-scoped
ListGatewayPointList / GatewayProvisioningController isolation tests
that catch a dropped gatewayId filter (previous mocks ignored the
gatewayId argument entirely).

Also fixes PublishPointListUpdatesAsync in OxiGraphSeedHostedService
so one gateway's point-list-push failure no longer blocks the
remaining gateways in the same seed run (the try/catch was around the
whole loop instead of per-iteration).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEG6csNA9GDkQzT37CwAUp
- Force genuine cross-thread concurrency in the GatewayIngress
  multi-gateway tests (Task.Run) — the fakes previously completed
  synchronously so RunAsync never yielded, making Task.WhenAll a
  no-op over already-sequential work.
- Add BuildingOsMetrics.PointListPushSignals so #224/push failures
  are metered per gateway, matching the "logged + metered" convention
  used elsewhere (e.g. GatewayIngressService).
- Make OxiGraphSeedHostedService's two SPARQL query constants
  internal so the new push test can route its fake HTTP responses by
  exact query match instead of a fragile "HAVING" substring heuristic.
- Reuse the GatewayProvisioningControllerTest Build helper (extended
  with an optional shared db) in the new multi-gateway isolation
  tests instead of a second hand-rolled HttpContext/controller
  construction path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEG6csNA9GDkQzT37CwAUp
SeedQueryRoutingHandler compared the decoded request body directly
against the service's query constants, but the body is a
FormUrlEncodedContent "query=<value>" pair — the literal "query="
prefix made every comparison fail, so ValidateGatewayUniquenessAsync's
query always hit the fallback throw. Strip the form-field key before
decoding so the fake correctly routes to the canned uniqueness-check
response.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XEG6csNA9GDkQzT37CwAUp
@takashikasuya
takashikasuya merged commit 35d7cf7 into main Jul 8, 2026
6 checks passed
@takashikasuya
takashikasuya deleted the claude/gateway-grpc-multi-connection-o605xc branch July 8, 2026 20:30
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.

2 participants