fix(cluster-manager): announce only persisted endorsement changes

[RecursiveIntell]

Description

The truststore previously announced cluster:trust-changed whenever an endorsement merge returned successfully, even when it received only duplicates, had no target pin, or had nothing to merge. That caused consumers to refresh trust-derived state without a persisted change.

The merge now reports whether it actually saved a new endorsement. Both direct endorsement additions and same-certificate re-pins announce only after that save succeeds. Failed writes leave the live and on-disk pin unchanged and emit no notification. The cluster manager spec now describes the notification and endorsement merge behavior.

Release intent

Changelog title

Avoid unnecessary cluster trust notifications

Changelog body

Cluster trust notifications now follow successfully saved peer endorsements. Duplicate endorsements and failed writes no longer cause unnecessary refreshes.

Bumps

• services: patch
• nvpair-cluster-manager: patch
• nvpair-engine-manager: none
• nvpair-errors: none
• nvpair-job-scheduler: none
• nvpair-manual-nodes: none
• nvpair-node-info: none
• nvpair-node-scanner: none
• nvpair-node-settings: none
• nvpair-proxy: none
• nvpair-tui: none
• nvpair-ui-broker: none
• nvpair-workload-manager: none

Scope

• Update endorsement persistence and notification decisions in services/nvpair-cluster-manager/truststore.go.
• Add regression coverage for direct merges, identical re-pins, duplicate and concurrent submissions, missing targets, failed writes, and read-back after persistence.
• Document cluster:trust-changed and endorsement merge semantics in services/nvpair-cluster-manager/spec.md.
• Leave services/versions.json and CHANGELOG.md to the release-intent automation.

Validation

• Passed on Windows: go test ./... -count=1 from services/nvpair-cluster-manager.
• Passed on Windows: go test ./... -run '^TestCluster(ManagerPairing|Fanout|Leave)$' -count=1 -timeout=180s from services/tests.
• Passed: git diff --check upstream/develop...HEAD.
• Race tests were unavailable on this Windows setup because CGO is disabled and no C compiler is installed.

Risk

The change affects when truststore notifications are sent after endorsement updates. It does not change certificate validation, endorsement verification, admission authorization, PIN handling, or revocation. No data migration or JSON-RPC payload change is required.

Checklist

• I have read the Contributing Guidelines.
• Every commit is signed off (git commit -s), certifying the Developer Certificate of Origin.
• New or existing tests cover the change.
• Relevant documentation is updated.
• I checked the diff, changed filenames, and commit messages for credentials, private data, internal URLs, internal issue identifiers, and generated artifacts.
• I recorded the validation commands and results above.
• I declared version bumps in the release-intent block above. services/versions.json is written by automation — do not edit it by hand.

[RecursiveIntell]

Reconciliation and validation update

Rechecked this PR against NVIDIA main at 13b68115fa2c9c1d94f1ead1358f8d5a527cfecf. That commit is already an ancestor of the branch, so no rebase, merge commit, or history rewrite was necessary. Updated head: da7df0a48060d7e729519fb5d6884778fb604c29.

What changed in this follow-up

The new commit is test-only; it preserves the existing runtime fix and the nvpair-cluster-manager patch bump from 1.1.4 to 1.1.5. The full PR remains limited to truststore.go, truststore_announce_test.go, and services/versions.json.

The tests now check the notification/persistence boundary rather than treating an in-memory read as disk-persistence evidence:

• Both AddEndorsements and identical-certificate, same-admission-epoch Pin merge an existing/new/duplicate batch with one notification. Repeating the batch or submitting an empty batch stays silent and leaves the stored contents unchanged.
• The notification callback reads the live pin and opens a fresh truststore from disk. The callback successfully read the updated live pin and reloaded it from disk; its read-lock acquisition provides evidence that the mutation lock was released before the callback ran.
• An injected failure at the final file-replacement step returns an error through both APIs, emits no notification, preserves the old live and on-disk endorsements, and leaves no temporary file. Retrying after storage recovery persists the update and announces once.
• In the focused race run, each of 50 repetitions exercised 16 concurrent identical submissions, split between the two APIs; each repetition produced one stored endorsement and one notification. The observer uses an atomic counter.
• Adding endorsements for a missing peer stays silent without creating live or on-disk state.

As a regression control, I ran the same test file against otherwise unchanged main at the SHA above. It fails on duplicate/no-op notifications, the missing-peer case, and the concurrent case (16 notifications instead of 1). Those cases pass with this PR's implementation. The failed-write cases already pass on main; they protect existing behavior rather than claiming a newly repaired write-atomicity defect.

Fresh local validation

Validation was executed on the pre-commit worktree whose three changed-path SHA-256 values match the blobs committed at da7df0a; those receipts therefore record parent HEAD be34de2, not the candidate commit itself. The focused/module checks passed against those matching candidate blobs. Repository-wide make test remains blocked by failures reproduced on clean main. After committing, I also reran go test ./... -race -run TestTrustStore -count=1 -timeout=60s -json from the cluster-manager module at da7df0a; it passed with no failed or skipped test events.

Environment: Linux/amd64, Go 1.25.6, Node 26.0.0, npm 11.12.1; Go checks used GOMAXPROCS=2.

From services/nvpair-cluster-manager:

go test ./... -count=1 -timeout=180s -json
go test ./... -race -count=1 -timeout=180s -json
go test ./... -race -run TestTrustStore -count=50 -timeout=120s -json
go vet ./...

All passed. The full module normal and race runs each executed 106 top-level tests with no failed or skipped test events. The repeated run includes the new concurrent regression; it is not a claim of universal race-freedom.

Also passed:

• Repository make check: SPDX headers, build-script verification, lint, type checks, service contracts, and 208 desktop unit tests.
• npm run dead-code:check from desktop.
• gofmt verification and git diff --check.
• A Linux cluster-manager build with VCS stamping disabled and the version read from services/versions.json; --version returned 1.1.5.
• Real-process integration from services/tests, using temporary cluster configurations and synthetic peers:

GOFLAGS=-buildvcs=false go test ./... \
  -run '^TestCluster(ManagerPairing|Fanout|Leave)$' \
  -count=1 -timeout=180s -json

All three selected integration tests passed without skips. This covers pairing/removal, transitive membership propagation, and leave behavior; it is not a full cross-process-suite result.

Broader gate remains open

Repository-wide make test is not green:

1. The ordinary command stops while LM Studio's TestMain builds its binary: error obtaining VCS status: exit status 128. The same build error occurs in the clean exact-main control.
2. GOFLAGS=-buildvcs=false make test reaches the LM Studio suite but fails TestHandleHTTP_RealSocketFlushDeadline: workload never terminated after the client stopped reading (flush path not deadline-aware).
3. That exact test fails with the same message on clean main under the same workaround. No LM Studio source is changed by this PR, and I have not hidden or skipped that failure to claim a full pass.

These are local results bound to the stated source snapshot, not hosted-CI results. No hosted checks are currently reported for this PR, and maintainer review is still required. Windows/macOS execution, power-loss durability, live-engine testing, and deployment are not claimed. The change does not modify certificate/endorsement verification, admission authorization, pairing-PIN security, or mTLS policy.

The original commits are retained, and the follow-up uses the same author with a matching DCO sign-off. This update does not merge the PR or change its draft state. The next gate is maintainer review of this focused fix and disposition of the broader baseline failure before treating the repository-wide validation gate as satisfied.

[jlacroix82]

Independent confirmation from a 9-node fleet (8× DGX Spark + 1 gateway box, all on PAIR 0.1.1 services): every node's nvpair-node.service journal shows cluster:trust-changed at a constant ~5.3/s — exactly 3,200 events per 10-minute bucket on every Spark, all day, with no pairing activity and trusted/*.json mtimes untouched for a week.

Tracing it landed on the same line this PR fixes: in TrustStore.Pin, the identical-re-pin branch sets changed = err == nil after mergeEndorsementsLocked, so a gossip re-pin that merges nothing still fires announce → announceTrustChanged → cluster:trust-changed. With nine peers gossiping, that is the storm. Each event also makes the broker re-derive scanner trust and push node-info, so it is not just log noise.

The fix here (announce only when an endorsement was actually persisted) matches what I would have written. +1 to landing it as a bug fix.

[kjlubick]

We'll need to rebaseline this against develop. I can help with that today, as I review the change.

[kjlubick]

LGTM. I tweaked some of the tests to be more aligned with project guidelines and update the spec.

Thank you for the contribution!

[Noah-Tervalon-Nvidia]

LGTM, thanks for catching this!