Skip to content

Feat/topology redesign - #21

Open
ammodev wants to merge 86 commits into
version/26.1from
feat/topology-redesign
Open

ammodev wants to merge 86 commits into
version/26.1from
feat/topology-redesign

Conversation

@ammodev

@ammodev ammodev commented Jul 29, 2026

Copy link
Copy Markdown
Member

No description provided.

ammodev and others added 30 commits July 29, 2026 11:15
Documents the redesign of the RabbitMQ layer: exchange-based topology
(direct RPC, topic events, DLX, alternate exchange), unified SurfRabbitApi
replacing the client/server split, target addressing, retry queues with
backoff, and per-service circuit breakers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Reframe P3: microservices need to publish events, not call each other.
  Synchronous service-to-service RPC is documented as an anti-pattern.
- Add SHARED and BROADCAST subscription modes. SHARED is the default so
  handlers with side effects do not run once per instance.
- Extract the circuit breaker into a standalone surf-circuitbreaker module
  with no RabbitMQ dependency, reusable from surf-broker and surf-redis.
- Document horizontal scaling behaviour and the absence of ordering
  guarantees across instances.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four plans, each producing working and tested software on its own:

1. surf-circuitbreaker - standalone module, no Docker needed
2. topology foundation - exchanges, module merge, SurfRabbitApi
3. events and fire-and-forget - SHARED/BROADCAST modes, per-kind TTL
4. reliability and migration - DLQ, retry ladder, breakers, KSP, docs

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reviewing what happens when a microservice dies while sending its reply
surfaced a real defect: chunk assembly is keyed by correlationId alone,
so a redelivered message's chunks get merged with the leftovers of the
aborted attempt. Size checks miss it because both attempts serialise to
the same length. Latent today because failed messages are discarded -
the retry ladder in this plan would have made redelivery routine.

Adds Task 10 (series id in the chunk header) and Task 11 (seven failure
scenarios: death before, during and after the handler, midway through a
chunked reply, with the last instance gone, and broker loss while
publishing or awaiting).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Integration tests are tagged 'integration' and skipped when no Docker
daemon is reachable, so the unit suite stays usable without Docker.
A process now declares only queues it consumes itself, which removes the
cross-declaration that broke client channels when queue arguments changed.

Fixed a test bug in RabbitTopologyDeclarerTest: after a 406 protocol
violation the channel auto-closes, so use{}'s implicit close() on the
already-dead channel threw AlreadyClosedException outside the runCatching.
Task 3 added public API surface without updating the committed ABI
dump, which fails the ABI validation check wired into build.
Mechanical move only. Six modules become two; no behaviour changes.

surf-rabbitmq-core keeps the plain "core" gradle plugin rather than
"standalone": the standalone convention plugin bundles surf-api's
dependencies into the shaded jar, which would bloat every Paper/Velocity
consumer and risk version conflicts. StandaloneLifecycleHookImpl (moved in
from the old surf-rabbitmq-server) instead gets a compileOnly dependency
on surf-api-standalone: it satisfies the compiler without shipping the
dependency to non-standalone consumers, and genuine standalone processes
(e.g. surf-rabbitmq-test-server) already apply the standalone plugin
themselves and supply the real dependency at runtime.
Capability now follows from which register methods a process calls rather
than from which API class it constructed. One instance can address any
number of services over one connection.

- SurfRabbitApi/SurfRabbitApiBuilder replace ClientRabbitMQApi,
  ServerRabbitMQApi and the RabbitMQApi base class.
- rpcService and connection are lazy: constructing a SurfRabbitApi (e.g.
  for tests) no longer requires a broker-facing implementation to already
  be registered via ServiceLoader.
- ClientRabbitRpcService/ServerRabbitRpcService merge into one
  RabbitRpcService interface; the merged RabbitRpcServiceImpl (core)
  replaces ClientRpcServiceImpl/ServerRabbitRpcServiceImpl.
- RabbitRpcServiceDescriptor.createInstance and RabbitRpcCall now carry a
  RabbitTarget; the KSP codegen threads it through the generated
  constructor and createInstance call.
- GlobalRabbitMQConfig's config cache is keyed by (path, fileName) instead
  of a single JVM-wide instance, so configFileName() and multiple
  SurfRabbitApi instances in one JVM (a Paper server full of plugins) no
  longer silently share whichever file loaded first.
- SurfRabbitApiAlreadyFrozenException/SurfRabbitApiNotFrozenException now
  extend IllegalStateException directly (state precondition, not a
  wire-protocol error).

surf-rabbitmq-core does not compile yet - the connection fusion is
Task 7. All failures there are exactly the deleted RabbitMQApi surface.
Requests publish to the direct exchange with the target as routing key
and mandatory=true. Reply, service and instance queues each get their
own consumer/channel, so a failed declare no longer takes down the RPC
reply path.

The reply queue now has a stable name (surf.reply.<instanceId>) instead
of a renamed-on-recovery callback queue, so recovery signaling no longer
needs the rename-tracking machinery: RabbitClient.newCallbackQueueName(),
setRecoveredQueueNameSupplier and RabbitQueueNames are all gone.

A process consumes its service and instance queues only if it registered
a handler or an RPC service. registerService() never touches the
connection directly, so RabbitRpcService gained hasRegisteredServices()
for the connection to check at connect time; RabbitListenerHandlerManager
separately tracks explicit registerRequestHandler() calls so its own
always-present internal RPC dispatcher entry doesn't make hasHandlers()
report true for a process that registered nothing itself.
Enforced by SharedPackagePurityTest so a later extraction into
surf-broker stays a move rather than a redesign.

- KotlinSerializerCache/KotlinSerializerNameCache move into
  shared.serialization.
- The reflection invoker (RabbitListenerHandlerTemplate,
  RabbitListenerMethodHandleProvider) moves into shared.dispatch,
  renamed to HandlerTemplate/HandlerMethodHandleProvider since the
  names must not say "Rabbit". HandlerTemplate is now public since it
  implements RabbitListenerHandler from a different package.

The Velocity reflection proxies (JavaPluginLoaderProxy,
SerializedPluginDescriptionProxy) are deliberately NOT moved into
platform.* in this commit: that move crosses from surf-rabbitmq-velocity
into surf-rabbitmq-core and touches classloader-sensitive reflection
plumbing with no Velocity runtime available here to verify against.
Left for a follow-up with real Velocity test coverage.
Fire-and-forget and events no longer inherit the request timeout as their
expiration, which would have discarded them while a service was down.
ammodev and others added 30 commits July 31, 2026 16:54
AuditReport lives in the bus API so the event and query dispatchers can
report without seeing a broker type.
Every match runs, a failing handler is isolated and audited, includeSelf
defaults to dropping own events, and an unresolvable type warns and audits
exactly once. Two-stage type resolution fixes the base-type subscription
that the pre-2.0 Rabbit path silently dropped.

EventEnvelope moves into surf-eventbus-bus-api (package unchanged) because
EventTransport's signature carries it and bus-core depends on bus-api, not
the reverse.

EventDispatcher.dispatch() also short-circuits on an empty registry: with no
subscriptions, EventTypeResolver has no known classes and no listener
classloaders to search, so it would otherwise report a false
UNKNOWN_EVENT_TYPE audit for every event published while nobody is
listening.
A query addresses nobody, so the annotation has no service attribute; the
channel follows the contract's FQCN. One implementation per contract per
process.
Transport enablement is code, not configuration: a verb whose transport is
missing fails loudly and names the builder call. freeze() rejects a
subscription without its transport and names the handler.

SurfEventBus, SurfEventBusBuilder and SurfEventBusImpl all live in
surf-eventbus-bus-core (package dev.slne.surf.eventbus unchanged): the
builder constructs SurfEventBusImpl directly, and bus-api cannot depend on
bus-core. query() and rpc() throw NotImplementedError naming the missing
Plan 3 KSP proxy generator rather than returning null silently; the query
dispatch path is a documented stub for the same reason.
Wildcard-free subscriptions let the broker filter; wildcard subscriptions
match locally because Redis glob crosses dots. BinaryFrame keeps the
envelope readable in front of a codec payload.

RedisTransportLocator now looks the transports up via ServiceLoader
(requiredService), matching RedisComponentProvider's own pattern, instead
of throwing a forward-reference error. RedisTransportProviderImpl.query()
still throws until Plan 3 Task 7 adds the Redis query transport.

Docker is reachable on this machine, so RedisEventTransportTest ran for
real (7/7 passed) instead of being skipped.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Their replacement is SurfEventBus over RedisEventTransport. BusEventCodecs
(bus-core) already discovers a binary codec from an event's companion
object with no stable eventId, no collision bookkeeping, and no packet-
size estimation, so EventCodecRegistry/EventCodecRegistration/
CustomEventPacketCodec are deleted rather than rekeyed onto the event
class — nothing would have called the rekeyed version. The JMH benchmark
now measures EventEnvelope + BinaryFrame + BusEventCodec directly instead
of the old packet codec.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
surf.events, the shared and per-instance event queues, the event
dispatcher and @RabbitSubscribe are gone; events are Redis
(RedisEventTransport, Plan 3 Task 1). @RabbitSubscribe had no external
consumers, so nothing outside this repository notices.

The test-paper smoke-test instance's broadcast-event demo used the
deleted API and is removed rather than ported; a Redis-based replacement
arrives once the platform module wires up SurfEventBus. RetryQueueTest's
shared-tier-for-events case is dropped: events no longer touch the
RabbitMQ retry ladder.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The KSP processor rejects a non-Unit return type on a @fireandforget
method at compile time. RabbitRpcServiceImpl.call() branches on the
callable's fireAndForget flag and publishes via connection.send() instead
of waiting for a reply; the server-side fire-and-forget path already
existed in RabbitListenerHandlerManager for the untyped API and needed no
changes. createService()/rpc() take a RabbitTarget? now instead of a bare
service name, with proxies cached per (interface, target) so an
InstanceTarget call in a loop body is cheap.

Verified against a real broker: FireAndForgetContractTest (4/4) and the
KSP FireAndForgetValidationTest (2/2, via kotlin-compile-testing).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RabbitRequestPacket/@RabbitHandler, send()/registerRequestHandler() and
the six standard response packets are gone; typed @RpcService and
@fireandforget contracts replace them. The internal packet layer -
RabbitPacket/RabbitRequestPacket/RabbitResponsePacket, RpcCallRequestPacket,
chunking, properties injection - stays, since RpcCallRequestPacket extends
it; the plan's file list named the base classes for deletion, which would
have taken the internal layer down with it.

RabbitListenerHandlerManager no longer scans for @RabbitHandler-annotated
methods via a hidden-class invoker: RpcCallRequestPacket is the only
request type left, so it's wired directly. HandlerTemplate.java,
HandlerMethodHandleProvider.java and the handler-registration exceptions
go with the mechanism they served.

Fixed a bug this surfaced: a throwing @fireandforget handler was acked as
if it had succeeded, because RpcServiceExecutor always encoded handler
failures into an RpcCallResponsePacket that a fire-and-forget caller never
reads. It now propagates the exception instead for a fireAndForget
callable, which RabbitListenerHandlerManager's existing fire-and-forget
branch already retries/dead-letters correctly.

Nine tests rewritten from untyped packets to typed contracts, all
verified against a real broker. Two scenarios dropped because the typed
API has no equivalent: per-handler retry opt-out (no annotation carries
it), and dead-lettering an unroutable message to an unregistered service
(now a structured error response for two-way calls, a quiet ack for
fire-and-forget).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A second, simplified generator alongside @RpcService's in surf-eventbus-ksp
(dev.slne.surf.eventbus.ksp.processor.query, kept separate from the
RabbitMQ-named RPC processor package since queries have nothing to do
with RabbitMQ). Three compile-time rules: nullable return, never Unit,
never @fireandforget.

New runtime types in bus-api/bus-core (QueryParameter/QueryInvoker/
QueryCallable/QueryServiceDescriptor, QueryParametersSerializer/
QuerySerializerCache) rather than reusing RPC's - those live in the
rabbitmq modules, and Query is transport-agnostic bus infrastructure.
Simplified relative to RPC: no per-call-site custom serializer support,
just SerializersModule.serializer(KType) resolution.

SurfEventBusImpl.query() now resolves the generated <Name>Descriptor via
reflection and builds the client proxy, mirroring RabbitRpcServiceImpl's
own descriptor lookup.

Verified: QueryServiceValidationTest (KSP, 5/5) and QueryServiceClientTest
(bus-core, 3/3, against a FakeQueryTransport - real dispatch is Task 7).
The processor skips a private/file-local @queryservice interface rather
than erroring, since QueryServiceRegistryTest already had private query
fixtures for direct-registry tests that a generated (necessarily
non-private) descriptor can't reference.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
QueryDispatcher now really dispatches: it resolves the generated
descriptor for a frame's contract, decodes the payload, invokes the
callable, and encodes a non-null result back. RedisQueryTransport carries
queries and answers over Redis Pub/Sub - one channel per contract, one
reply channel per instance, first reply wins. The old Redis request/
response API (RedisRequest/RedisResponse/RequestResponseBus/
RequestContext/HandleRedisRequest/RequestTimeoutException) is gone along
with RedisApi.sendRequest()/registerRequestHandler().

QueryTransport.connect() gained an instanceId parameter (needed for the
reply channel name, not available yet at withRedis() time). Verified
against a real broker: RedisQueryTransportTest (6/6) and
QueryDispatcherTest (4/4, using an internal rather than private contract
since a private interface can't be referenced from its own generated
descriptor).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
RedisConfig knew config.yml plus overwriteFromEnv() (two layers); it now
resolves env > plugin yaml > global yaml > default via
resolveRedisConfig(global, plugin, environment), matching RabbitMQ's
layering. Unlike RabbitMQ's per-field IntOr.Default merging, a YAML layer
here wins or loses as a whole, since RedisConfig has no per-field "unset"
sentinel - only the environment layer overrides individual fields.

RedisApi.connect()/freezeAndConnect()/disconnect() are suspend now; the
blocking internals (Redisson.create(), the INFO server eval,
redisson.shutdown()) run under Dispatchers.IO. RedisTransportProviderImpl
no longer eagerly connects at construction (impossible once connect() is
suspend) - RedisEventTransport/RedisQueryTransport take an ensureConnected
callback instead, and the provider supplies a Mutex-guarded one that
connects the shared RedisApi exactly once, on whichever transport's first
real operation reaches it first. SurfEventBusImpl.disconnect()
deliberately does not tear down that shared RedisApi: RedisTransportLocator
caches one RedisTransportProvider per JVM, so every SurfEventBus built
with .withRedis() in one process shares the same Redis connection, and
disconnecting it on one bus's disconnect() would break every other bus
still using it.

This completes Plan 3 (Tasks 1-8): full build green, both ABI checks
clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ports

Five loss paths now report over a @fireandforget call to surf-eventbus-audit.
Three guards keep the audit from feeding itself, flood control keeps an
outage from becoming two, and the identity header groups ladder attempts.
The four merge plans left two contract models for one idea, two KSP
processors, three opt-in markers and packages named after modules that no
longer exist. The design says what becomes one API and which plan-4 tasks
ride along.

surf-eventbus-test goes: its three modules pointed at the stale RabbitModule
enum and at a surf-rabbitmq-paper plugin that no longer exists, and CI
excluded them anyway. The unit and Docker tests in surf-eventbus-core cover
the same ground and actually run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nine tasks in dependency order: one opt-in marker, one contract model, one
KSP processor, packages named after their subject, symmetry between the
transports, the workarounds removed rather than annotated, and plan 4's open
tasks 4, 5 and 6.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
InternalRabbitMQ, InternalRedisAPI and InternalEventBusApi disagreed on
level, on targets and on what this project is called. A consumer had to opt
in three times to reach one internal surface.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Descriptor, callable, parameter, type and invoker existed twice, once per
transport, differing in names and in nothing else. Query parameters gain
type annotations along the way, so @contextual now reaches them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two providers meant two resolver walks over the same file, two Names
objects, two model factories and two descriptor codegens. One walk, one
model, two backends where the output genuinely differs.

The shared emitters are the RPC ones, so a query parameter now carries its
type annotations into the descriptor instead of an empty list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
'common' was a drawer. 'rabbitmq.api' carried an .api infix that redis never
had, inside a module already called -api. 'core' sat inside the api module.
PackageLayoutTest keeps all three from coming back.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…transports

One eventbus.yml with a rabbitmq and a redis section, in two layers
(eventbus.yml, eventbus-plugin.yml), replacing GlobalRabbitMQConfig,
PluginRabbitMQConfig, CommonRabbitMQConfig, RabbitEnvironment and a separate
RedisConfig in another module. Every field is a sentinel, so env > plugin >
global > default now resolves per field for Redis too - a redis plugin file
with one key in it used to discard everything the global file set.

PluginRabbitMQConfig resolved each of its fourteen fields through
GlobalRabbitMQConfig.getConfig(), a mutable process-wide singleton, so what a
plugin config resolved to depended on which file had been loaded last. Both
layers are passed to the resolver explicitly instead.

The file type (EventBusConfig, sentinels) and the resolved type
(EventBusSettings, values) are now named apart. RabbitMQSettings is a data
class, so a test double is a constructor call rather than thirteen overrides.

Also: one EventBusInstance SPI instead of RabbitMQInstance plus an abstract
RedisInstance that built a Netty event loop, with the runtime objects moved
to RedisRuntime; one credentials seam for both transports; one exception root;
one RequiresDocker. StandaloneRabbitMqInstance and StandaloneLifecycleHookImpl
leave surf-eventbus-core, where a standalone class never belonged - which is
what made core's own unit tests read a never-initialised dataPath.

Fixes the Redis URI: redis://secret@host names a user called 'secret'; the
password belongs after a colon.

Removes LegacyEnvironmentGuard and the plugin config migration, both outdated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hand-written META-INF/services entry existed because @autoservice and
this project's processor cannot share a kspTest task. StandaloneLifecycleHook
no longer resolves a requiredService in a companion, so absence is NoOp and
the double has no reason to exist; withStandaloneHook injects one where a test
does want to observe the lifecycle.

AggregateCompletenessTest asserted that this module's classes are on this
module's classpath, which cannot fail. InternalNotInAbiTest asks the opposite
and useful question. PackageNamingTest and RelocationBaseTest no longer depend
on the working directory.

AuditReports replaces four hand-built AuditReport call sites that each filled
in the same six identity fields; a wrong messageUuid turned one message that
failed four times into four unrelated incidents.

The protocol-version-mismatch TODO is resolved as UNDESERIALIZABLE: the peer
speaks another protocol and will speak it again, so there is nothing to retry.

The publish workflow named jars that stopped existing at the rename.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paper and Velocity shipped a RabbitMQ instance and no Redis one, so publish
and query - the two verbs that ride Redis - did not work there at all. The
three EventBusInstance implementations landed with the SPI merge; this adds
the standalone module's test block and its first tests.

Verified on both shadow jars: exactly one ByteBuf class, and it is the
relocated one, so there is no unrelocated second Netty on the classpath.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every test is named after its number in the spec so the two stay pointed at
each other. The audit suite is missing on purpose - its microservice is still
blocked - but DatabaseContainerExtension exists so unblocking it is the only
remaining step.

Contrary to the plan's premise, Docker IS reachable here, so these ran. That
found two defects, both older than this branch and both reproduced at 256e950:

- RedisComponentProviderImpl.tryExtractPluginNameFromClass threw
  ServiceConfigurationError with no platform registered, so RedisApi.create(uri)
  failed in any process without one. The twelve existing RedisEventTransportTest
  and RedisQueryTransportTest cases had therefore never passed. The name only
  labels a connection in CLIENT LIST; it now falls back.

- EventSubscriptionRegistry never called trySetAccessible. A public method on a
  non-public class is listed by Class.methods but is not callable from another
  package, so the dispatcher threw IllegalAccessException on every delivery.
  Since a throwing handler is contained by design, the only trace was one
  EVENT_HANDLER_FAILED row per event: registered, reported healthy, never ran.
  Pinned by NonPublicListenerTest, which needs no container.

Full run: 275 tests, 0 failures, 0 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README still called this project surf-rabbitmq and documented
@RabbitSubscribe, SubscriptionMode and SURF_RABBITMQ_* variables - an API that
no longer exists and variable names that are no longer read.

It now leads with the three verbs and, under the promises, with the one that
costs data if assumed otherwise: events have no durability. Anything that must
not be lost is a @fireandforget rpc call, not an event.

The rollout note leads with the two steps that break silently: renamed
environment variables (a leftover old name is not read, so the process comes up
healthy pointing at localhost) and queue arguments that cannot be redeclared.
It also says plainly that the audit path has no writer yet, so a missing audit
row is not evidence that nothing went wrong.

Plan 4 Task 2 is unblocked: surf-database-r2dbc 2.3.2 relocates its @metadata
correctly. Verified by compiling suspendTransaction, insert, select and
deleteWhere against it inside surf-eventbus-core rather than by reading bytes -
the type descriptors already looked correct in 2.3.1. Recorded in the note,
including that deleteWhere lives in .r2dbc and not .core.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bring master's consumer/publisher hardening into the 2.0 module layout.

The eight files master touched live under surf-eventbus-* now, so most of the
commit followed the renames. Ported by hand where it could not:

- RabbitConsumer: took master's handlerTimeout, active-delivery cancellation
  and channel-level consumer recovery, keeping our withChannel helper.
- RabbitPublisher: dropped our inner catch in favour of master's outer
  resetChannelSafely handling; kept the ReturnListenerBridge hook.
- RabbitConnectionImpl: the handlerTimeout backstop and the warning for a
  reply that publishes but cannot be acked, previously in
  ServerRabbitMQConnectionImpl.
- RabbitListenerHandlerManager: master's select() over responseDeferred and
  handler failure, which closes the race where a detached invokeOnCompletion
  could settle a delivery twice. Adapted to retryOrDeadLetter, and a broker
  failure now requeues rather than spending a retry.

Kept version=2.0.0 over master's 1.6.3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…next ones

`docs/audit-2026-08-01-repository.md` is a full read of the tree at e4863ad. Its
critical findings share one shape: a documented guarantee that does not hold, and
fails without saying so. This commit acts on them.

Nothing ran the tests. `publish.yml` only triggered on `version/*`, so 21 commits
of breaking rewrite had never been built by anything but a laptop — and
`DockerAvailableCondition` turning a missing daemon into a skip meant a local run
could report green while verifying nothing. `ci.yml` now builds and tests every
push and PR, and passes `-PrequireIntegration`, which turns that skip into a
failure. `docker-compose.yml` and `docs/development.md` cover the local side.

The silent failures:

- A `suspend` `@SurfSubscribe` handler passed validation and could never run —
  the dispatcher invokes with one argument, a suspend method's JVM signature
  takes two, and the `IllegalArgumentException` became one `EVENT_HANDLER_FAILED`
  row per event. Now rejected at registration, which is the honest answer until
  the dispatcher can call one.
- `RedisEventTransport.disconnect()` did not unsubscribe. Disposing the
  `Disposable` from `.subscribe()` is a no-op; only `removeListener(id)` detaches.
  Subscribes now await the listener id — which also closes the race where an
  event published right after `connect()` returned could beat the in-flight
  SUBSCRIBE, and Pub/Sub does not redeliver.
- `EVENT_HANDLER_FAILED`, `QUERY_HANDLER_FAILED` and `UNKNOWN_EVENT_TYPE` never
  reached the audit service: the dispatchers were hardcoded to `LoggingAuditSink`
  with no injection point. The sink is now a constructor parameter.
- The audit correlation id was regenerated on the first hop, so every message's
  first failure grouped under a different UUID than the retry carried.
- A registered `@QueryService` with no query transport was stored, never
  subscribed, and turned every query into a silent abstention.
- Handler failures reported `InvocationTargetException` instead of what the
  handler actually threw.

The Lua that reads the server OS out of `INFO server` never worked. `[^\\r\\n]`
reached Lua verbatim, Lua collapsed it to `[^\rn]` — a literal backslash, `r`, `n`,
since Lua patterns escape with `%` — so `os:Linux 5.15…` matched `"Li"`. The match
succeeded with the wrong value, so `contains("Windows")` was never true and the
`os == null` fallback never ran either. `RedisOsDetectionTest` runs it against a
real Redis, since a Kotlin reimplementation of a Lua pattern would only test the
reimplementation.

Reactor and Redisson are out of the published ABI. `SyncStructure`, `SimpleRedisCache` and
`SimpleSetRedisCache` extended `reactor.core.Disposable` and `Initializable`
returned `Mono<Void>` — in a jar whose shadow config excludes `reactor.core`, so
the supertype named in the signature was one no consumer could resolve. They now
use `RedisDisposable` and `suspend fun init()`; Reactor stays the mechanism
inside the core module. `RedisApi`'s Redisson clients and its `RedisURI`/`Path`
`create` overloads are `@InternalEventBusApi` now, for the same reason — the
shadow jar relocates `org.redisson`, so those signatures named types no consumer
could resolve; `createForAddress` is what remains public. The two unused
`Mono.asDeferred` extensions are gone — one violated Reactive Streams §2.13, the
other duplicated `awaitSingle`. The ABI dump is regenerated to match.

`warnedTypes` is bounded now; its key came off the wire, so any peer could grow
this process's heap by publishing random type names.

`WireConstantsTest` pins the two identifiers that changed without appearing in
the API — the AMQP version header and the Redis sync key prefix. The sync prefix
is the dangerous one: a 1.6.x process left running against the same Redis keeps a
disjoint copy of every sync structure and nothing reports it. `rollout-2.0.md`
now says so.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three conflicts, all where both branches had been working on the same RabbitMQ
consumer paths.

`RabbitConnectionProvider`: both sides added a different import. Kept both.

`RabbitConsumer.getChannel`: topology-redesign added the channel shutdown
listener and consumer recovery, and `createChannel` now takes the connection
generation; surf-event-bus had moved the blocking open onto `Dispatchers.IO`.
Kept the recovery machinery and wrapped its `createChannel(connectionGeneration)`
in `withContext(Dispatchers.IO)`, so the blocking AMQP round trip still leaves
the caller's dispatcher — the same reason `awaitOpen` hops to IO.

`RabbitListenerHandlerManager`: both branches fixed the same double-settle bug
in different ways. surf-event-bus guarded `settleWithRetry` with a `settled`
CAS; topology-redesign removed the race by construction, completing a
`CompletableDeferred` from `invokeOnCompletion` and racing it against the
response in a `select`, and added the `SurfRabbitConnectionException` requeue
and `CancellationException` branches. Took topology-redesign's structure and
routed its settlements through the CAS-guarded `settleWithRetry`, which the
already-merged tail of the same function calls too.

Verified: ./gradlew check -PrequireIntegration — 280 passed, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…f the ABI

The rest of the audit's merge gate: C6, C7, the two Tier-2 regression tests that
were missing, and the linter from Step 1.

**C6 — Redis reads the plugin layer from the consumer's folder.**

`env > plugin yaml > global yaml > default` is what the README promises for both
transports. Redis had three of those: it passed the *platform* plugin's folder as
both the global and the plugin layer, so the `eventbus-plugin.yml` it opened sat
next to `eventbus.yml` in a folder no consumer owns, and nothing a plugin wrote
could reach Redis. RabbitMQ, one function away, had it right.

Which files feed the layers is now decided in one place — `resolveEventBusSettings`,
next to the `resolveEventBusConfig` that applies them — and both transports ask it
the same question. `SurfRabbitApiBuilder.resolveConfig` delegates to it rather than
answering separately.

`redisConfig` stays, and stays the Redis half's entry point, but it is keyed by the
consumer's data folder instead of being one `by lazy` fixed on first touch: the
plugin layer is per plugin by definition, so a single process-wide value could not
express it, and on a server with two plugins whichever touched Redis first decided
for both. `RedisSettings` is threaded from the bus builder through
`RedisTransportProvider` into `RedisApi.create`, `RedisCredentialsProvider` and
`RedissonConfigDetails`, so `clientName` comes from the same resolution as the host
rather than from a global the plugin layer never reached.

That also makes the production path testable, which it had never been. The Redis
suites built their buses through the `withRedis(event, query)` seam because a
process-wide value could not be pointed at a container — so the wiring real callers
use was the one thing those suites never exercised. They go through `withRedis()`
now, each bus with its own data folder holding the `eventbus.yml` an operator would
write. 17 suite tests moved onto it.

**C7 — no relocated third-party type is left in the published ABI.**

The dump named `io.netty.buffer.ByteBuf` 85 times and `org.redisson` 5, all in the
codec surface: `AbstractCodec : org.redisson.client.codec.BaseCodec`, `RedisCodec`,
`BusEventCodec`, the `ByteBuf` read/write helpers, and three Java types. The shadow
jar relocates both packages, so those signatures named types a consumer compiling
against the artifact cannot resolve, and a Netty upgrade was a breaking change for
anyone who managed it. All of it is `@InternalEventBusApi` now — declared internal,
not hidden by an ABI filter rule, so using it still requires opting in knowingly.
`BusEventCodec` is the one a consumer has real reason to implement and says so; the
way out is a buffer type of our own.

netty, redisson, reactor and RedisURI: 0 occurrences each.

**The two missing Tier-2 tests.** Both were confirmed to fail against the defect
before being kept:

- `EventBusConfigFileLayeringTest` — which file each layer is read from (C6). Three
  of its five cases fail if the plugin layer is read from the platform folder.
- `TimeoutSettlementTest` — a timed-out handler republishes exactly once (H4). The
  timeout branch settles the delivery *and* cancels the job, completing the handler
  and waking the second party with an opinion about the same message; settling twice
  parks two copies on the retry tier and the ladder doubles at every rung. This
  branch and topology-redesign fixed that two different ways and the merge combined
  them, with nothing pinning the result until now.

**Step 1 — ktlint, with a per-module baseline, in `check`.**

Detekt is deliberately absent, and `docs/development.md` says why: 1.23.8 embeds
Kotlin 1.9's compiler, whose `JvmTarget` stops at 22, and it reads that target from
the JVM it runs on — the Gradle daemon, because it invokes its CLI in-process rather
than forking. On a JDK 25 toolchain every task dies with a bare
`IllegalArgumentException: 25` before opening a file, and there is no fork to point
at another JDK. No 2.x is released.

Also: `org.gradle.jvmargs`. The daemon defaults (512m heap, 384m metaspace) run out
part way through a full `check`, which surfaces as an OOM or an `EOFException` from
a KSP worker rather than as anything naming memory.

Verified: ./gradlew check -PrequireIntegration — 286 passed, 0 failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Finishes the topology redesign: the transporter/connection/credentials
abstractions existed but only RabbitMQ was wired into them, and several of the
moved files never had their package declarations updated.

EventBusTransporter now owns freeze/connect/disconnect for both transports.
`connection` is abstract, a failed connect tears down through the normal
disconnect path rather than each transport hand-rolling cleanup, and disconnect
is idempotent and terminal. Redis gets a real EventBusConnection: RedisConnection
owns the Redisson clients, the initializable/disposable registries and the OS
probe, leaving SurfRedisApi as registration and serialization. Its dead
syncStructureScope is gone and redisListenerScope folds into the inherited scope.

Two bugs in the in-flight work:

- CircuitBreaker.open() took the Mutex while its only caller onFailure() already
  held it. A Mutex is not reentrant the way the synchronized block it replaced
  was, so the breaker self-deadlocked and CircuitBreakerConcurrencyTest hung.
  onSuccess/onFailure now run NonCancellable as well: a Mutex refuses to lock in
  a cancelled coroutine, which would strand probeInFlight and leave the breaker
  rejecting for good.
- EventBusInstance lost getResourceAsStream, tryExtractPluginName and orNull,
  all of which surf-eventbus-core still calls. Restored without the delegating
  companion, because a top-level requiredService() throws at class-init and
  orNull() has to answer for a process that registered no platform - the same
  reasoning StandaloneLifecycleHook already carries.

RedisCredentials was an empty data class and RedisCredentialsProvider did not
satisfy its own CredentialsProvider<C, CF> seam. redisURI() now takes the
credentials rather than the settings, so a provider sourcing the password from a
vault actually gets it into the URI instead of having it computed and discarded.

Codestyle, repo-wide: package declarations follow the directories the files were
moved into, and twelve main-source files holding more than one top-level type are
split one-per-file. Test fixtures are left alone - several sit next to the single
test that uses them and splitting those costs readability.

SurfRedisApiBuilder was an empty stub; it now mirrors SurfRabbitApiBuilder and is
the only route that can express the per-plugin config layer, which none of the
seven create(...) overloads can.

ABI dump updated. SurfRedisApi.connection stays protected and SurfRabbitApi's is
annotated, so neither re-exports a connection type through the published surface.

Verified with `check -PskipIntegration`: 218 tests, ktlint, lincheck, ABI check.
The testcontainers suites did not run - no Docker daemon on this machine - and
they are exactly what covers the Redis connect path this moves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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