Repository navigation
Conversation
…sion rules that make loopback safe The `parallel-consumer-proxy` module arrives with exactly one thing in it: the transport. It binds, it decides who may talk to it, and it hosts whatever service it is given - which for now is nobody, because the engine is a later change. The three classes are inseparable and were not separated. `ProxyServer.start()` constructs `AuthorityAllowlistInterceptor` and `SingleConnectionGuard` itself and wraps the hosted service in both, in that order, so a rejected authority is turned away before it can consume the admission slot. A "server" without them would be a different server with a different security posture, and the posture is the reviewable claim here: - Loopback-only by default, on an ephemeral port. The bind is `forAddress` with an explicit address and never `forPort`, which binds the wildcard. A non-loopback address refuses to start unless the opt-in is set, the refusal names the setting, and when it is set the warning states the whole surface - no authentication, offset advancement, plus receipt of Kafka credentials and a class-instantiating property map. - One live stream, rejected rather than replaced, so the engine never observes its peer silently swapped mid-session. The slot carries a generation counter because release is driven by asynchronous termination callbacks, and a late one from a superseded stream must not open the door to a second concurrent stream. - An unlisted declared `:authority` is refused with PERMISSION_DENIED, distinct from the slot's RESOURCE_EXHAUSTED so a client can tell "not allowed" from "taken" by status code alone. A connection declaring NO authority is accepted, because the browser threat this exists for always declares one. Both rejections are proven to happen BEFORE the service method by counters on the stand-in service - service invocations and application messages, both unchanged across a rejection - because a test asserting only the client-visible status passes just as well after the method has already run. THE ENGINE-FACING BOUNDARY IS `BindableService` AND NOTHING MORE. That is what makes this rung possible: the transport compiles against no generated protocol type and no engine type, so it can be reviewed without either. The connect-time configuration handler will be handed to the same builder method the tests hand a counter to. Taken from #293 by path-scoped checkout rather than rewritten. Two differences, both forced by master having moved under that branch: - `error_prone_annotations` is managed to 2.47.0, module-locally. gRPC drags it in twice at different versions, resolution settles on the lower one, and master's `requireUpperBoundDeps` fails the build. The same pin, for the same reason, as the protocol module's; repeated rather than hoisted, because hoisting would impose gRPC's transitive graph on every module in the repository. - `docs/simplifications-declined.md` is trimmed to the candidates whose subject is in this module. The rest travel with the code they are about. The file has to come now rather than later because `AuthorityAllowlistInterceptor` cites it by path at the one site where the diff looks strictly better and only the reasoning says otherwise - Guava's `HostAndPort` throws on a bare unbracketed IPv6 authority that method deliberately admits, which is a security-posture change wearing a library swap. One inherited comment was reworded rather than left dangling: `ProxyServerTest`'s note on why the log capture needs a synchronized list cited a test in a module that is not here, so it now states the race directly.
…ill holds a group membership The sidecar is the application's child. If it outlives its parent it keeps a consumer-group membership on behalf of an application that no longer exists, and the partitions stay assigned to a process nobody is talking to - so parent-death detection is correctness, not housekeeping, and it arrives with the entry point rather than after it. TWO SIGNALS, AND THE SECOND ONE IS WHY THE FIRST IS NOT ENOUGH. The parent holds the write end of an inherited pipe and never writes down it, so when it dies for ANY reason including SIGKILL the kernel closes the last write end and the read returns -1. Pure Java, identical on Linux and macOS, unchanged between the jar and a native image. `PR_SET_PDEATHSIG` was rejected on reach and on the merits: it needs `java.lang.foreign`, unavailable at this module's Java 17 release level, and it fires on parent THREAD death, is cleared across fork and setuid, and races a parent that died before the call. The pipe has one hole a client can fall into by accident: a wrapper process between the application and the sidecar inherits the write end and holds it open, so EOF never arrives. Launching through a shell is the everyday way to create one. The contract is "launch the proxy directly", and because "must" is not a mechanism, a `ProcessHandle` parent-pid poll catches it anyway. THE PARENT HANDLE IS CAPTURED ONCE, at construction. After the real parent dies this process is re-parented to init or a subreaper, so re-reading the parent on each poll would find a living process and report health forever - it would be answering "does this process have a parent" when the question is "is the parent it started with still alive". A JVM with no visible parent gets a poll that always says alive: unable to tell is not the same as dead. Reporting death is all it does. What happens next belongs to the caller, and separating them is what lets either be tested without killing a process. Four tests, no wall-clock sleeps - every wait is a bounded latch await, so a slow machine makes them slower rather than red. Two prove the signals fire; one is the NEGATIVE CONTROL that a live parent on an open pipe never reports death, because a watchdog that fired spuriously would pass the other two and kill every healthy sidecar in production. The fourth uses a stub stream rather than a `PipedInputStream`, because closing one of those from another thread does not wake a reader already blocked in `read()` - the pipe could only ever time out and read as a defect in this class. Taken unchanged from `feats/sidecar-entry-point` (unit U10 of the language-proxy plan), which is where this class and the sidecar's entry point were written. That branch has no PR; this is the extraction it will reconcile against, and its shutdown drain - the wait for records still held in a foreign process - is deliberately left behind, because it needs the engine.
…serves, and shuts down cleanly The proxy has never had a production entry point. Nothing in this module has ever been a process, which is what made "the sidecar" an assertion rather than an artifact. `Main` makes it one, and the claim it lets a reviewer check is exactly the packaging and runtime boundary: it builds, it starts, it serves its lifecycle, and it stops - releasing the socket - when its parent dies. THE SPAWNING CONTRACT. Bind an ephemeral loopback port, print `port: <n>` as the first line of stdout, then serve until the parent dies. Everything a client needs to find this process is on that one line, so the client half of the contract is a line read rather than a discovery protocol. IT PARSES NO CONFIGURATION, AND THAT IS A RULE RATHER THAN AN OMISSION (R39, U7). Bootstrap servers, credentials, ordering, concurrency and subscription all arrive connect-time in `Configure` over the protocol. An argument is therefore a caller misunderstanding, and this refuses to start rather than ignoring it: a sidecar that silently discarded `--bootstrap-servers` would be configured over the protocol anyway, and the operator would be left debugging why their flag did nothing. The usage text says where configuration actually goes, so the refusal is an answer rather than a rejection. WHAT IT HOSTS, AND WHY THAT IS NOT A STUB. The transport's contract with the thing it serves is `BindableService` and nothing more, which is what let the listener be reviewed with no engine in the room - but a server with no service is not a server, so something has to fill the slot. `NoEngineSessionService` refuses every session with UNIMPLEMENTED and a description naming the piece that is missing. That is the literal state of this build. The alternative - answering a `Configure` with a fabricated `Configured` - would let a client believe it had configured an engine that does not exist, which is a worse failure than a clear refusal. It is replaced, not extended: `Main#sessionServiceFactory` is the one call site the engine changes, and this class goes with it. A BIND FAILURE EXITS 4, NOT 2. `feats/sidecar-entry-point` returned the usage code for both, and that is the one behaviour here deliberately changed rather than carried: a port collision sends an operator who sees a usage code to the command line for a socket problem. Its own exit code costs nothing and is exercised by a test that occupies a port first. That branch's exit code 3 - a drain that timed out with records still held - is NOT here, because there is nothing to drain until there is an engine; it returns with the drain. THE CLOSE IS ASSERTED AGAINST THE SOCKET, NOT THE RETURN VALUE. A run that returns 0 proves the method returned; it proves nothing about whether the gRPC server was shut down, because a leaked one lives on its own Netty threads and keeps the port bound afterwards. So the test connects before, and requires the connect to be refused after. Proven able to fail: replacing the try-with-resources with a plain assignment - one term, nothing else touched - and the post-death probe connects happily, `Expected java.io.IOException to be thrown, but nothing was thrown`. Restored, green again. Two more arms, same method. Removing the `onError` from `NoEngineSessionService` so it accepts the session silently: the session test fails on its terminate latch after 30s rather than hanging the suite. Returning 0 instead of the bind-failure code: `expected: 4`. Control arms on both sides of all three, tree byte-identical afterwards. Five tests through the entry point rather than around it: arguments refused with a message naming where configuration goes; the port announced, proven to answer, and proven gone after parent death; a real gRPC session reaching the hosted service and being refused with a status naming what is missing; two sidecars binding different ports, because one host running two applications must not be a bind race; and the bind-failure exit code. Taken from `feats/sidecar-entry-point` (unit U10), whose `Main` this is minus the engine, the connect-time configuration handler and the drain. Two smaller differences from that branch, so neither side reads as an oversight: its `MainTest` sat in the `lifecycle` package while testing a class in the module root, and moves to sit beside its subject; and this module's `src/test/CLAUDE.md` write-time bridge is left behind for the same reason the protocol rung left its own - the document it imports and the file-reference gate's parser fix both arrive with #378. The module-maturity and testing-evidence records say what this does and does not evidence, in the two directions that are easy to get wrong: the process boundary is real and tested, and nothing here evidences a record being processed, an offset committed, or any protocol behaviour beyond a handshake being refused. The protocol module's support-posture row said the sidecar was not on master at all, which this change makes false, so it is corrected rather than left to rot.
…nd say why it is gone #293's pom declares `parallel-consumer-core` in compile scope, because the module it belongs to has an engine in it. This one does not, and carrying the dependency anyway would be an unused declaration that reads as an oversight to a reviewer and to `dependency:analyze` alike. Removing it is better than tolerating it, because the absence is the claim: the transport's contract with the service it hosts is `io.grpc.BindableService`, so nothing in this module references a Parallel Consumer type at all, and the pom is now the shortest possible proof of that. Verified by building the whole reactor clean and running the module's suite with it gone, rather than by reading imports. The pom says so at the site, so the next reader adds it back with the engine that needs it rather than as a fix. The core TEST-jar stays: it is a different artifact and it is used, for the shared ArchUnit convention rules. `commons-lang3` stays too, deliberately, even though this rung's tests pass without it. Its declaration carries an inherited reason - core's test-jar reaches for it, and test scope is not transitive - and "today's tests happen not to reach that path" is not evidence against a claim about what the test-jar needs.
Dependency ReviewThe following issues were found:
License Issuesparallel-consumer-proxy/pom.xml
OpenSSF Scorecard
Scanned Files
|
6 tasks done
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its |
|
…erent library matches The whole-tree CVE scan went red on this branch's first push: `com.google.code.gson:gson:2.11.0`, CVE-2025-53864, CVSS 6.9, reaching the tree for the first time because gRPC 1.75 brings gson in at runtime for JSON service-config parsing and this is the first module to depend on gRPC's transport. THE ADVISORY IS ABOUT `com.nimbusds:nimbus-jose-jwt`, NOT GSON. Its affected product is Nimbus JOSE + JWT before 10.0.2 / 9.37.4, and its own text says the flaw is "independent of the Gson 2.11.0 issue" - so Sonatype's matcher has taken a version out of a sentence that exists to distinguish the two. Nothing in this repository depends on nimbus-jose-jwt. PINNED FORWARD ANYWAY, RATHER THAN EXCLUDED, and that is the decision worth recording. An `excludeVulnerabilityIds` entry is repo-wide debt that has to carry a retirement condition, and this one would have no dischargeable condition: the record is CORRECT about a library we do not use, so it will never be withdrawn and the exclusion would sit there permanently. Moving off the matched version costs one managed line, removes the finding at its source, and is an upgrade wanted regardless. gson is transitive and not independently upgradable, so a managed version is the only lever available. Settled by the scan itself rather than by argument, one term changed and nothing else: CONTROL (gson 2.11.0) Detected 1 vulnerable components: com.google.code.gson:gson:jar:2.11.0:runtime * [CVE-2025-53864] ... (6.9) FIXED (gson 2.14.0) no vulnerable components; gson:jar:2.14.0:runtime in the tree `error_prone_annotations` moves 2.47.0 -> 2.48.0 in the same change, because gson 2.14.0 asks for 2.48.0 and `requireUpperBoundDeps` fails a graph that settles below its highest requester. That leaves this module a version above the protocol module's pin, which is not drift: the rule is "manage it up to what the highest requester asks for", and the two modules have different highest requesters. The pom says so, because two nearly-identical pins a version apart is exactly what a later reader would try to "fix". Full reactor build and the module's suite green afterwards; `bin/check-all.sh` exit 0.
Draft
5 tasks done
astubbs
marked this pull request as draft
August 31, 2026 06:25
5 tasks done
astubbs
added a commit
that referenced
this pull request
Aug 31, 2026
…han abandoning it SpotBugs on the PR: awaitAnnouncedPort "may fail to close stream". It was right, and the fix is the one the finding points at rather than a suppression - the method wrapped the process's stdout in a BufferedReader, read one line, and dropped it. The reader is now opened beside the process it belongs to and closed with it, in a try-with-resources around the whole arm, with the helper taking it as a parameter. That is also why it is not closed the moment the port line is parsed: closing it closes the process's stdout pipe, which must outlive the parse. The two remaining findings on these lines are UNENCRYPTED_SOCKET, and they are declined for the same reason #384 declines five of them: every one is a connection to this sidecar's own loopback listener, which is plaintext BY DESIGN - loopback-only plus one-connection admission is what this module offers instead of transport security. A probe that used TLS would be probing a listener that does not exist. Re-run: the native build and all four lifecycle arms green, the module suite green, bin/check-all.sh 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 tasks done
astubbs
added a commit
that referenced
this pull request
Aug 31, 2026
…s engine back Merges `feats/proxy-dispatch-clients` (#390, the top of the Wagon A stack) into this branch, so that #293 can be retargeted onto that rung and its displayed diff collapse to the engine residue. This is the plan's "retarget move" - `docs/plans/2026-08-31-001-process-god-branch-decomposition-plan.md` - and it is an ordinary merge: no history was rewritten and nothing was force-pushed. The pre-merge tip is preserved as `origin/backup/pre-stack-merge-293`. The merge also brings `master` forward, because the stack is current with it and this branch was not. That is most of the volume and most of the conflicts. ## Which side won, and why The rule was: the god branch's SEMANTIC content, expressed on the stack's refined STRUCTURE. - **Engine-only files stayed** - `ProxyProcessor`, `ConfigureHandler`, the dispatch waves, epochs, leases, reconnect and the produce path. They are the residue this campaign exists to expose. - **Stack-only files were taken** - the ten foreign clients, the runner registry, the sidecar shim, the self-retiring guards, `Main` and `ParentDeathWatchdog`. - **Records lost, everywhere.** The stack converted five value types from Java records to plain final classes because Jabel rewrites a record into a class with no source positions and Error Prone 2.42.0 crashes rather than reporting on it. This branch never met that only because its root pom had no Error Prone; master's does now, so the conversion is load-bearing here. - **Dependency pins are the stack's**: protobuf 3.25.8 (`requireUpperBoundDeps` refuses 3.25.5 against grpc-protobuf's transitive ask), gson 2.14.0, `error_prone_annotations` 2.48.0 in the proxy and the Java client aggregator and 2.47.0 in the protocol module - deliberately unequal, because their highest requesters differ. - **`clients.yml` is this branch's**, not the stack's. The stack's copy is a smaller earlier file whose own header says it has no conformance lane, no per-language static analysis and no dependency audit; only its `--component clippy` fix and its workflow-level `PC_FOREIGN_CLIENTS_STRICT` were folded in. - **Master's copy won for everything master owns** - the gate scripts, the copyright checker and its self-test, `repo-hygiene.yml`, `docs/ci.md`, `docs/copyright.md`. This branch's only delta in most of those was a copyright header in a pre-canonical form, written before #338 landed the canonical one on master. - **The two ledgers were merged as unions**, not resolved to a side. `docs/inflight/bug-857-family.md` keeps both sides' sightings, this branch's renumbered per the file's own stated convention; `docs/quarantined-tests.md` drops three entries whose owning PRs retired the annotation on master, because a registry entry without a `@Quarantined` test is a hard gate failure. ## The verdict-free work return, re-expressed on master's shape Unit U4 is still on this branch as `WorkManager.onAbandonedResult` and its call sites, and `WorkManager.java` auto-merged keeping them - so resolving `WorkContainer`, `ProcessingShard` and `ShardManager` to master alone would have compiled to nothing. Master moved underneath it: #335 collapsed `inFlight` and `maybeUserFunctionSucceeded` into one atomic `ExecutionState`, and #336 and #373 replaced the shard's raw `availableWorkContainerCnt` with a compare-and-set claim. `ProcessingShard.onAbandoned` and `ShardManager.onAbandoned` are therefore taken verbatim from `feats/proxy-verdict-free-return` (#295), which is cut on current master and is the authoritative re-expression - it routes the restore through `includeInSelection` rather than incrementing a counter that no longer exists. `WorkContainer` is NOT #295's, and the difference is deliberate: that branch marks abandonment with an `AtomicBoolean` cleared by the claim winner, while this branch keys it by delivery (`markAbandoned(long)`, `isAbandonedForCurrentDelivery`, `isReturnForSupersededDelivery`). The delivery-keyed form is strictly more: it tells a late return for delivery n from a live return for delivery n+1, which the boolean cannot, and acting on that confusion ends a running flight and decrements `numberRecordsOutForProcessing` twice. It also needs no clearing on redelivery, so it does not race the claim. Master's own `deliveryCount` - already an `AtomicLong` incremented only by a won claim - supplies the identity, so the field is the one thing added. ## The reconciliation duties #387 and #390 recorded Both are discharged, and the guards they left behind now pass because the gap closed rather than because anything was edited. **`ProxyHarness` and `ConformanceHarness` are one class again.** #387 cut the latter out of the former with the engine lane removed, and wrote into the class header, the module pom and a deferred inflight note that whoever landed the engine must merge them - leaving open which module it ends up in, on the grounds that "the engine lane decides". It decides the sidecar module: the lane needs `ConfigureHandler` and `ProxyProcessor`, and `TestModeMain` - the process a spawned foreign runner actually starts - is in that same test tree and would otherwise have to import the conformance module and close a cycle. So the NAME is the extracted rung's and the MODULE is this branch's, and `parallel-consumer-proxy-conformance` reaches it through the proxy test-jar its pom already declared. The stack's refinements to the class survive: `Delivery` stays a plain final class, and the `startEmbeddedClient` lane stays beside the restored `startEngine`. **`ConformanceDriver.spawnAgainst` is back**, with `LanguageRunner` a `ConformanceBinding` again - it stopped being one on #390 for exactly as long as there was no engine to spawn against - and the ten foreign bindings are registered in `ConformanceBindings.selectable()`. `ConformanceBindings.deferredUntilTheEngineArrives()` is kept and now returns an empty list, which is the honest reading: nothing is deferred. It is not deleted because `TheEngineArrivingMustBringTheForeignCellsTest` reads it, and because the next capability to arrive without a cell needs that list rather than a new one. ## Two duplications the merge surfaced, resolved rather than left - **The protocol documents existed twice.** #383 moved `protocol-specification.md` and `client-authoring-guide.md` into `parallel-consumer-proxy-protocol/docs/` and predicted the rename conflict here. The protocol module wins on the merits - all three artifacts are the contract rather than the server - but the CONTENT is this branch's, because the stack's copies carry "no client generates from this schema yet" and a `git show` fallback for a Go generator script that exists here. `SpecificationCoverageTest` moved with them; every citation was re-pointed. - **`HarnessScenario` existed twice**, once per harness. The stack's plain-class version won, in the sidecar module beside the class it serves. ## Five Java records had to become plain final classes, or nothing compiled `master` gained Error Prone after this branch forked, and Error Prone 2.42.0 does not report on a Jabel-desugared record - it crashes, failing the whole compilation with a message attributed to line 1 of an unrelated file. #387 hit this one rung down and converted its five value types; the same thing was waiting here for `ProxyProcessor.ManifestOutcome`, `ManifestReconciler.Reconciliation`, `InFlightRegistry.InFlight`, `LivenessSettings` and `TestModeMainTest.Run`. Neither term can move - the root pom pins Error Prone at 2.42.0 because 2.43.0 needs a JVM this build cannot use, and Jabel serves the release 8 target - so the records lost, which is what the rest of the repository already does. `grep -rn "record "` now finds none. ## Two defects this merge introduced and then fixed - **The default lane ran a test that needs a profile.** Keeping BOTH sidecar-spawning tests - this branch's engine-backed `OneRecordThroughTheSidecarTest` and the stack's engine-less `SidecarHandshakeTest` - left the Kotlin and Scala exclusion property naming only the first, so the second ran in an ordinary build with no `sidecar-classpath.txt` and failed. Both are now behind one regex, which is also the arrangement that makes the pairing legible. - **Every proxy module's data record existed twice.** This branch keeps module-maturity and testing-evidence records in per-module `docs/data/*.d/` fragments; the stack, which never imported that mechanism, wrote them into the monolithic files. The merge kept both and `bin/check-docs-data.sh` went from green to 37 structural problems. The fragments win, and deleting the monolithic copies also deleted eleven rows asserting "NO RECORD HAS CROSSED THE WIRE" - true on a stack whose sidecar hosts no engine, false here. ## How it was verified JDK 17, macOS. Full default reactor `./mvnw --fail-at-end test`: **BUILD SUCCESS**, every module green including the conformance suite - so the `java-grpc` cell runs against a live engine for the first time, and `GrpcSpikeConformanceTest` answers all five scenarios over a real gRPC stream. The four self-retiring guards #387 and #390 left behind all pass, and **none of them was edited**: `TheEngineArrivingMustBringTheGrpcBindingTest`, `TheEngineArrivingMustBringTheGrpcCellTest`, `TheEngineArrivingMustBringTheForeignCellsTest` (which asserts about every registered language, not one) and `SelectorMatchingNothingFailsTest`. `bin/check-all.sh` real exit code 1, with three gates failing and all three inherited, established against the pre-merge tip rather than assumed: `check-inflight-tags` reports the same 53 problems before and after; `check-file-refs` was already red for the `@`-prefixed bridge imports that #378 fixes; and `check-branch-self-reference` did not exist on this branch before the merge - its findings are on notes inherited from `master` byte-identical. `check-proto-lint` and `check-proto-breaking` report CANNOT RUN for want of `buf`. **Not run locally: `-Dpc.foreignClients`.** It builds C++ and Swift inside containers, and this box is under a low-disk warning that names a container build as what would tip it over. CI's per-language rows are where that lane runs. ## Two findings recorded rather than fixed - `ParallelEoSStreamProcessorTest.processInKeyOrder` failed its own preamble sanity check once here - and the control arm reddened `master` HARDER: unmodified `master`, uncontended, failed all three parameterisations where this branch under concurrent load failed one. That refutes the existing ledger's "the base branch was green" row, and is the first evidence the flake is master-state. Recorded in `docs/inflight/test-processinkeyorder-sanity-check-races-the-first-poll.md`. - One `java-grpc` conformance run left its last record unsettled, 1 of 2 full-reactor runs and green 3 of 3 in isolation. `core` and `java-direct` were green in the same run, so the suspect is the engine's settle path under a full ceiling rather than the scenario. The scenario was NOT weakened - `docs/inflight/proxy-a-java-grpc-ceiling-run-left-its-last-record-unsettled.md`. ## What is deliberately NOT done here `Main#sessionServiceFactory` still returns `NoEngineSessionService`, so the production entry point hosts no engine. That is unit U10's, which #384 named as still having no PR, and it owns the drain that must land with the engine - plus eight cross-language handshake tests assert the `UNIMPLEMENTED` refusal by status code specifically. Recorded rather than faked, in `docs/inflight/proxy-the-production-entry-point-still-hosts-no-engine.md`. The engine is exercised end to end through `TestModeMain` regardless.
astubbs
added a commit
that referenced
this pull request
Sep 1, 2026
… engine back Forward-merges `feats/proxy-requirements` (#293) into this branch after that branch took the Wagon A stack (merge 8a7dc3e) and was retargeted onto `feats/proxy-dispatch-clients`. This is the descendant-chain half of the plan's retarget move - `docs/plans/2026-08-31-001-process-god-branch-decomposition-plan.md` - and it is an ordinary merge: no history was rewritten, nothing was force-pushed, and this PR's base is unchanged. The pre-merge tip is preserved as `origin/backup/pre-forward-merge-328`. Most of the volume is content this branch was simply behind on: the ten foreign clients, the conformance framework, the protocol module's documents, current `master`, and the five records that became plain final classes because Error Prone crashes on a Jabel-desugared record. Only four files conflicted. ## The one that mattered: two `Main`s, and only one of them can be the sidecar `Main` was an add/add conflict, and the two sides are the same class written for opposite worlds. - This branch's is **unit U10**: it hosts `ConfigureHandler`, drains records held in a foreign process on shutdown, accepts `--socket` to listen on a Unix domain socket, and reserves exit 3 for a drain that timed out. `ReferenceDemo` - the whole point of this rung, and the 27x headline - spawns it through `SidecarProcess`, so a `Main` without an engine is a demo that cannot run. - The stack's is #384's engine-less shell: `NoEngineSessionService` behind a named `sessionServiceFactory()` seam, `EXIT_BIND_FAILED`, and a package-private `run` overload that lets a test name a port so the bind failure is reachable. The result is this branch's semantics on the stack's structure. `Main` keeps the seam, the bind exit code and the testable port; `sessionServiceFactory()` now returns the engine; the drain runs against whatever the session actually reached. Both usage texts, both sets of exit codes and both transports survive. ## Landing the engine there was not free, and the rung below said exactly what it would cost `docs/inflight/proxy-the-production-entry-point-still-hosts-no-engine.md` was written for this moment. It named the substitution as one call site, and then named the three things that had to happen with it - all three are done here, and the note is deleted rather than left describing a world that no longer exists: - **`NoEngineSessionService` is demoted to a test fixture, not deleted**, which is the outcome the note predicted. Eight cross-language `SidecarHandshakeTest`s (Kotlin, Scala, Go, Python, TypeScript, Rust, Ruby, C#) spawn a sidecar and assert the refusal arrives as `UNIMPLEMENTED` *specifically* - `PERMISSION_DENIED` and `RESOURCE_EXHAUSTED` are what the two interceptors raise before the service method runs, so the status code is the assertion rather than "it failed". An engine behind `Main` makes their subject vanish. - **`NoEngineMain` is the entry point they now spawn.** It is in the proxy module's test tree, so it ships in the test jar every client's sidecar classpath already includes for `TestModeMain` - nothing about how any of them spawns changed except the class name. It reimplements no lifecycle: it calls `Main`'s own seam with the no-engine supplier, so the binary those tests drive is the production lifecycle with one substitution, not a second copy that can drift. - **Nine call sites were re-pointed**: the eight foreign constants plus the Java client's own handshake test, which runs in-JVM and so calls `NoEngineMain.run` rather than spawning. The prose around each was corrected too - six of them described the thing they spawn as "the production `Main`", which is now the wrong binary rather than a wrong adjective. **`MainTest` is one class again.** Both sides grew one for the same class, in different packages, overlapping on arguments-refused, port-announced and two-sidecars. They are merged in `Main`'s own package keeping every assertion either side had, with the `UNIMPLEMENTED` test re-pointed at the no-engine service through the seam. `theProductionEntryPointHostsTheEngine` is new and is the substitution made assertable: without it the two suppliers could quietly swap back and every other test here would still pass, because the lifecycle is identical either way. ## The other three conflicts - **`.github/workflows/maven.yml`** - both sides added a matrix entry and neither displaced the other: this branch's `Demo (both entry points)` lane and master's `Chaos Pain Suite`. Union. - **`bin/check-copyright-headers.sh`** - master's refactor wins for the code (`check_derived`, `check_always_derived`, `fork_point_path`; this branch's copy still called `fp_path_of`, which no longer exists). The `Demo.java` recovery comment is this branch's, because master's version says the entry is ahead of its file and cites no ledger path *because none resolved on master* - both the file and `docs/inflight/branch-classic-comparison-demo.md` are here, so the path is cited. - **`ParentDeathWatchdog`** - a javadoc line only. This branch's wording wins because it is the one that is now true: the response to death is `DrainCoordinator`'s, and master's "once there is an engine it is the shutdown drain's" describes a future that has arrived. ## What was corrected because this merge falsified it `AGENTS.md`'s module table said the proxy module "hosts no engine, so a session is answered UNIMPLEMENTED", and the Python client's README called the engine-less sidecar "the production `Main`". Both are now wrong by one commit, so both were changed rather than left to rot. ## What was NOT corrected, and is a finding rather than an omission `docs/data/testing-evidence.d/parallel-consumer-proxy.yaml` says the module has no `src/test-integration` at all and that "there is no production entry point yet". Both were already false on this branch's own pre-merge tip - U10 landed `SidecarLifecycleIT` and `Main` without updating the record - so this is inherited rather than introduced, and rewriting an evidence record is the author's editorial call rather than a merge resolution. ## Two gates that the merge, not the code, turned red Both are 328's own files meeting analysers that arrived from `master` after this branch forked, so neither existed as a finding on either parent alone. - **SpotBugs now runs over this module** (`includeTests`, plus fb-contrib), and reported thirteen findings in the demo: three SLF4J format rules, class-envy and an over-concrete parameter on `ReferenceDemo`, a non-owned lock in its raw-gRPC arm, five exception-softening findings in `DemoBroker`, and two more in the test tree. **They arrive in two batches depending on whether `target/test-classes` exists when the gate runs** - the gate binds at `process-classes`, which is before `test-compile`, so a build that stops at `test-compile` sees the main-code findings only and a full `test` on a warm tree sees both. Do not read the first count as the whole set. Each was read at the site and dismissed in this module's own `spotbugs-exclude.xml`, class by class and pattern by pattern with the reason beside it - the shape the Java client aggregator's pom already prescribes and the api module already uses. None is a mute: a demo's output *is* pre-rendered text, a table renderer *does* read the rows it renders, and a `main()` whose only caller is a person reading a terminal is the one place `EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS` is the wrong rule. - **An unreadable exclude filter is a WARNING SpotBugs continues past.** The first version of that file quoted two command-line flags in an XML comment, and a double hyphen may not appear inside one - so the filter loaded as nothing, the gate kept failing with the same count, and the only evidence was one line reading `Unable to read filter` buried in the build log. Worth knowing before writing the next one. ## How it was verified JDK 17, macOS/arm64, toolchains through `mise` (`ruby 3.4.10`, `buf 1.72.0`). - **Full default reactor `./mvnw --fail-at-end test`.** Every module green, including the conformance suite over eleven of the thirteen bindings - so `java-grpc` runs against a live engine, and the Java, Kotlin and Scala handshake tests exercise the re-pointed no-engine entry point end to end. - **`bin/check-all.sh` on its real exit code: 1**, three gates failing and all three inherited. Established rather than assumed: the same gates on the incoming tip report the same `check-file-refs` and `check-branch-self-reference` findings, `check-inflight-tags` reports the same problems, and every *additional* finding names a file this merge did not touch - each verified with `git diff --quiet HEAD -- <file>`. `check-proto-lint` and `check-proto-breaking`, which report CANNOT RUN without `buf`, both pass under `mise`. - **`COPYRIGHT_CHECK_REQUIRE_FORK_POINT=1 bin/check-copyright-headers.sh`** - 0 violations against a real fork point. **Two things this box cannot answer, both established as inherited rather than assumed:** - **The `cpp` and `swift` conformance cells fail all ten of their scenarios with exit 126**, "found but cannot execute". Their runners are built in a container and extracted, so on macOS the artefact is a Linux binary - `file` reports `ELF 64-bit ... GNU/Linux` for both on a `Darwin arm64` host, which is the whole diagnosis and has nothing to do with anything in this merge. It is the condition #331 documents and fixes with `bin/run-conformance-in-container.sh`, one rung up. They are deselected here with `-Dpc.conformance.language`, named rather than silently skipped. - **Six of the eight foreign `SidecarHandshakeTest`s were not run** - Go, Python, TypeScript, Rust, Ruby and C# sit behind `-Dpc.foreignClients`, and C++ and Swift build inside containers on a box under a low-disk warning. Kotlin and Scala are JVM modules in the default reactor and did run, as did the Java client's in-JVM equivalent. What changed in all of them is one constant, and the process they now spawn is the same lifecycle with one supplier swapped.
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
1 participant
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.
Part of #242.
depends on #383
Description
The
parallel-consumer-proxymodule: a sidecar that is a real process, and nothing above that.It builds, it starts, it binds a loopback port and announces it, it admits or rejects a connection
under its own rules, it hosts a service a client can actually reach, and it shuts down cleanly -
releasing the socket - when its parent dies. There is no engine in it.
The claim this PR makes, and what to check it against: the packaging and runtime boundary exists
and is exercised end to end through the entry point, and the failure modes that would make that
claim hollow - a server that never shut down, a session that hangs, a bind failure that looks like a
clean exit - each go red when the term responsible is broken. The sabotage arms are below.
The honest state of the build is a first-class part of it. A client that opens a session is
answered
UNIMPLEMENTEDwith a description naming the piece that is missing. The alternative -answering a
Configurewith a fabricatedConfigured- would let a client believe it hadconfigured an engine that does not exist, which is a worse failure than a clear refusal, and it
would also make this rung unreviewable: a reader could not tell a real engine from a mock.
Where this came from
Extracted from #293, the language-proxy god PR, as extraction A5 of
docs/plans/2026-08-31-001-process-god-branch-decomposition-plan.md- the rung above#383, whose "deliberately left for the rungs above" list names exactly this
content ("the proxy module, its executable and its gRPC lifecycle (A5)"). Taken by path-scoped
checkout rather than rewritten.
The entry point is not on that branch. #293 has no production
mainatall - its only one is
TestModeMain, insrc/test, booting onMockConsumer. The productionsidecar entry point was written on
feats/sidecar-entry-point(unit U10), which is a siblingof #293 stacked above it, not an extraction of it, and which has no PR. See
"the prior-art branch" below.
What is in it, and why each piece is in the minimal cut
The transport - and the three classes are inseparable, which is a fact about the code rather than
a judgement.
ProxyServer.start()constructsAuthorityAllowlistInterceptorandSingleConnectionGuarditself and wraps the hosted service in both, in that order, so a rejectedauthority is turned away before it can consume the admission slot. A "server" without them is a
different server with a different security posture. What they assert:
forAddresswith an explicit addressand never
forPort, which binds the wildcard. A non-loopback address refuses to start unless theopt-in is set, the refusal names the setting, and when set the warning states the whole surface.
termination callback from a superseded stream cannot release the slot out from under the holder.
:authorityrefused withPERMISSION_DENIED, distinct from the slot'sRESOURCE_EXHAUSTEDso a client can tell "not allowed" from "taken" by status code alone.Both rejections are proven to happen before the service method by counters on the stand-in
service, because a test asserting only the client-visible status passes just as well after the
method has already run.
Main. Refuses arguments - configuration travels the protocol, and a sidecar that silentlydiscarded
--bootstrap-serverswould leave the operator debugging why their flag did nothing.Prints
port: <n>as stdout line one. Serves until the parent dies. Closes the listener.ParentDeathWatchdog. An orphaned sidecar holds a consumer-group membership on behalf of anapplication that no longer exists, so this is correctness rather than housekeeping - and without it
Mainhas nothing to wait on and no way to stop. EOF on the inherited pipe is the primary signal;a
ProcessHandlepid poll backs it up for the wrapper-process case, with the parent handle capturedonce because a re-parented process would otherwise report a living parent forever.
NoEngineSessionService. The transport's contract with what it hosts isBindableService, buta server with no service is not a server.
Main#sessionServiceFactoryis the one call site theengine rung replaces, and this class goes with it.
Deliberately NOT in this rung
Named so neither side reads as an oversight: connect-time configuration and the options mapping,
dispatch waves and the remote-execution semantics, the in-flight registry, epochs, the manifest and
reconnect reconciliation, leases and heartbeats, the produce path, capability negotiation, the
shutdown drain that waits on records held in a foreign process, conformance, the clients, and the
native image. The engine semantics remain #293's residue.
Also left behind: this module's
src/test/CLAUDE.mdwrite-time bridge, for the same reason#383 left its own - the document it imports and the file-reference gate's
parser fix both arrive with #378.
The prior-art branch, and what it turned out to be
feats/sidecar-entry-pointwas checked before anything was cut, on the possibility that it wasalready a shell extraction to build on rather than duplicate. It is not. It is unit U10 stacked
on top of #293's whole tree - forward work that ADDS the production entry
point, the parent-death watchdog and a shutdown drain to a module that already has an engine. Its
commit bodies say so outright, and one of them names the seam it deliberately leaves open.
So it is built on rather than duplicated:
ParentDeathWatchdogand its four tests are taken from itunchanged, and
Mainis itsMainminus the engine, the configuration handler and the drain.Writing a fresh EOF loop beside a reviewed one on a sibling branch would have been the
parallel-implementation antipattern this repository names by name. That branch has no PR yet, and
whoever opens it reconciles against this rung - its remaining content is the drain, the
integration test that spawns a real sidecar against a real broker, and the shared spawn helper.
feats/graalvm-sidecar-virtual-threadswas read too: it is inflight notes and benchmark arms aboutGraalVM and virtual threads, with no module shell in it. Nothing to reconcile.
Four places this differs from its sources on purpose
parallel-consumer-coreis dropped from compile scope. feat(proxy) astubbs#242: a sidecar giving non-JVM runtimes key-ordered concurrency, in eleven languages #293declares it because the module it belongs to has an engine. This one does not reference a
Parallel Consumer type anywhere, so carrying it would be an unused declaration - and its absence
is the shortest possible proof of the rung's whole claim. The pom says so at the site so nobody
adds it back as a fix. The core test-jar stays and is used, for the shared ArchUnit rules.
feats/sidecar-entry-pointreturns the usage codefor both; a port collision that reports a usage error sends an operator to the command line for a
socket problem. Its exit code 3 - a drain that timed out with records still held - is not here,
because there is nothing to drain until there is an engine.
error_prone_annotationsmanaged module-locally - the same pin, for the same reason, as theprotocol module's: gRPC drags it in twice at different versions and master's
requireUpperBoundDepsfails the resolved graph. Repeated rather than hoisted, because hoistingwould impose gRPC's transitive graph on every module in the repository. It sits a version above
the protocol module's, and the pom says why - see the CVE finding below.
docs/simplifications-declined.mdis trimmed to the candidates whose subject is in thismodule. It has to come now rather than later because
AuthorityAllowlistInterceptorcites itby path, at the one site where the diff looks strictly better and only the reasoning says
otherwise (Guava's
HostAndPortthrows on a bare unbracketed IPv6 authority that methoddeliberately admits - a security-posture change wearing a library swap). The rest of the pass
travels with the code it is about.
Two smaller ones:
MainTestmoves out of thelifecyclepackage to sit beside its subject, andProxyServerTest's comment on why its log capture needs a synchronized list cited a test in a modulethat is not here, so it now states the race directly.
Three sabotage arms, with a control on either side
Each arm changes one term, runs the one test that should break, and restores from a pristine copy.
Same-magnitude-different-position, not a bigger hammer.
Arm 1 is the one that matters most, because it is the assertion a reviewer would most reasonably
expect to be redundant. A run that returns 0 proves the method returned; it proves nothing about
whether the gRPC server was shut down, because a leaked one lives on its own Netty threads and keeps
the port bound afterwards. The port is the observable that separates "returned" from "shut down", so
the close is asserted against the socket rather than against the return value - and the arm shows
that without it the test passes on a sidecar that never released anything.
Arm 2's shape is the finding, not just its colour. Removing the refusal does not fail fast; it
hangs, and the test lands on its bounded latch after 30 seconds. That is the behaviour a client
author would hit against a silently-accepting sidecar, and it is why the refusal is explicit rather
than left to gRPC's generated default.
How it was verified
JDK 17, macOS, on this branch:
./mvnw cleanthen a full-reactorinstall- BUILD SUCCESS, seventeen modules, the new oneamong them.
the authority allowlist, the one-connection slot), the watchdog's two signals plus its negative
control, the entry point's five, and the ArchUnit conventions wrapper.
bin/check-all.shon its real exit code: 0. Two gates report CANNOT RUN -check-proto-breaking.shandcheck-proto-lint.sh, both wanting abufthis machine does nothave. Both belong to the base rung, neither is affected by this diff, and CI is where they run.
COPYRIGHT_CHECK_REQUIRE_FORK_POINT=1 bin/check-copyright-headers.sh- 0 violations against areal fork point, with
--reportshowing every new file classifiedenforcerather than skipped.Two dependency questions were settled by experiment rather than by reading imports, since both
answers looked obvious in opposite directions: removing
parallel-consumer-corefrom compile scopeand rebuilding the whole reactor (green - so it is genuinely unused, and it went); and removing the
commons-lang3test dependency, which also passes but was kept, because its declaration carriesan inherited reason about what core's test-jar needs and "today's tests do not reach that path" is
not evidence against it.
Does this compile against a
masterthat does not have #295? Yes, andthe clean reactor build above is that arm: this branch's base contains current
master, and#295 is open and unmerged. Nothing here touches
WorkContaineror theverdict-free abandonment path - the module has no
bz.stub.parallelconsumerimport outside theproxy packages at all, except the ArchUnit rules from core's test-jar. So there is no
depends on astubbs/parallel-consumer#295line, deliberately.What CI added that the local runs could not
The whole-tree CVE scan went red, and it was right to.
com.google.code.gson:gson:2.11.0,CVE-2025-53864, CVSS 6.9 - reaching the tree for the first time because gRPC brings gson in at
runtime for JSON service-config parsing and this is the first module to depend on gRPC's transport.
The advisory's affected product is
com.nimbusds:nimbus-jose-jwt, which nothing here depends on;its own text says the flaw is "independent of the Gson 2.11.0 issue", and Sonatype's matcher has
taken a version out of the sentence that exists to distinguish the two. It was pinned forward to
gson 2.14.0 rather than excluded anyway: an
excludeVulnerabilityIdsentry has to carry aretirement condition, and this one would have none that anybody could ever discharge - the record is
correct about a library we do not use, so it will never be withdrawn. Moving off the matched version
costs one managed line and is an upgrade wanted regardless. Verified by re-running the same scan
with the one term changed:
Detected 1 vulnerable componentsbefore, none after.error_prone_annotationsmoves to 2.48.0 in the same change, because gson 2.14.0 asks for it. Thatleaves this module a version above the protocol module's 2.47.0 pin, which is not drift - the rule
is "manage up to the highest requester", and the two modules have different highest requesters. The
pom says so, since two near-identical pins a version apart is exactly what a later reader would
"fix".
The duplication reports, read
dups: clonespasses both engines - PMD CPD and jscpd each report zero new clones and aduplication percentage at or below the base branch's.
dups: similarityis red on theTestConventionsArchTest.javafamily, at ~77-83% against thenine that already exist. That duplication is deliberate and required by a different gate:
EveryModuleWiresUpArchUnitTestfails any module with asrc/test/javaand no wired wrapper, andits javadoc records why the wrapper cannot be inherited instead (surefire's
dependenciesToScanpulls the whole test-jar into every module). Those nine already sit at similar percentages against
each other on
master; this is the tenth, and it only reads as new because the file is. Samefinding and same answer as #380 and #383.
The analysis surfaces, read
bin/check-pr-analysis-surfaces.sh 384reports 13 findings on lines this diff wrote and noneinherited, all from SpotBugs, which passes as a check - they are advisory. Every one is declined,
in four groups, with a reason rather than silently:
UNENCRYPTED_SOCKET/UNENCRYPTED_SERVER_SOCKET(5, all in tests). Every one is a testconnecting to this sidecar's own loopback listener, which is plaintext by design: loopback-only
plus one-connection admission is what this module offers instead of transport security, and the
reasoning is in
ProxyServer's javadoc and the module README. A test that used TLS would betesting a listener that does not exist.
USO_UNSAFE_METHOD_SYNCHRONIZATION(4).synchronizedmethods onProxyServerandParentDeathWatchdoglockthis, which a caller holding the instance could also lock. The threatmodel here is entirely out-of-process - anything in this JVM holding a
ProxyServercan alreadycall
close()on it - so a private lock object would buy nothing against any attacker this modulehas, at the cost of diverging from feat(proxy) astubbs#242: a sidecar giving non-JVM runtimes key-ordered concurrency, in eleven languages #293's copy of both classes.
PSC_PRESIZE_COLLECTIONS(1). ALinkedHashSetinAuthorityAllowlistInterceptorbuiltwithout a capacity hint, once per server start, over an allowlist of about five entries. This
module's
docs/simplifications-declined.mdrecords "any change toAuthorityAllowlistInterceptoror
SingleConnectionGuard" as declined - security posture, and their apparent simplicity is thepoint - and a presize is not the exception that earns.
LO_SUSPECT_LOG_CLASSandSLF4J_ILLEGAL_PASSED_CLASS(2, inProxyServerTest). The testasks
LoggerFactoryforProxyServer's logger on purpose: it is asserting on whatProxyServerlogs. Fixing the finding would remove the test's subject. Same for
USO_UNSAFE_OBJECT_SYNCHRONIZATIONon the capture list, which is synchronized deliberately becausethe server's own threads write it while the test reads it.
Known-red, and not a fault
Check PR Dependenciesis red until feat(proto) astubbs#242: the frozen v1 proxy wire contract, and the gates that keep it frozen #383 and its own parent merge.That is the dependency gate doing its job on a stack rung, not something to fix.
claude-reviewis red until somebody asks for a review; both rungs below are in the samestate.
roadmap-stage: N/A - the
language-proxy-sidecarentry names #293 as its carrier and sits atlimited-pocon the strength of that PR's content. Landing a sidecar that hosts no engine on master makes no more advanced artifact exist - it cannot process a record or reach a broker - so its stage is unchanged.Checklist
README.mdrewritten to describe what is actually in it and what isnot,
AGENTS.md's module table,docs/simplifications-declined.mdtrimmed to this module'sentries, and the module-maturity and testing-evidence records. The protocol module's
support-posture row said the sidecar was not on master at all, which this change makes false, so
it is corrected rather than left to rot.
docs/features/- N/A - nothing here is auser-facing feature. The module publishes to no registry and cannot be configured to reach a
broker; the release-documentation records it does earn are the maturity and evidence rows, which
are in this PR and say in both directions what is and is not evidenced.
watchdog's four are new to
master. Three were proven able to fail by sabotage, with control armson either side, above.
ce-simplifyandce-code-reviewlocally - N/A - neither was run. Most of the diff iscode taken from feat(proxy) astubbs#242: a sidecar giving non-JVM runtimes key-ordered concurrency, in eleven languages #293 and
feats/sidecar-entry-pointwhere it was alreadyreviewed, and the changes this rung made to it are named one by one above; the cheaper place to
spend a review is
@claude review thison the PR, which is also the only route that opens inlinethreads.