Make pinned client Pin ID handling tolerant of duplicate headers

[colprog]

This PR makes pinned client Pin ID handling more robust when multiple Nats-Pin-Id header values are present.

This was discovered while testing an echo service. The echo service returns incoming headers as-is, so if a request already contains a Nats-Pin-Id the client can observe multiple Nats-Pin-Id values in the same header collection.

The previous code was not necessarily wrong for the normal case, but it assumed Nats-Pin-Id behaved as a single-value header. With duplicate values, converting the full StringValues to string could use a combined value instead of the current pin id.

This change updates the client to use the last Nats-Pin-Id value when multiple values are present, matching the latest value appended by the server and making pinned client handling more defensive.

Changes

• Use TryGetLastValue("Nats-Pin-Id", out var pinId) when updating the consumer pin id.
• Add a regression test covering duplicate Nats-Pin-Id headers.
• Keep existing behavior unchanged for the common single-header case.

Why
This is primarily a robustness improvement rather than a fix for the standard path. It protects the client from stale pin ids when server responses preserve previous headers and append a newer Nats-Pin-Id.

Testing

• Added coverage for multiple Nats-Pin-Id values and verified that the latest value is used.

[synadia-claude-reviewer]

Claude finished @colprog's task in 2m 8s —— View job

---

PR Review: Make pinned client Pin ID handling tolerant of duplicate headers

• Read CLAUDE.md and gather repo context
• Examine PR diff
• Read relevant source files for context
• Analyze changes for correctness, security, and performance
• Post inline comments and final review summary

---

Summary

The fix is correct and well-scoped. StringValues.ToString() on a multi-value header concatenates with , (e.g. "pin-stale, pin-current"), so the old code would send a malformed pin ID to the server. Switching to TryGetLastValue properly extracts only the last appended value.

Findings

NatsJSExtensionsInternal.cs — core fix ✓

The replacement of TryGetValue + pinIdValues.ToString() with TryGetLastValue is the right call. The pre-existing !string.IsNullOrEmpty(pinId) guard also doubles as protection against the unlikely case where TryGetLastValue returns true with a null last element (its [NotNullWhen(true)] contract can technically be violated if StringValues contains a null entry, though this won't happen with NATS wire data).

One edge case not covered: TryGetLastValue will throw IndexOutOfRangeException if StringValues.Empty is somehow stored for the key. This is pre-existing behavior in the method itself, not introduced here, and can't happen from the NATS protocol parser. Not a blocker.

NatsJSConsume.cs — debug log expansion ✓

The added fields (pinId, expires, idleHeartbeat, group) are useful for diagnosing priority-group pull behavior. Evaluation is already guarded by if (_debug), so no runtime overhead in production.

Test coverage ✓

The test correctly exercises the multi-value path using StringValues(["pin-stale", "pin-current"]) and verifies "pin-current" is used. Using MockServer + GetConsumerAsync to instantiate NatsJSConsumer is reasonable given the class structure. One gap: no test for the single-value case to confirm backward compatibility, but the existing test suite covers that path.

Verdict

No blocking issues. The change is correct, conservative, and well-targeted. Good to merge.

[mtmk]

LGTM thanks @colprog