fix(protocol): complete protocol conformance, delivery recovery and regression coverage

.buildkite/start-unit-tests.sh

@@ -29,9 +29,7 @@ cd mapsmessaging_server
 
 ulimit -a
 
-git clone https://github.com/eclipse/paho.mqtt.testing.git
-
-mvn clean test -Dpython_command=python3 -Dcom.datastax.driver.FORCE_NIO=true -Ddebug_domain=false -Psnapshot
+mvn clean test -Dcom.datastax.driver.FORCE_NIO=true -Ddebug_domain=false -Psnapshot
 
 # Extract test results
 RESULTS_DIR="$HOME/mapsmessaging-test-results"

CODING_STANDARDS.md

@@ -0,0 +1,112 @@
+# MapsMessaging Java Coding Standards
+
+**Status:** Agreed
+**Scope:** Java production code, tests, and supporting infrastructure in MapsMessaging.
+**Baseline:** The Java version configured by the repository build. Do not introduce features requiring a newer language level.
+
+## Principles and scope
+
+New code should look and behave as though it belongs in the existing MapsMessaging codebase. Prioritise correctness, clarity, maintainability, and predictable runtime behaviour over brevity or individual preferences.
+
+Apply these standards to new code and code being substantively modified. Follow surrounding conventions where they do not contradict this document. Do not reformat the entire repository or unrelated sections of a file. Keep cosmetic-only changes in separate commits from behavioural changes.
+
+## Explicit types
+
+Do **not** use Java local-variable type inference (`var`) in production code or tests, including enhanced loops and local declarations. State the type explicitly and prefer interfaces when appropriate.
+
+```java
+DeliveryStateCoordinator coordinator = deliveryStateCoordinator;
+List<Integer> packetIds = new ArrayList<>();
+Map<Integer, Message> messages = new HashMap<>();
+```
+
+## Formatting and control flow
+
+- Use two-space indentation and spaces rather than tabs.
+- Use a **soft target of 180 columns**. Do not wrap merely because a line exceeds that target by one or a few characters. Break lines where it improves comprehension, not to meet a mechanical threshold.
+- Put opening braces on the same line as the statement or declaration.
+- Use braces for all `if`, `else`, `for`, `while`, `do`, and other control-flow bodies, even one-statement bodies.
+- Expand `try`, `catch`, `finally`, and conditional blocks across multiple lines. Do not compress multiple independent statements onto one line.
+- Use consistent spacing around operators, after commas, and between logical sections. Avoid drive-by formatting of unrelated code.
+
+```java
+if (message == null) {
+  return;
+}
+
+try {
+  processMessage(message);
+} catch (IOException exception) {
+  logger.error("Unable to process message", exception);
+  throw exception;
+}
+```
+
+## Imports and naming
+
+Use normal imports and simple type names. Avoid fully qualified Java class names inside method bodies unless needed to disambiguate types. Prefer explicit imports to wildcard imports in new code.
+
+Give classes, fields, methods, parameters, and local variables descriptive names. Avoid unclear one-letter names and unnecessary abbreviations. Conventional, tightly scoped identifiers are acceptable where unambiguous. Preserve established public API names and recognised protocol terminology.
+
+## Methods, features, and class organisation
+
+Create focused helper methods when they clarify a meaningful operation, express a domain concept, reduce duplication, or make resource ownership or control flow easier to understand. Do not introduce trivial wrappers solely to shorten methods. Keep related lifecycle operations grouped.
+
+Modern Java features such as records, switch expressions, pattern matching, and text blocks are permitted where they improve readability and remain compatible with the configured Java version. Prefer loops over streams when ordering, mutation, exceptions, or debugging are clearer with a loop. Do not use `var`.
+
+Use Lombok consistently with surrounding code; do not add it indiscriminately.
+
+## Defensive programming
+
+**Agreed policy:** Validate aggressively at external and untrusted boundaries, but rely on explicit, enforced contracts inside the application.
+
+- Validate network/protocol input, configuration, persisted state, and other externally controlled values.
+- Establish internal invariants through constructors, API contracts, and appropriate state transitions.
+- Do not scatter redundant null checks and arbitrary defaults through internal code to hide broken invariants.
+- Use explicit failures for violated internal contracts rather than silently continuing with invalid state. Choose a meaningful exception type and preserve context.
+- Use fallback behaviour only when it is part of a defined protocol, compatibility, or recovery contract.
+- Differentiate malformed external input (which should normally be handled and isolated) from programmer errors (which should remain visible).
+- Do not add defensive branches merely to satisfy static analysis if they weaken the stated contract.
+
+## Concurrency and lifecycle
+
+Document important ownership, synchronization, reference publication, callback, shutdown, recovery, and replacement assumptions where they are not obvious.
+
+Use `volatile` for visibility and safe publication when appropriate; it does not make compound operations atomic or mutable referents thread-safe. Use synchronization, atomic types, and concurrent collections only when they provide the actual required guarantees. Read a changeable reference once into a local variable if a null check and later operation must act on the same object.
+
+Prefer invoking externally supplied callbacks outside internal locks when safe. Capture the necessary state under lock, preserve lifecycle semantics, and test ordering, closure, recovery, and replacement paths.
+
+## Exceptions and resources
+
+Preserve original exceptions and their causes. Never silently swallow meaningful operational errors or mask the primary exception during cleanup. Use explicit error handling at a level where the failure can be handled meaningfully.
+
+Prefer try-with-resources for locally owned resources. When ownership transfers to a longer-lived component, document the hand-off and close the resource on failed initialization. Never close a resource after a successful ownership transfer merely to satisfy a static-analysis warning.
+
+## Tests
+
+Prefer constructor-valid instances, existing test builders, and reusable fixtures for stateful components. Mock external collaborators as needed, but avoid partial mocks that bypass essential initialization (including `CALLS_REAL_METHODS`) unless specifically justified and fully initialized.
+
+Tests should verify observable behaviour and preserve meaningful assertions. Regression tests should reproduce the defect and verify the fix; do not disable or weaken failing tests to make a build green.
+
+Use established test data locations such as `target` or `target/data`. For concurrent behaviour, test lifecycle, ordering, and completion rather than depending solely on fixed sleeps.
+
+## Comments and documentation
+
+Explain purpose, non-obvious decisions, invariants, ownership, compatibility requirements, and concurrency contracts. Do not narrate self-explanatory statements. Write Javadoc for public APIs and complex contracts where useful. Preserve the existing source licensing and copyright conventions.
+
+## Static analysis
+
+Treat SonarCloud and other static-analysis results as findings to investigate, not automatic proof of defects. Fix actual issues. For justified false positives, use a narrowly scoped suppression or accepted finding with a concise technical explanation. Do not distort working architecture merely to achieve a passing grade.
+
+## Changes, commits, and validation
+
+- Keep formatting-only refactors separate from behaviour changes. Preserve interfaces, execution order, exceptions, concurrency, and lifecycle semantics when formatting.
+- Keep commits focused and use the repository's established commit-message and Jira-reference conventions.
+- Inspect diffs for accidental unrelated changes.
+- Verify compilation and run focused tests for relevant changes; run broader regression suites when warranted.
+- Report tests and builds as passing only when results have been observed.
+- Do not create branches, Jira issues, run Jenkins, or merge without the required approval.
+
+## Adoption
+
+These standards govern future and substantively modified Java code. Clean up existing code gradually when touching the relevant area; do not initiate a repository-wide reformat. If an existing public interface or compatibility requirement conflicts with a style preference, preserve behaviour and document the exception.

Jenkinsfile

@@ -139,10 +139,16 @@ def runBlock = { String stageName ->
             echo "${stageName}: running ${selectedTests.size()} test classes"
 
             def selector = selectedTests.join(",")
-            sh """#!/bin/bash
-                set -euo pipefail
-                mvn clean test -Dtest='${selector}' ${mavenCommon}
-            """
+            // Start each block with fresh authentication and data state.
+            // Keep MAPS_DATA inside this workspace even if the agent defines it globally.
+            def testDataDirectory = "${pwd()}/target/data"
+            withEnv(["MAPS_DATA=${testDataDirectory}"]) {
+                sh """#!/bin/bash
+                    set -euo pipefail
+                    rm -rf -- target/data
+                    mvn clean test -DMAPS_DATA='${testDataDirectory}' -Dtest='${selector}' ${mavenCommon}
+                """
+            }
         }
 
         stage("${stageName} - JUnit") {

docs/protocols/mqtt-sn-2-csd01-review-status.md

@@ -0,0 +1,34 @@
+# MQTT-SN 2.0 CSD01 architecture and interoperability status
+
+Reference: MQTT-SN 2.0 CSD01 (August 2026), OASIS source revision `0dae5066d123d068ddca2ddb861ce8858aafee92`.
+
+Tracking: MSG-324. This document distinguishes implementation changes from executed validation. **It is not a declaration of full CSD01 conformance.**
+
+| Requirement | CSD01 trace | Implementation status | Validation still required |
+| --- | --- | --- | --- |
+| Dual-version sessionless PUBWOS safety | §3.6, topic-alias clauses | Unprotected predefined-alias form rejected on shared dual-version listener; direct-topic form retained | MQTT-SN 1.2 colliding SUBSCRIBE and allowed/unallowed PUBWOS integration tests |
+| Protected sender/Client ID session isolation | §3.17-3 to §3.17-5 | Protection decoder carries the verified sender; MAPS session key combines sender and wire Client ID; mismatched subsequent packets fail closed | Two-sender/two-Client-ID, reconnect, replay and authentication-failure integration tests |
+| Outbound persistent QoS 1/2 recovery | §4.1.1, §4.4.1-1 | **Not implemented.** The outgoing manager still retains the original protocol's in-flight callbacks and packet state. | Durable session-scoped state, three QoS stages, Clean Start, expiry and restart |
+| Existing virtual-connection CONNECT takeover | §4.14-4 | A verified CONNECT replaces the old v2 protocol context; close and failed-start cleanup now check protocol identity before removing a virtual connection | Connected/asleep/awake takeover and Will/session-expiry lifecycle tests |
+| Malformed/protocol error removes connection | §4.12.1-2, §4.12.2-1 | Established v2 IOException path closes and removes the affected virtual connection; v1.2 path unchanged | Malformed frame/state-transition and multi-client isolation tests |
+| Duplicate QoS 2 PUBREL | §4.3.4-12 | Committed identifiers and PUBCOMP-write-pending identifiers guard one MAPS transaction commit; completed transaction references now leave the protocol map after PUBCOMP write completion | Concurrent duplicate before PUBCOMP callback; after-completion protected UDP regression added but not executed |
+| Sleeping-message enqueue/wake race | §3.15, §4 wake semantics | ASLEEP/AWAKE state decisions and queue insertion coordinate under the queue monitor; network delivery is outside that monitor | Deterministic latches/barriers test, ordering and Remaining Messages accounting |
+| Post-CONNACK enhanced-authentication reauthentication | §4.11.1.1 | **Not supported.** Lifecycle currently permits AUTH only during initial authentication, not after CONNECT completes. Do not claim complete enhanced-authentication support. | Specification/product decision before implementation |
+| MQTT-SN 1.2 interoperability | 1.2 standard | Shared listener and legacy 1.2 path retained | MQTT-SN 1.2 full regression suite |
+| Protection schemes and negative interoperability | §3.17 | Previously passed 15 reference-client protection schemes, before this change block | Re-run after current modifications, including identity and reconnect cases |
+
+## Cleanup and deployment gates
+
+The obsolete `MQTT_SNProtocolV2`, `PacketFactoryV2`, and the legacy v2 packet/state/listener hierarchy have been removed (32 files). The active `MqttSn2Protocol`, its CSD01 codecs, security providers and the MQTT-SN 1.2 implementation are retained. Compile and regression-test the removals before merging; absence of references in repository searches is not proof of runtime compatibility.
+
+Format all remaining active Java sources using the repository's Google Java conventions with a 180-character line limit, and use the standard Maps Messaging copyright/license header. Formatting must not alter packet handling or protocol semantics.
+
+Do not merge, change Jira status, claim full CSD01 conformance, or queue Jenkins until local 1.2 and 2.0 regression tests are confirmed. A green pre-change Jenkins run does not validate these changes.
+
+## Outstanding MSG-324 correctness gates (11 October 2026)
+
+- **Outbound QoS persistence:** Still outstanding. The protocol-instance delivery queue and callback references must be integrated with MAPS session persistence before recovery can be claimed.
+- **Negative acknowledgements:** Terminal PUBACK, PUBREC rejection, and PUBCOMP now resolve the MAPS delivery completion callback exactly once, whether positive or negative. The unit assertions have been updated; protocol-level transaction ACK/rollback sequencing still requires integration validation.
+- **QoS 2 packet identifier lifecycle:** Completed IDs remain in a finite (16-bit) set until fresh non-DUP reuse or protocol close; verify reconnect, bounded retention and rollback semantics. The completed transaction object is now removed from the local map on PUBCOMP completion.
+- **Asynchronous ownership:** Address-scoped virtual connection removal is now guarded by protocol identity; asynchronous session creation and pending destination callbacks still need race tests.
+- **Build gate:** No compile or targeted MQTT-SN 1.2/2.0 suite was executed for this change block; Jenkins must remain unqueued until these items are resolved.

docs/protocols/mqtt-sn-2-protection.md

@@ -0,0 +1,64 @@
+# MQTT-SN 2.0 protection (CSD01)
+
+MQTT-SN 2.0 protection uses Java ServiceLoader implementations of
+`MqttSn2CryptoProvider`, the Authentication library's
+`SecretKeyManager`, and the durable indexed counter store.
+
+## Configuration
+
+The MQTT-SN protocol configuration may contain a private `protection`
+section (it is deliberately not exposed in the REST protocol DTO).
+
+```yaml
+protection:
+  enabled: true
+  senderIdentifier: "0011223344556677"
+  outboundScheme: 0x48
+  acceptedSchemes: "0x48,0x49"
+  counterDirectory: "/opt/maps_data/mqtt-sn/protection"
+  keyEntryPasswordEnv: "MQTT_SN_KEY_ENTRY_PASSWORD"
+  keyStore:
+    type: JCEKS
+    store: file
+    path: "/opt/maps_data/security/mqtt-sn.jceks"
+    passphrase: "<resolve through existing secure configuration>"
+  senderAliases:
+    "0011223344556677:72": "gateway-aes-256"
+    "aabbccddeeff0011:72": "sensor-aes-256"
+```
+
+This block is nested inside the MQTT-SN protocol configuration, not at the
+top level of the server configuration. Set the named environment variable in
+the server process to the **key-entry** password. The keystore passphrase is
+loaded by the existing Authentication `KeyStoreManager` configuration.
+Do not place a real passphrase into source-controlled configuration files.
+Sender identifiers must be 8 octets, hex encoded, with lowercase keys in
+`senderAliases`; suffix is the decimal scheme identifier.
+
+An endpoint fails initialization when a selected ServiceLoader provider,
+outbound key, or durable counter store is unavailable. Registration for UDP
+reads takes place only after successful protection initialization.
+
+## Supported scheme families
+
+* `0x00`, `0x01`: HMAC-SHA256 / HMAC-SHA3-256
+* `0x40`–`0x45`: AES-CCM 64/128 tag, 128/192/256 keys (Bouncy Castle)
+* `0x46`–`0x48`: AES-GCM 128/192/256 keys
+* `0x49`: ChaCha20-Poly1305
+
+Providers are discovered through the standard
+`META-INF/services/...MqttSn2CryptoProvider` resource. Discovery is not
+authorization: only `acceptedSchemes` are enabled on the endpoint.
+
+The encryption nonce is derived from SHA-256 of the serialized envelope
+prefix. Authenticate/decrypt and validate the inner packet **before**
+advancing durable replay state. Outgoing counters use durable block
+reservations; unused values are discarded across restart.
+
+## Validation before deployment
+
+Do not enable on production endpoints until the server has compiled against
+`authentication_library:3.0.3-SNAPSHOT`, focused provider and envelope
+tests have passed, and independent interop vectors have confirmed
+CSD01 nonce/tag handling. The sender/security association must have a
+well-defined key rotation and counter continuity policy.