clean-architecture-backend-template/docs/reviews/2026-08-19-p1-remediation-status.md

P1 remediation status — the five module reviews

Reviews: docs/reviews/2026-08-14-{jpa,graphql,messaging,mongodb,notification}-module-code-review.md Baseline: the P0 pass was already complete when this pass began; this file records the P1 pass, which is complete — all 31 findings closed, two of them by establishing that the review's own accepted outcome was already met rather than by writing code. Verified at: repo-wide test, spotlessCheck, verifyCleanArchitectureDependencies, verifyEnvKeys, verifyPublicPathSnapshot and the root CleanArchitectureTest all green.

This file exists because the status was previously carried only in conversation and had to be reconstructed. A finding's row is the claim; the evidence column is where the claim is falsifiable.

The recurring defect

Nearly every P1 in these five reviews is one shape: a control that exists, passes its own tests, and is reached by nothing. Not a wrong algorithm — an unreachable one. The tests passed because they constructed the class directly; the capability was absent because no configuration could.

Examples closed in this pass: the Mongo typed-update path (MongoBulkExecutor and MongoAtomicOperationsTemplate were constructed by nothing), the entire Mongo change-stream capability (no production code opened a stream at all), GraphQlBatchLoaderRegistrar.register (no caller, so the wrong-key refusal never ran on an executing query), PublishOptions.timeout() (read by nobody on the real publish path), markExhausted (no caller, so a row that spent its budget stayed AMBIGUOUS forever), OutboxMessagePublishPort.publishForOutcome (no caller, so every ambiguous publish collapsed into an exception), the outbox relay itself (no bean ran a pass), and BackpressureController (a limiter the publish path never consulted, reporting globalInFlight: 0 under any load).

The lesson worth keeping: a passing unit test is not evidence a capability exists. The reachability question — what constructs this, and on which request path — has to be asked separately, and several of the tests added in this pass exist only to ask it.

Status

Finding	Verdict	Evidence
JPA-005	closed already	roll-up in NotificationRequestStatusPolicy; port is tenant-scoped; ArchUnit PERSISTENCE_DOES_NOT_DEPEND_ON_APPLICATION_SERVICES
JPA-006	closed already	PersistenceJpaRootAutoConfiguration is in AutoConfiguration.imports and imports the real runtime config
JPA-007	closed under item 6	the review offers two accepted outcomes: full integration, or the interim state under item 6's two conditions. Both hold and were verified in code — FullTransactionRetryCoordinator:89-93 resolves the policy per call from the calling profile, and DefaultJpaRetryPolicy:61-64 returns on !failure.retryable() before consulting the category allowlist
JPA-008	closed	nothing registered a VendorFailureTranslator, so every executor ran withoutCatalogs() and a 40001 never reached the retry classifier
JPA-026	closed	a rehydrated callback event had no matcher once attempt_id was null; hash fallback + write-back added
JPA-028	closed	the reaper query had no caller, uploads had no terminal state, cleanup decided from a lease it had read rather than claiming
JPA-029	closed already	tuple cutoff implemented; the signal is documented best-effort by decision
GQL-004	not a defect	every evidence bullet false at HEAD; no WebFlux dependency exists, transport disagreement fails startup
GQL-011	closed	unkeyed truncated SHA-256 over actor/tenant replaced with a keyed, rotating HMAC; no default key
GQL-015	closed	schema extensions were invisible to the comparator, so a field removed by extend type produced no change at all
GQL-016	closed	the batch executor returned the loader's map verbatim, so the wrong-key refusal and missing-key policy never ran
GQL-017	closed	the blocking bridge was opt-in and null by default, so a reactive runtime ran blocking chunks on the event loop
MNG-010	closed	the guardrail was a Set<String> asserted against itself; now an ArchUnit rule over the real production graph at the composition root
MNG-012	closed	a failed abort or close on a committed transaction was discarded by a closing brace
MNG-018	closed	TLS and auth were asserted against a settings object; four TLS cases now run against real servers
MNG-024	closed	the reactive binder carried read preference and write concern only, so reactive writes skipped auditing and callbacks
MNG-026	closed	both typed-update paths were unreachable, and the bulk executor could be built with no policy at all
MNG-028	closed	no production code opened a change stream; the consumer now owns load → resume → stream → project → checkpoint
MSG-006	closed	markExhausted had no caller and the scheduler's backoff was never written; a relay worker now runs passes
MSG-008	closed	validators were beans nothing injected, and the documented configuration bound nowhere; destination/broker/security sections now bind under app.messaging and the reference document is executed by a test
MSG-010	closed	BackpressureController deleted as an unreachable duplicate; its one unique capability moved into the gate that is called
MSG-012	closed	header values accepted CR/LF/NUL, identifiers were bounded in chars not bytes, traceparent was any string, denylists matched exact spellings only
MSG-014	closed	hasLiveBrokerCertification is derived from recorded evidence rather than declared, and the evidence is now a manifest a fault lane wrote against a real broker rather than a hand-authored list
MSG-015	semantic half closed	the outcome-aware publish path is wired; the anti-corruption bridge needs a modules.json edge and is an architecture decision
MSG-016	closed	no reserved name existed for tenant, so every consumed message was rebuilt with none; the canonical metadata is now columns, and the CDC event key moved off destination, which had put every message on a topic onto one partition
MSG-017	closed	the timeout is an absolute deadline; contradictory PublishResult combinations are unrepresentable; brokerHints removed
NTF-015	closed	webhook signing was one shared secret for every subscription; SES silently dropped attachments and now sends them as raw MIME
NTF-016	closed	retired keys were forced to one purpose so a provider-credential drain failed; required purposes now follow enabled capabilities
NTF-019	closed already	split inbound ports carry capabilities; four ArchUnit rules with negative fixtures close the gate

Found while closing, not in any review

SmtpMimeMessageFactory handed JavaMail the resolver's one-shot stream. JavaMail reads an attachment twice — once to choose the part's transfer encoding, once to write it — so the second read returned nothing and the message went out announcing a filename and carrying no bytes, with the attempt recorded as accepted. Every existing test asserted on the outcome of the send rather than on what was sent, which is why a bug that emptied every attachment on the one provider family this platform can actually assemble survived a full review pass.

SmtpAttachmentBodyTest now reads the attachment back off the serialised message the way a receiving client would. It was confirmed to fail against the original code and pass against the fix, because a regression test nobody has watched fail is a regression test of unknown shape.

Observations that are not open P1 items

Both were checked against the reviews rather than assumed, because "looks unfinished" and "is an open finding" are different claims.

The GraphQL cursor key gate. GraphQlPlatformStartupValidator refuses to start a production deployment without backend.graphql.cursor.key-ids, and nothing signs a cursor with it: GraphQlConnectionAssembler and HmacGraphQlCursorCodec have no consumer anywhere in this repository, because the template ships no paginating resolver. This is not GQL-010, which is about the codec's framing, rotation and scope and is implemented — versioned framing, the codec choosing the active key rather than the caller, v1 decode kept only for migration, tenant scope bound. It belongs to the modelled grading the leaf's own GraphQlPolicyRequestPathTest already documents in as many words. Declaring beans for it would create the unreachable-control defect this pass exists to close, and the present behaviour fails closed, which is the safe direction. Left as it is, on purpose.

Product work, not remediation

Notification provider transports. Only SMTP has a ProviderRuntimeAssembler. NotificationProviderAssembly refuses to start a profile whose family has no assembler, naming the transport as a seam rather than an implementation — which is the honest fail-closed behaviour, not a defect. Building SES, Twilio, FCM, APNs and WebPush transports is product work.