bug: 44 spec POJO fields use volatile on non-thread-safe types (Sonar S3077)

[jlouvel]

Component

Core Engine

Description

Actual behavior:

55 fields across io.naftiko.spec.*, io.naftiko.engine.*, and io.naftiko are declared volatile on non-thread-safe types — volatile List<T>, volatile Map<K, V>, volatile String, volatile char[], etc. Each one is flagged by SonarQube rule java:S3077 — "Non-thread-safe fields should not be volatile".

volatile only publishes the reference, not the contents: if thread A reads volatile List<X> xs and thread B mutates the list elements, behavior is undefined. The current code therefore gives a false sense of thread-safety.

The Naftiko spec layer is also evolving toward two new write paths:

1. Fluent Java builders — user code constructs/mutates spec objects after deserialization.
2. Runtime spec edits via the Control port — operators update a running capability without restart.

Both turn spec POJOs into read-mostly, occasionally-written, concurrently-accessed objects. The current volatile approach will not survive either.

Expected behavior:

Every multi-write field on a non-thread-safe type should hold an immutable snapshot inside an AtomicReference<T>. Writers replace the reference atomically with a freshly-copied snapshot (List.copyOf / Map.copyOf / plain String / char[].clone()); readers always see a fully-constructed value.

This satisfies S3077, gives genuine thread-safety with no synchronized, supports atomic compound updates via updateAndGet, and keeps the public Jackson-facing API as plain List<T> / Map<K, V> / String / char[].

Steps to Reproduce

1. Run the full SonarQube scan (Quality Gate workflow).
2. Filter issues by rule = java:S3077.
3. Observe 55 occurrences flagged across the codebase (run #1516, main @ dcfd01c).

By layer:

Layer	Files	Bug Count
io.naftiko.spec.* (spec POJOs)	25	44
io.naftiko.engine.* (runtime)	4	6
io.naftiko (core)	1	7
io.naftiko.spec.consumes.http (auth char[])	2	2 (MINOR but same pattern)

Files with the highest concentration:

File	Bugs
Capability.java	7
AggregateFunctionSpec.java	3
McpServerToolSpec.java	3
ObservabilitySpec.java	3

Capability File (if relevant)

Not capability-specific.

Logs & Stacktrace

Not a runtime failure (latent thread-safety issue). Sonar evidence:

• Quality Gate run #1516 — main @ dcfd01c — 59 bugs total, 55 of them java:S3077.

Version

1.0.0-alpha3-SNAPSHOT (current main)

Runtime

JVM

Agent Context (optional)

agent_name: GitHub Copilot
llm: claude-opus-4.7
tool: copilot-chat
confidence: high
source_event: SonarQube quality gate run #1516 (main @ dcfd01c)
discovery_method: code_review
files_suspected:
  - src/main/java/io/naftiko/Capability.java
  - src/main/java/io/naftiko/spec/NaftikoSpec.java
  - src/main/java/io/naftiko/spec/OperationSpec.java
  - src/main/java/io/naftiko/spec/aggregates/AggregateFunctionSpec.java
  - src/main/java/io/naftiko/spec/consumes/http/HttpClientSpec.java
  - src/main/java/io/naftiko/spec/consumes/http/HttpClientResourceSpec.java
  - src/main/java/io/naftiko/spec/consumes/http/HttpClientOperationSpec.java
  - src/main/java/io/naftiko/spec/consumes/http/OAuth2AuthenticationSpec.java
  - src/main/java/io/naftiko/spec/exposes/ServerSpec.java
  - src/main/java/io/naftiko/spec/exposes/ServerCallSpec.java
  - src/main/java/io/naftiko/spec/exposes/control/ControlServerSpec.java
  - src/main/java/io/naftiko/spec/exposes/control/ControlManagementSpec.java
  - src/main/java/io/naftiko/spec/exposes/mcp/McpServerResourceSpec.java
  - src/main/java/io/naftiko/spec/exposes/mcp/McpServerToolSpec.java
  - src/main/java/io/naftiko/spec/exposes/rest/RestServerResourceSpec.java
  - src/main/java/io/naftiko/spec/exposes/rest/RestServerOperationSpec.java
  - src/main/java/io/naftiko/spec/exposes/rest/RestServerStepSpec.java
  - src/main/java/io/naftiko/spec/exposes/skill/ExposedSkillSpec.java
  - src/main/java/io/naftiko/spec/exposes/skill/SkillToolSpec.java
  - src/main/java/io/naftiko/spec/observability/ObservabilitySpec.java
  - src/main/java/io/naftiko/spec/observability/ObservabilityTracesSpec.java
  - src/main/java/io/naftiko/spec/observability/ObservabilityMetricsSpec.java
  - src/main/java/io/naftiko/spec/observability/ObservabilityExportersSpec.java
  - src/main/java/io/naftiko/spec/scripting/OperationStepScriptSpec.java
  - src/main/java/io/naftiko/spec/util/BindingSpec.java
  - src/main/java/io/naftiko/spec/util/OperationStepCallSpec.java
  - src/main/java/io/naftiko/spec/util/StructureSpec.java

---

Proposed Fix (Phase 2 of sonar-bug-remediation)

Adopt AtomicReference<T> + immutable snapshots as the standard pattern for every flagged spec field.

Pattern A — scalar (String, Integer, ...)

private final AtomicReference<String> description = new AtomicReference<>();

@JsonProperty("description")
public void setDescription(String description) { this.description.set(description); }
public String getDescription() { return this.description.get(); }

Pattern B — List<T> with immutable snapshot

private final AtomicReference<List<InputParameterSpec>> inputParameters =
    new AtomicReference<>(List.of());

@JsonProperty("inputParameters")
public void setInputParameters(List<InputParameterSpec> params) {
    this.inputParameters.set(params == null ? List.of() : List.copyOf(params));
}
public List<InputParameterSpec> getInputParameters() { return this.inputParameters.get(); }

Pattern C — Map<K, V> with immutable snapshot

private final AtomicReference<Map<String, BindingSpec>> bindings =
    new AtomicReference<>(Map.of());

@JsonProperty("bindings")
public void setBindings(Map<String, BindingSpec> bindings) {
    this.bindings.set(bindings == null ? Map.of() : Map.copyOf(bindings));
}
public Map<String, BindingSpec> getBindings() { return this.bindings.get(); }

Public API contract preserved: getters return List<T> / Map<K, V> / String (Jackson sees no AtomicReference). The returned collections are unmodifiable by virtue of being List.copyOf / Map.copyOf.

Scope of this issue: 25 spec POJO files (44 bugs). Engine layer (Phase 3) and char[] auth fields (Phase 4) are tracked separately.

Out of scope: fluent builder methods (addX / putX / withX) — they will land in a follow-up PR once the migration is in place.