feat: add JetStream KV (Key-Value Store) support

[hermes-fco]

Summary

Adds Nats::KV — a JetStream-based Key-Value Store for nats.raku.

New Features

• Nats::KV class with put / get / delete / keys / watch operations
• Nats.kv method to create KV bucket instances
• KV buckets use JetStream streams with max_msgs_per_subject=1 and discard=new
• Get uses DIRECT.GET.LAST (already implemented)
• Delete publishes a tombstone (empty payload)
• Watch subscribes to $KV.<bucket>.> for real-time changes
• Keys lists all non-tombstoned subjects in the bucket

Files Changed

• lib/Nats/KV.rakumod — new KV class
• lib/Nats.rakumod — added use Nats::KV, kv() method, POD docs
• META6.json — added Nats::KV to provides
• t/kv.rakutest — tests for put/get/delete/keys/watch/destroy

Test Plan

• Create bucket
• Put and get
• Get missing key (returns Nil)
• Overwrite (last write wins)
• Delete (tombstone)
• List keys
• Watch changes
• Nats.kv factory method
• Destroy bucket

[hermes-fco]

Code Review — feat: add JetStream KV (Key-Value Store) support

Verdict: Request Changes (0 critical, 3 warnings, 4 suggestions)

Adds Nats::KV class for JetStream-based key-value operations. 4 files changed, +246/−8 lines. Includes comprehensive test suite (9 subtests).

---

✅ Looks Good

• Clean module structure — clear section headers (CRUD, Bulk, Watcher, History), inline usage examples, good documentation comments
• lib/Nats/KV.rakumod:106-108 — Uses DIRECT.GET.LAST for efficient reads, avoiding per-key consumer setup
• lib/Nats/KV.rakumod:43-47 — create() is idempotent — existing buckets are reused, which is correct KV behavior
• lib/Nats/KV.rakumod:62-63 — Str() coercion on $value parameter in put() allows passing non-String values
• Test coverage — 9 subtests covering create, put/get, missing key, overwrite, delete, keys, watch, factory method, destroy
• Security scan — No secrets, credentials, merge conflict markers, or debug statements found

---

⚠️ Warnings

• lib/Nats/KV.rakumod:83-98 — keys() does not filter tombstones. Deleted keys publish an empty-payload message (line 77), but the subject persists in state.subjects. After $kv.delete('foo'), keys() will still return 'foo' — contradicting the stated behavior in the PR body ("non-tombstoned subjects"). Suggestion: check each subject's last message payload (empty = tombstone) and filter them out, or document this as a known limitation.

• lib/Nats/KV.rakumod:50-57 — info() and destroy() call methods on $!stream without a nil guard. If a user creates a KV object via $nats.kv('bucket') and calls .info() or .destroy() before .create(), $!stream is the Any type object — the error will be "No such method 'info' for invocant of type 'Any'", which is cryptic. Add a guard:

  method info {
      die "Bucket not created. Call .create first." unless $!stream.defined;
      $!stream.info;
  }

• t/kv.rakutest:24,39,46-48,58 — Tests use sleep 0.1 / sleep 0.2 for async synchronization between put and get/delete/keys. This is fragile in CI — under load, 100ms may not be enough for NATS + JetStream to process. Suggestion: use the $kv.watch supply to await the write before reading, or poll with retry:

  # Instead of: $kv.put('hello', 'world'); sleep 0.1;
  $kv.put('hello', 'world');
  await $kv.watch.supply.head(1).Promise;  # wait for any change
  my $val = $kv.get('hello');

---

💡 Suggestions

• lib/Nats/KV.rakumod:62-64 — put() is fire-and-forget (discards the result of $!nats.publish). While this matches NATS pub/sub semantics, consider accepting an :$ack parameter or returning the publish result so callers can await delivery confirmation.

• lib/Nats/KV.rakumod:92-97 — The gather/take block for key extraction can be expressed more idiomatically without gather:

  %info<state><subjects>.grep(*.starts-with($prefix)).map(*.substr($len)).Seq

• lib/Nats/KV.rakumod:112-114 — history() uses a generic die. Consider throwing a typed exception for better error handling upstream:

  die X::Nats::KV::NotSupported.new: :feature<history>

• META6.json — The test-depends, resources, and trailing-comma changes are purely cosmetic (multi-line [⏎ ] → []). These add noise to the diff without functional change. Consider reverting or separating into a style-only commit.

---

📊 Stats

Metric	Value
Files changed	4
Lines added	+246
Lines removed	−8
New tests	t/kv.rakutest (9 subtests)
Security scan	Clean — no secrets, debug statements, or merge conflicts

---

Reviewed by Hermes Agent

[hermes-fco]

PR #9 Code Review — JetStream KV (Key-Value Store)

TL;DR: 🔴 Changes Requested — one critical type bug, plus several warnings and suggestions.

🔴 Critical

• lib/Nats/KV.rakumod:67 — Return type constraint violated: method get(Str $key --> Str) returns Nil on missing/deleted keys (lines 69, 71). Raku's --> Str type constraint will throw X::TypeCheck::Return at runtime:

  Type check failed for return value; expected Str but got Nil (Nil)
  

  Fix options:
  • Change return type to --> Str:_ (accepts both defined Str and Nil/type objects)
  • Or return Str type object instead of Nil for absent keys, and let callers test .defined

⚠️ Warnings

• lib/Nats/KV.rakumod:69,85 — Dead without check on always-defined values: $!stream.info and $!stream.get-last-msg both return a Supply (via $!nats.request). Supplies are always defined, so return Nil without $resp on line 69 and return ().Seq without $resp on line 85 are dead code — they'll never trigger. The real failure case is if the NATS request itself fails (network error, timeout), but that would manifest as an unhandled exception, not a falsy value.

• lib/Nats/KV.rakumod — Missing use Nats::Stream: The class uses Nats::Stream in !build-stream (line 33) but never imports it. This works only because Nats.rakumod already does use Nats::JetStream before use Nats::KV. However, standalone use Nats::KV would fail because Raku can't auto-load Nats::Stream (it lives inside lib/Nats/JetStream.rakumod, not its own file). Add use Nats::JetStream to lib/Nats/KV.rakumod for robustness.

• t/kv.rakutest — Fragile sleep-based synchronization: Tests use sleep 0.1 and sleep 0.2 as wait mechanisms between publish and get operations. These are race conditions that will cause flaky CI failures under load. Fix: Use await $stream.info.Promise to confirm the stream state after operations, or use a retry-with-timeout pattern.

💡 Suggestions

• lib/Nats/KV.rakumod:85 — keys() calls $!stream.info directly, bypassing self.info: self.info exists as a public accessor (line 50-51) but keys() uses $!stream.info directly. Consider using self.info for consistency, or inline the logic — the current indirection through a private attribute is confusing.

• lib/Nats/KV.rakumod:110-113 — history() is a stub: The method just dies with an explanation. Consider whether users should discover this at runtime. A better approach: document that max_msgs_per_subject=1 KV buckets don't support history, and either (a) remove the method entirely, or (b) make it return an empty list instead of dying.

• META6.json — Mix of real and formatting changes: The diff includes JSON formatting changes (trailing comma removal, resources array reformatting, Unicode → escape sequence for Corrêa) mixed with the actual KV additions. Consider separating formatting changes into their own commit for cleaner review.

• t/kv.rakutest — No done-testing inside subtests: While done-testing at the end is fine, consider adding explicit done-testing inside each subtest block for better isolation — if one subtest fails, it's clearer where the failure happened.

✅ Looks Good

• Clean, well-documented API design — the KV class is well-structured
• create is idempotent — reusing existing buckets is a practical design choice
• Tombstone pattern for delete (empty payload) — aligns with NATS KV spec
• watch returns a standard Nats::Subscription — composes well with existing APIs
• keys correctly handles the subject prefix stripping from stream state
• Comprehensive test plan: create, put/get, missing key, overwrite, delete, keys, watch, factory method, destroy
• max-age parameter support with sensible default (0 = no TTL)
• Security scan: clean (no secrets, no merge markers, no debug leftovers)

---

Reviewed by Hermes Agent

[hermes-fco]

Code Review — feat: add JetStream KV (Key-Value Store) support

Verdict: Comment (0 critical, 1 warning, 4 suggestions)

Adds Nats::KV — a JetStream-based Key-Value Store with put/get/delete/keys/watch/destroy. Solid implementation: 4 files, +246 −8, well-structured new module plus tests.

---

✅ Looks Good

• lib/Nats/KV.rakumod — Clean class design. Proper use of $!stream, clear method boundaries, good separation of CRUD / bulk / watch operations.
• method get (lib/Nats/KV.rakumod:67-73) — Excellent tombstone detection: checks $msg.payload.chars > 0 to distinguish deleted keys from missing ones. The without guard for $resp and $msg is correct.
• method put (lib/Nats/KV.rakumod:62) — Str() $value coercion is idiomatic Raku — allows $kv.put('count', 42) without explicit .Str.
• method delete (lib/Nats/KV.rakumod:76-78) — Correct tombstone publishing (empty payload) per NATS KV spec.
• method keys (lib/Nats/KV.rakumod:88-89) — Proper try from-json + $! check for JSON parse errors.
• t/kv.rakutest — Good coverage: create, put/get, missing keys, overwrite, delete, list keys, watch, factory method, destroy. 9 subtests with clear assertions.
• META6.json — Nats::KV correctly added to provides and kv tag added.
• POD documentation — Good usage examples in lib/Nats.rakumod (lines 388-444) showing put/get/delete/keys/watch.
• Security scan — Clean. No secrets, debug statements, or merge conflict markers.

⚠️ Warnings

• lib/Nats/KV.rakumod:50-57 — method info and method destroy lack nil-guards on $!stream. If a user calls $kv.info or $kv.destroy before $kv.create, $!stream is an uninitialized Any and the error is cryptic: "No such method 'info' for invocant of type 'Any'". Add a guard with a clear message:

  method info {
      die "Bucket not created. Call .create first." unless $!stream.defined;
      $!stream.info;
  }
  
  method destroy {
      die "Bucket not created. Call .create first." unless $!stream.defined;
      $!stream.delete;
  }

  The method keys has the same potential issue — add a similar $!stream.defined check before line 84.

💡 Suggestions

• lib/Nats/KV.rakumod:24 — has $.stream makes the stream attribute public. If external access is not intended, consider has $!stream (fully private). If public access is by design (e.g., for advanced users who want to call $kv.stream.consumer(...)), document it in the POD.

• lib/Nats/KV.rakumod:67 — (--> Str) return annotation with return Nil. This is idiomatic Raku (Nil bypasses type checks), but the annotation implies to readers that .get always returns a string. Since Nil is returned for missing keys, consider omitting the annotation or using (--> Mu):

  # Clearer about Nil possibility
  method get(Str $key --> Mu) { ... }
  # Or just omit
  method get(Str $key) { ... }

• META6.json — Unicode escape changed from "Fernando Corrêa de Oliveira" to "Fernando Corr\u00ea de Oliveira". This is a cosmetic JSON encoding change likely introduced by a serializer. No functional impact, but reduces human readability. Consider reverting to the UTF-8 literal.

• t/kv.rakutest:24,39,46,48,58 — sleep calls for async coordination are fragile. A 0.1s sleep may not be enough under CI load, leading to flaky tests. Consider using await $kv.get(...) with a retry loop or Promise.in(...) timeout instead of fixed sleeps. (Low priority — this is a common pragmatic pattern in NATS integration tests.)

🔍 Minor observations (non-blocking)

• The Nats::KV header comment block (lines 3-17) shows $kv.put('foo', 'bar') using parens, but the actual method uses colon syntax self!nats.publish: .... Minor inconsistency — purely cosmetic.
• method history (line 112) — good that it's not just missing but explicitly fails with a clear message. Nice touch.

---

📊 Stats

Metric	Value
Files changed	4
Lines added	+246
Lines removed	−8
New files	lib/Nats/KV.rakumod (115 lines), t/kv.rakutest (86 lines)
New tests	9 subtests covering create/put/get/delete/keys/watch/destroy
Security scan	Clean

---

Reviewed by Hermes Agent

[hermes-fco]

Code Review — feat: add JetStream KV (Key-Value Store) support

Verdict: Request Changes (0 critical, 1 warning, 2 suggestions)

Adds a new Nats::KV class for JetStream Key-Value operations, with CRUD (put/get/delete), bulk key listing, a change watcher, and a convenience $nats.kv method. 4 files, +246/−8. Clean security scan.

---

⚠️ Warnings

• lib/Nats/KV.rakumod:50-56 — method info and method destroy delegate to $!stream.info / $!stream.delete without a nil-guard. If .create hasn't been called first, $!stream is the type object and the resulting error will be cryptic (e.g., "No such method 'info' for invocant of type 'Nats::Stream'"). Add an explicit guard:

  method info {
      die "Bucket not created. Call .create first." unless $!stream.defined;
      $!stream.info;
  }
  
  method destroy {
      die "Bucket not created. Call .create first." unless $!stream.defined;
      $!stream.delete;
  }

  This follows the pattern documented in the Raku idiom checklist for attribute nil-guards.

💡 Suggestions

• lib/Nats/KV.rakumod:92-97 — The gather/take in keys() is a simple filter+transform that could be expressed more concisely as a functional pipeline:

  method keys(--> Seq) {
      ...
      %info<state><subjects>.List
          .grep(*.starts-with($prefix))
          .map(*.substr($len))
          .Seq
  }

  Per the Raku idiom checklist, prefer .grep().map() over gather/take for simple filter+transform operations.

• lib/Nats/KV.rakumod:40 — The |($!description ?? :$!description !! Empty) pattern works but the with form is more idiomatic for optional parameters:

  |(:$!description with $!description)

  This also correctly handles the Nil case (though truthiness check is equivalent here).

✅ Looks Good

• lib/Nats/KV.rakumod — Clean module structure with clear method separation (CRUD / Bulk / Watcher / History). Good inline documentation with usage examples and subject-format notes.
• method get(Str $key --> Str) — Correctly handles (--> Str) with return Nil for missing values, which is idiomatic in Raku (Nil bypasses return type checks).
• method put(Str $key, Str() $value) — The Str() coercion on $value is a nice touch, allowing callers to pass non-String values without explicit conversion.
• method keys — The try from-json + $! error handling pattern is idiomatic Raku. Properly filters subjects by the bucket prefix.
• method history — Explicitly unsupported with a clear die message explaining why. Good defensive design.
• t/kv.rakutest — Comprehensive test coverage: create, put+get, get missing, overwrite, delete, list keys, watch, factory method, destroy. 9 subtests that exercise the full API surface.
• lib/Nats.rakumod:240-242 — The method kv on Nats is a clean factory, consistent with the existing method stream pattern.
• META6.json — Properly updated: description, provides entry, tags, test-depends. The \\u00ea fix in authors is a nice catch.

---

📊 Stats

Metric	Value
Files changed	4
Lines added	+246
Lines removed	−8
New tests	1 test file (t/kv.rakutest, 9 subtests)
Security scan	Clean

---

Reviewed by Hermes Agent

[hermes-fco]

Code Review — feat: add JetStream KV (Key-Value Store) support

Verdict: Request Changes (0 critical, 2 warnings, 4 suggestions)

Adds Nats::KV — a clean, well-structured JetStream KV implementation. 4 files, +246/−8 lines, with comprehensive tests (9 subtests). The overall architecture is sound, but there are some correctness and robustness concerns that should be addressed before merge.

---

✅ Looks Good

• lib/Nats/KV.rakumod — Clean module with clear method separation (CRUD, bulk, watch, history). Good documentation comments.
• t/kv.rakutest — Comprehensive test coverage: create, put/get, missing key, overwrite, delete, keys, watch, destroy. 9 subtests covering the full lifecycle.
• Idiomatic Raku: correct use of try + $! for JSON parsing in keys(), correct (--> Str) with return Nil per Raku's Nil-bypassing semantics, colon-pair syntax throughout.
• (--> Str) with return Nil in get() — idiomatically correct. Nil bypasses Raku return type checks; appropriate for signaling "key not found".
• Security scan clean — no secrets, debug statements, merge conflict markers, or TODO markers in the diff.
• META6.json correctly registers Nats::KV in provides and updates the description.

---

⚠️ Warnings

• lib/Nats/KV.rakumod:50-52 (info), lib/Nats/KV.rakumod:55-57 (destroy), lib/Nats/KV.rakumod:67-73 (get), lib/Nats/KV.rakumod:83-98 (keys) — No guard when $!stream is uninitialized. If any of these methods are called before create(), the error is "No such method 'info' for invocant of type 'Any'" — cryptic and hard to debug. Suggestion: add guards with clear error messages:

  method info {
      die "Bucket not created. Call .create first." unless $!stream.defined;
      $!stream.info;
  }

  Apply the same pattern to destroy, get, keys. This follows the Raku idiom checklist recommendation for lazy-initialized attributes.

• lib/Nats/KV.rakumod:82-98 (keys) — Does not filter tombstoned subjects. The PR body says "Keys lists all non-tombstoned subjects in the bucket", but keys() iterates ALL subjects from stream info without checking if the last message is a tombstone (empty payload). A deleted key will still appear in the output. To actually list only non-tombstoned keys, you'd need to fetch each subject's last message and check for non-empty payload. Alternatively, adjust the PR description to match the current behavior ("lists all subjects in the bucket, including deleted ones").

---

💡 Suggestions

• lib/Nats/KV.rakumod:24 — $.stream is a public attribute. This exposes an implementation detail — callers could accidentally reassign $kv.stream and break internal state. Suggestion: make it private:

  has $!stream;

• lib/Nats/KV.rakumod:43-46 (create) — Idempotency claim is optimistic. The comment says "idempotent — existing buckets are reused", but Nats::Stream.create sends a stream creation request. If the stream exists with different config, NATS returns an error. Consider either handling the "already exists" case explicitly, or softening the comment to "creates the bucket (will reuse if config matches)".

• lib/Nats/KV.rakumod:62-64 (put) — Fire-and-forget with no confirmation. put() publishes and returns immediately without awaiting confirmation. The tests work around this with sleep 0.1. This is a valid design choice for throughput, but consider documenting that put is asynchronous and callers may want to await on the returned value for ordering guarantees.

• lib/Nats/KV.rakumod:112 (history) — Unused parameter. The method takes Str $key but ignores it. Since the method always dies with an explanation, the parameter is harmless but slightly misleading. Consider removing the parameter or using it in the error message: "history($key) requires max-msgs-per-subject > 1...".

---

🔍 Minor observations (non-blocking)

• lib/Nats/KV.rakumod:93 — .List on %info<state><subjects> is redundant (arrays are already iterable). Harmless, but unnecessary.
• lib/Nats.rakumod:240-242 — kv() method cleanly passes through extra config via |c. Good use of Raku capture forwarding.

---

📊 Stats

Metric	Value
Files changed	4
Lines added	+246
Lines removed	−8
New tests	t/kv.rakutest (9 subtests)
Security scan	Clean
Syntax check	✅ Passed

---

Reviewed by Hermes Agent

[hermes-fco]

Code Review — feat: add JetStream KV (Key-Value Store) support

Verdict: Comment (0 critical, 1 warning, 3 suggestions)

Adds Nats::KV — a JetStream-based Key-Value Store with put/get/delete/keys/watch operations. Clean implementation, good test coverage, and well-structured code.

---

✅ Looks Good

• lib/Nats/KV.rakumod — Clean class design with clear separation of concerns. CRUD operations are concise and correct.
• lib/Nats.rakumod — kv() factory method is minimal and idiomatic (:$bucket colon pair, |c capture).
• t/kv.rakutest — Good test coverage (9 subtests covering create, CRUD, missing keys, overwrite, delete tombstone, listing, watch, destroy). Uses random bucket names to avoid conflicts.
• Raku idioms — Proper use of try + $! for JSON parsing, without for nil checks, colon-pair constructor syntax, (--> Seq) and (--> Str) return annotations.
• META6.json — Nats::KV correctly registered in provides, new "kv" tag added.
• Security scan — Clean (no secrets, debug statements, or merge conflict markers).

---

⚠️ Warnings

• lib/Nats/KV.rakumod:50-57 — info() and destroy() methods delegate to $!stream without a nil-guard. If called before create(), they produce a cryptic "No such method for invocant of type Any" error. Consider adding a guard:

method info {
    die "Bucket not created. Call .create first." unless $!stream.defined;
    $!stream.info;
}

method destroy {
    die "Bucket not created. Call .create first." unless $!stream.defined;
    $!stream.delete;
}

---

💡 Suggestions

• lib/Nats/KV.rakumod:67 — (--> Str) annotation with return Nil is misleading to callers. Raku allows Nil to bypass return type checks, but a (--> Str) method that returns Nil breaks type expectations. Consider removing the annotation or using (--> Mu) to signal "may return Nil".

• lib/Nats/KV.rakumod:24 — $.stream is a public attribute but it's set internally by create(). External mutation could break the object's state. Consider making it private ($!stream) and expose read-only access via a method if needed.

• lib/Nats/KV.rakumod:92-97 — The gather/take block in keys() could be simplified to a functional pipeline (per Raku idiom checklist):

@subjects.grep(*.starts-with($prefix)).map(*.substr($len)).Seq

---

📊 Stats

Metric	Value
Files changed	4
Lines added	+246
Lines removed	−8
New tests	1 test file (t/kv.rakutest, 9 subtests)
Security scan	Clean

---

Reviewed by Hermes Agent