From f6e57895bb1b76fbe5c71a1d25a4fef1ffcaa241 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 11:35:54 -0700 Subject: [PATCH 01/20] docs(kv): spec + implementation plan for KV MCP LLM ergonomics (#192) Design and 13-task TDD implementation plan for the five issue-#192 KV-store MCP improvements plus two adjacent LLM-ergonomics gaps: - insert-vs-overwrite signal + set_if_absent write guard (no silent data loss) - server-side value_path for kv_set - byte-size reporting + soft 1 MiB warning - kv_set_many atomic batch write + values flag on kv_list - JSON-in-TEXT query docs + ::numeric truncation gotcha - PermissionDenied I/O-error fidelity + corrected misleading JSON-error suggestion Planning docs only; no code changes. The 0.6.1 -> 0.7.0 bump is carried by the feat!: commits the plan produces, per release-please. --- .../plans/2026-07-11-kv-mcp-llm-ergonomics.md | 1972 +++++++++++++++++ ...2026-07-10-kv-mcp-llm-ergonomics-design.md | 429 ++++ 2 files changed, 2401 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md create mode 100644 docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md diff --git a/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md b/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md new file mode 100644 index 0000000..6d30745 --- /dev/null +++ b/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md @@ -0,0 +1,1972 @@ +# KV Store — LLM-First MCP Ergonomics (Issue #192) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the HyperDB KV MCP tools materially easier for an LLM to drive — insert/overwrite signal + write guard, server-side `value_path`, byte-size reporting, batch write/read, and JSON-query/error docs — without silent data loss. + +**Architecture:** Thread a `created` bit (already computed and discarded in `upsert`) up through new `SetOutcome`/`BatchSetOutcome` types in both the sync `KvStore` and async `AsyncKvStore` twins; add `set_if_absent`, `byte_size`, and an atomic batch-if-absent variant in the API crate; then expose the new signals and two new tools (`kv_set_many`, `values` flag on `kv_list`) plus improved error fidelity and docs in the `hyperdb-mcp` crate. The breaking API change is carried by a `feat!:` commit so release-please performs the `0.6.1 → 0.7.0` minor bump. + +**Tech Stack:** Rust (workspace), `hyperdb-api` (sync + async KV over the shared `_hyperdb_kv_store` TEXT table, no PK, no `ON CONFLICT`), `hyperdb-mcp` (rmcp 1.x `#[tool_router]`), real `hyperd` subprocess for integration tests, release-please (`release-type: simple`, `bump-minor-pre-major`). + +## Global Constraints + +Every task's requirements implicitly include this section. Values copied verbatim from the spec and project rules. + +- **Base branch:** `main` @ `d2ab4fc` (== `origin/main` == `upstream/main` == tag `v0.6.1`; KV work #182/#185/#188/#189 all merged). +- **Test invocation:** all tests run under `HYPERD_PATH=~/dev/bin/hyperd` (real `hyperd` subprocess). Never invent `hyperd` flags; start servers only via `HyperProcess::new()` / Makefile / `HYPERD_PATH`. +- **CI lint gate (exact):** `cargo clippy --workspace --all-targets --all-features -- -D warnings` on CI's `stable`, plus `cargo fmt --all --check`. The workspace pins `pedantic = "warn"`, which under `-D warnings` is effectively DENY. Consequences: every new `pub fn` needs `# Errors` rustdoc, every new struct needs `#[derive(Debug)]`, and NO float casts (report raw byte counts, never computed MiB — avoids `cast_precision_loss`). +- **No narrowing `as` casts on integers** (AGENTS.md rule 7): use `TryFrom`. `value.len()` is `usize`; report it as-is or via `i64::try_from(...).unwrap_or(i64::MAX)` where an `i64` is needed — never `as i64`. +- **Sync/async twin lockstep:** every KV change lands in BOTH `kv_store.rs` and `async_kv_store.rs`. The async `set_batch` uses a different loop shape (`let mut inner; for … { if let Err(e) = …await { inner = Err(e); break; } }`) than the sync closure form — preserve each file's existing pattern. +- **Changelog:** append per-crate `## [Unreleased]` bullets ONLY. Do NOT hand-edit `Cargo.toml` versions, the workspace version, the root `CHANGELOG.md`, or `.release-please-manifest.json` — release-please owns all of those. The `0.6.1 → 0.7.0` bump is carried by a `feat!:` commit / `BREAKING CHANGE:` footer. +- **Never report a build/test green without captured output** (AGENTS.md rule 10); check exit codes; ~30s of no output = hanging/failed. +- **Run `cargo clippy` + `cargo fmt` before every commit.** +- **Update `hyperdb-mcp/src/readme.rs`** whenever the MCP tool surface changes (adding `kv_set_many`), and update the `readme_tests.rs` tool-name list to match. + +--- + +## File Structure + +| File | Responsibility | Tasks | +|---|---|---| +| `hyperdb-api/src/kv_store.rs` | Sync `KvStore`: `SetOutcome`/`BatchSetOutcome`, reshaped `set`/`set_as`/`set_batch`/`upsert`, new `set_if_absent`/`byte_size`/`entries`/`set_batch_if_absent` | 1, 3, 4, 5 | +| `hyperdb-api/src/async_kv_store.rs` | Async twins of every `kv_store.rs` change | 2, 6 | +| `hyperdb-api/src/lib.rs` | Re-export `SetOutcome`, `BatchSetOutcome` | 1 | +| `hyperdb-api/tests/kv_store_tests.rs` | Sync integration tests (real `hyperd`) | 1, 3, 4, 5 | +| `hyperdb-api/tests/async_kv_store_tests.rs` | Async integration tests | 2, 6 | +| `hyperdb-api/CHANGELOG.md` | `## [Unreleased]` bullets (Changed BREAKING + Added) | 13 | +| `hyperdb-mcp/src/error.rs` | `PermissionDenied` mapping + fix misleading 0A000/JSON suggestion | 7 | +| `hyperdb-mcp/src/server.rs` | `KvSetParams`/`KvListParams`/`KvSetManyParams`/`KvEntry`; `kv_set`/`kv_size`/`kv_set_many`/`kv_list` handlers; `value_path`; size warning + `value_bytes` | 8, 9, 10, 11 | +| `hyperdb-mcp/src/readme.rs` | JSON-query + `::numeric` docs, `kv_set_many`, `value_path`/`overwrite`/`values` | 12 | +| `hyperdb-mcp/tests/kv_tools_tests.rs` | MCP tool coverage via `TestHarness` | 8, 9, 10, 11 | +| `hyperdb-mcp/tests/readme_tests.rs` | Add `kv_set_many` to the asserted tool-name list | 12 | +| `hyperdb-mcp/CHANGELOG.md` | `## [Unreleased]` Added/Fixed bullets | 13 | + +**Task map:** 1 sync outcome · 2 async outcome · 3 sync `set_if_absent` · 4 sync `byte_size`+`entries` · 5 sync `set_batch_if_absent` · 6 async twins (3/4/5) · 7 error.rs fixes · 8 MCP `kv_set` overhaul · 9 MCP `kv_size` bytes · 10 MCP `kv_set_many` · 11 MCP `kv_list values` · 12 readme+readme_tests · 13 CHANGELOGs. + +--- + +### Task 1: `SetOutcome`/`BatchSetOutcome` + reshape sync `set`/`set_as`/`set_batch`/`upsert` + +The `created` bit is already computed in `upsert` (`updated == 0`) and discarded. Introduce the two outcome types, change `upsert` to return `Result` (`created`), and thread it up. This is the BREAKING change (`Result<()>` → `Result`). + +**Files:** +- Modify: `hyperdb-api/src/kv_store.rs:184-222` (`set`, `upsert`), `:242-253` (`set_as`), `:403-430` (`set_batch`); add the two structs near the top of the module. +- Modify: `hyperdb-api/src/lib.rs:217` (re-export) +- Test: `hyperdb-api/tests/kv_store_tests.rs` + +**Interfaces:** +- Produces: `pub struct SetOutcome { pub created: bool }`; `pub struct BatchSetOutcome { pub created: usize, pub overwritten: usize }`; `KvStore::set(&self, &str, &str) -> Result`; `KvStore::set_as(&self, &str, &T) -> Result`; `KvStore::set_batch(&self, &[(&str,&str)]) -> Result`; private `KvStore::upsert(&self, &str, &str) -> Result`. +- Consumed by: Task 2 (MCP `kv_set`), Task 7 (`kv_set_many`), Task 6 (async twin mirrors these names). + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-api/tests/kv_store_tests.rs`: + +```rust +#[test] +fn set_reports_created_then_overwritten() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("outcome")?; + let first = kv.set("k", "v1")?; + assert!(first.created, "first write of a key must report created=true"); + let second = kv.set("k", "v2")?; + assert!(!second.created, "overwrite must report created=false"); + assert_eq!(kv.get("k")?, Some("v2".to_string())); + Ok(()) +} + +#[test] +fn set_batch_reports_created_and_overwritten() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("batch_outcome")?; + kv.set("a", "1")?; // pre-existing → will be overwritten + let out = kv.set_batch(&[("a", "10"), ("b", "20"), ("c", "30")])?; + assert_eq!(out.created, 2, "b and c are new"); + assert_eq!(out.overwritten, 1, "a existed"); + assert_eq!(kv.get("a")?, Some("10".to_string())); + Ok(()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests set_reports_created` +Expected: FAIL — `no method named created` / `set_batch` returns `()` not `BatchSetOutcome`. + +- [ ] **Step 3: Add the outcome types** + +Near the top of `hyperdb-api/src/kv_store.rs` (after the existing `use` lines, before `struct KvStore`), add: + +```rust +/// Outcome of a single KV write. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SetOutcome { + /// `true` if the key did not previously exist (insert); `false` if an + /// existing value was overwritten. + pub created: bool, +} + +/// Outcome of a batch KV write. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BatchSetOutcome { + /// Number of keys newly inserted. + pub created: usize, + /// Number of keys whose prior value was replaced. + pub overwritten: usize, +} +``` + +- [ ] **Step 4: Reshape `upsert` to return `created`** + +Replace the body tail of `upsert` (`kv_store.rs:201-222`). Change the return type and return `updated == 0`: + +```rust + /// UPDATE-then-conditional-INSERT upsert. Assumes `key` is validated. + /// Returns `true` if the row was newly inserted (created), `false` if an + /// existing value was overwritten. + /// + /// Hyper has no `ON CONFLICT`; this mirrors the proven `_table_catalog` + /// idiom. The conditional INSERT uses distinct placeholders (`$4`/`$5`) + /// so it is unambiguous under the extended-query protocol. + fn upsert(&self, key: &str, value: &str) -> Result { + let store = self.store_name.as_str(); + let updated = self.connection.command_params( + &format!( + "UPDATE {} SET value = $3 WHERE store_name = $1 AND key = $2", + self.table_ref + ), + &[&store, &key, &value], + )?; + if updated == 0 { + self.connection.command_params( + &format!( + "INSERT INTO {t} (store_name, key, value) \ + SELECT $1, $2, $3 \ + WHERE NOT EXISTS (SELECT 1 FROM {t} WHERE store_name = $4 AND key = $5)", + t = self.table_ref + ), + &[&store, &key, &value, &store, &key], + )?; + } + Ok(updated == 0) + } +``` + +- [ ] **Step 5: Reshape `set`, `set_as`, `set_batch`** + +`set` (`kv_store.rs:191-194`): + +```rust + pub fn set(&self, key: &str, value: &str) -> Result { + validate_kv_name(key, "key")?; + Ok(SetOutcome { created: self.upsert(key, value)? }) + } +``` + +`set_as` (`kv_store.rs:249-253`): + +```rust + pub fn set_as(&self, key: &str, value: &T) -> Result { + validate_kv_name(key, "key")?; + let json = serde_json::to_string(value).map_err(|e| Error::serialization(e.to_string()))?; + Ok(SetOutcome { created: self.upsert(key, &json)? }) + } +``` + +`set_batch` (`kv_store.rs:412-430`) — accumulate per-entry bool into the counts; keep the closure form: + +```rust + pub fn set_batch(&self, entries: &[(&str, &str)]) -> Result { + for (key, _) in entries { + validate_kv_name(key, "key")?; + } + self.connection.begin_transaction_raw()?; + let result = (|| { + let mut outcome = BatchSetOutcome { created: 0, overwritten: 0 }; + for (key, value) in entries { + if self.upsert(key, value)? { + outcome.created += 1; + } else { + outcome.overwritten += 1; + } + } + Ok(outcome) + })(); + match &result { + Ok(_) => self.connection.commit_raw()?, + Err(_) => { + let _ = self.connection.rollback_raw(); + } + } + result + } +``` + +Also update the `set_batch` rustdoc `# Errors` block to keep the existing bullets (unchanged text) and the `set`/`set_as` doc comments' first line to mention the returned outcome. + +- [ ] **Step 6: Re-export the types** + +In `hyperdb-api/src/lib.rs:217`, change: + +```rust +pub use kv_store::KvStore; +``` + +to: + +```rust +pub use kv_store::{BatchSetOutcome, KvStore, SetOutcome}; +``` + +- [ ] **Step 7: Fix in-crate `set()` callers (compile break)** + +The only in-crate consumers are tests. `hyperdb-api/tests/kv_store_tests.rs:67-69` calls `kv.set("k","v1")?;` in statement position — these still compile (the `Result` is just `?`-unwrapped and the `SetOutcome` dropped). No change needed unless a test binds the return. Confirm with the build in Step 8. + +- [ ] **Step 8: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests` +Expected: PASS (new tests green; existing `set_then_get_and_overwrite` still green). + +- [ ] **Step 9: Commit** + +```bash +git add hyperdb-api/src/kv_store.rs hyperdb-api/src/lib.rs hyperdb-api/tests/kv_store_tests.rs +git commit -m "feat(kv)!: sync set/set_as/set_batch return SetOutcome/BatchSetOutcome" +``` + +### Task 2: async twin — reshape `AsyncKvStore::set`/`set_as`/`set_batch`/`upsert` + +Mirror Task 1 in the async file. The outcome types already exist (re-exported); this only changes `async_kv_store.rs`. Note the async `set_batch` uses a **different loop shape** than sync (mutable `inner` + `break`), which must be preserved. + +**Files:** +- Modify: `hyperdb-api/src/async_kv_store.rs:100-146` (`set`, `upsert`), `:162-171` (`set_as`), `:331-358` (`set_batch`) +- Test: `hyperdb-api/tests/async_kv_store_tests.rs` + +**Interfaces:** +- Consumes: `SetOutcome`, `BatchSetOutcome` from Task 1. +- Produces: `AsyncKvStore::set(&self, &str, &str) -> Result`; `set_as(&self, &str, &T) -> Result`; `set_batch(&self, &[(&str,&str)]) -> Result`; private `upsert(&self, &str, &str) -> Result`. + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-api/tests/async_kv_store_tests.rs`: + +```rust +#[tokio::test(flavor = "current_thread")] +async fn async_set_reports_created_and_batch_outcome() -> Result<()> { + let (_hyper, conn) = fresh_async_conn("async_kv_outcome").await?; + let kv = conn.kv_store("outcome").await?; + assert!(kv.set("k", "v1").await?.created); + assert!(!kv.set("k", "v2").await?.created); + + kv.set("a", "1").await?; // pre-existing + let out = kv.set_batch(&[("a", "10"), ("b", "20")]).await?; + assert_eq!(out.created, 1); + assert_eq!(out.overwritten, 1); + Ok(()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test async_kv_store_tests async_set_reports_created` +Expected: FAIL — `no field created on ()`. + +- [ ] **Step 3: Import the outcome types** + +At the top of `hyperdb-api/src/async_kv_store.rs`, add `SetOutcome` and `BatchSetOutcome` to the existing `use crate::...` / `use super::...` import of `kv_store` items (they are defined in `kv_store.rs`; import via `use crate::kv_store::{BatchSetOutcome, SetOutcome};` if not already re-imported). + +- [ ] **Step 4: Reshape async `upsert` (return `updated == 0`)** + +Change `async_kv_store.rs:115-146` return type to `Result` and the tail `Ok(())` to `Ok(updated == 0)`: + +```rust + async fn upsert(&self, key: &str, value: &str) -> Result { + let updated = self + .connection + .command_params( + &format!( + "UPDATE {} SET value = $3 WHERE store_name = $1 AND key = $2", + self.table_ref + ), + &[&self.store_name.as_str(), &key, &value], + ) + .await?; + if updated == 0 { + self.connection + .command_params( + &format!( + "INSERT INTO {t} (store_name, key, value) \ + SELECT $1, $2, $3 \ + WHERE NOT EXISTS (SELECT 1 FROM {t} WHERE store_name = $4 AND key = $5)", + t = self.table_ref + ), + &[ + &self.store_name.as_str(), + &key, + &value, + &self.store_name.as_str(), + &key, + ], + ) + .await?; + } + Ok(updated == 0) + } +``` + +- [ ] **Step 5: Reshape async `set`, `set_as`, `set_batch`** + +`set` (`:105-108`): + +```rust + pub async fn set(&self, key: &str, value: &str) -> Result { + validate_kv_name(key, "key")?; + Ok(SetOutcome { created: self.upsert(key, value).await? }) + } +``` + +`set_as` (`:167-171`): + +```rust + pub async fn set_as(&self, key: &str, value: &T) -> Result { + validate_kv_name(key, "key")?; + let json = serde_json::to_string(value).map_err(|e| Error::serialization(e.to_string()))?; + Ok(SetOutcome { created: self.upsert(key, &json).await? }) + } +``` + +`set_batch` (`:339-358`) — preserve the mutable-`inner`+`break` shape, accumulating into the outcome: + +```rust + pub async fn set_batch(&self, entries: &[(&str, &str)]) -> Result { + for (key, _) in entries { + validate_kv_name(key, "key")?; + } + self.connection.begin_transaction_raw().await?; + let mut inner: Result = Ok(BatchSetOutcome { created: 0, overwritten: 0 }); + for (key, value) in entries { + match self.upsert(key, value).await { + Ok(true) => { + if let Ok(o) = inner.as_mut() { + o.created += 1; + } + } + Ok(false) => { + if let Ok(o) = inner.as_mut() { + o.overwritten += 1; + } + } + Err(e) => { + inner = Err(e); + break; + } + } + } + match &inner { + Ok(_) => self.connection.commit_raw().await?, + Err(_) => { + let _ = self.connection.rollback_raw().await; + } + } + inner + } +``` + +- [ ] **Step 6: Fix the existing `async_kv_full_surface` test** + +`async_kv_store_tests.rs:39,46,53` call `set`/`set_as`/`set_batch` in `?`-statement position — they still compile (return value dropped). No change needed; confirm in Step 7. + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test async_kv_store_tests` +Expected: PASS. + +- [ ] **Step 8: Commit** + +```bash +git add hyperdb-api/src/async_kv_store.rs hyperdb-api/tests/async_kv_store_tests.rs +git commit -m "feat(kv)!: async set/set_as/set_batch return SetOutcome/BatchSetOutcome" +``` + +### Task 3: sync `set_if_absent` (write guard, no race) + +Add a non-breaking write guard that inserts only if the key is absent, in a single statement (no check-then-write race). This is the API primitive behind MCP `overwrite:false` (Task 5). + +**Files:** +- Modify: `hyperdb-api/src/kv_store.rs` (add method after `set_as`, around `:253`) +- Test: `hyperdb-api/tests/kv_store_tests.rs` + +**Interfaces:** +- Produces: `KvStore::set_if_absent(&self, key: &str, value: &str) -> Result` — `true` if written, `false` if the key already existed (nothing written). +- Consumed by: Task 5 (MCP `overwrite:false`), Task 6 (async twin). + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-api/tests/kv_store_tests.rs`: + +```rust +#[test] +fn set_if_absent_guards_existing_key() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("guard")?; + assert!(kv.set_if_absent("k", "first")?, "absent key must be written"); + assert!(!kv.set_if_absent("k", "second")?, "present key must be skipped"); + assert_eq!(kv.get("k")?, Some("first".to_string()), "value must be unchanged"); + Ok(()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests set_if_absent_guards` +Expected: FAIL — `no method named set_if_absent`. + +- [ ] **Step 3: Implement `set_if_absent`** + +Add after `set_as` in `hyperdb-api/src/kv_store.rs`. It reuses the conditional-INSERT half of the upsert idiom; the row count tells us whether a row was inserted: + +```rust + /// Inserts `value` under `key` only if `key` is absent. + /// + /// Returns `true` if a row was written, `false` if the key already existed + /// (in which case nothing is written). A single `INSERT ... WHERE NOT + /// EXISTS` statement decides, so there is no check-then-write race. + /// + /// # Errors + /// + /// - [`Error::InvalidName`] if `key` is invalid. + /// - [`Error::FeatureNotSupported`] on gRPC transport. + /// - [`Error::Server`] if the `INSERT` fails. + pub fn set_if_absent(&self, key: &str, value: &str) -> Result { + validate_kv_name(key, "key")?; + let store = self.store_name.as_str(); + let inserted = self.connection.command_params( + &format!( + "INSERT INTO {t} (store_name, key, value) \ + SELECT $1, $2, $3 \ + WHERE NOT EXISTS (SELECT 1 FROM {t} WHERE store_name = $4 AND key = $5)", + t = self.table_ref + ), + &[&store, &key, &value, &store, &key], + )?; + Ok(inserted > 0) + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests set_if_absent_guards` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add hyperdb-api/src/kv_store.rs hyperdb-api/tests/kv_store_tests.rs +git commit -m "feat(kv): add sync KvStore::set_if_absent write guard" +``` + +### Task 4: sync `byte_size` + `entries` (size reporting + read-all) + +Add `byte_size()` (total value bytes, for MCP `kv_size.bytes` in Task 5) and `entries()` (key+value pairs, for MCP `kv_list values:true` in Task 10). Both are non-breaking reads. + +**Files:** +- Modify: `hyperdb-api/src/kv_store.rs` (add methods after `keys`, around `:333`) +- Test: `hyperdb-api/tests/kv_store_tests.rs` + +**Interfaces:** +- Produces: `KvStore::byte_size(&self) -> Result` (`COALESCE(SUM(OCTET_LENGTH(value)), 0)`; 0 for empty store); `KvStore::entries(&self) -> Result>` (key+value, sorted by key ascending; mirrors `keys()` streaming). +- Consumed by: Task 5 (`kv_size.bytes`), Task 10 (`kv_list values:true`), Task 6 (async twins). + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-api/tests/kv_store_tests.rs`: + +```rust +#[test] +fn byte_size_and_entries() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("sized")?; + assert_eq!(kv.byte_size()?, 0, "empty store has 0 bytes"); + kv.set("a", "hello")?; // 5 bytes + kv.set("b", "worlds")?; // 6 bytes + assert_eq!(kv.byte_size()?, 11, "sum of OCTET_LENGTH"); + assert_eq!( + kv.entries()?, + vec![ + ("a".to_string(), "hello".to_string()), + ("b".to_string(), "worlds".to_string()), + ], + "entries sorted by key with values" + ); + Ok(()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests byte_size_and_entries` +Expected: FAIL — `no method named byte_size` / `entries`. + +- [ ] **Step 3: Implement `byte_size` and `entries`** + +Add after `keys` in `hyperdb-api/src/kv_store.rs`: + +```rust + /// Returns the total byte length of all values in this store + /// (`SUM(OCTET_LENGTH(value))`). Returns 0 for an empty store; `NULL` + /// values contribute 0 via `COALESCE`. + /// + /// # Errors + /// + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub fn byte_size(&self) -> Result { + let sql = format!( + "SELECT COALESCE(SUM(OCTET_LENGTH(value)), 0) FROM {} WHERE store_name = $1", + self.table_ref + ); + Ok(self + .connection + .query_params(&sql, &[&self.store_name.as_str()])? + .scalar::()? + .unwrap_or(0)) + } + + /// Returns this store's `(key, value)` pairs, sorted by key ascending. + /// + /// Materializes the whole store — intended for small scratchpad stores. + /// + /// # Errors + /// + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub fn entries(&self) -> Result> { + let sql = format!( + "SELECT key, value FROM {} WHERE store_name = $1 ORDER BY key ASC", + self.table_ref + ); + let mut result = self + .connection + .query_params(&sql, &[&self.store_name.as_str()])?; + let mut entries = Vec::new(); + while let Some(chunk) = result.next_chunk()? { + for row in &chunk { + if let Some(k) = row.get::(0) { + entries.push((k, row.get::(1).unwrap_or_default())); + } + } + } + Ok(entries) + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests byte_size_and_entries` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add hyperdb-api/src/kv_store.rs hyperdb-api/tests/kv_store_tests.rs +git commit -m "feat(kv): add sync KvStore::byte_size and entries" +``` + +### Task 5: sync `set_batch_if_absent` (atomic batch write guard) + +The `overwrite:false` variant of `kv_set_many` (Task 10) needs an atomic, all-in-one-transaction batch that skips existing keys. Add it next to `set_batch`, built on the same validate-all-then-transaction shape but calling `set_if_absent` per entry and reporting `{written, skipped}`. + +**Files:** +- Modify: `hyperdb-api/src/kv_store.rs` (add method after `set_batch`, around `:430`) +- Test: `hyperdb-api/tests/kv_store_tests.rs` + +**Interfaces:** +- Consumes: `set_if_absent` (Task 3), `BatchSetOutcome` is NOT reused here — a distinct outcome shape is needed (`written`/`skipped`, not `created`/`overwritten`). +- Produces: `pub struct BatchGuardOutcome { pub written: usize, pub skipped: usize }` (`#[derive(Debug, Clone, Copy, PartialEq, Eq)]`); `KvStore::set_batch_if_absent(&self, entries: &[(&str,&str)]) -> Result`. +- Consumed by: Task 10 (MCP `kv_set_many` with `overwrite:false`), Task 6 (async twin). Re-exported in Task 1's lib.rs line — **note:** add `BatchGuardOutcome` to that re-export too (see Step 4). + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-api/tests/kv_store_tests.rs`: + +```rust +#[test] +fn set_batch_if_absent_skips_existing() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("batch_guard")?; + kv.set("a", "orig")?; // pre-existing → must be skipped + let out = kv.set_batch_if_absent(&[("a", "new"), ("b", "b1"), ("c", "c1")])?; + assert_eq!(out.written, 2, "b and c are new"); + assert_eq!(out.skipped, 1, "a existed"); + assert_eq!(kv.get("a")?, Some("orig".to_string()), "existing value untouched"); + assert_eq!(kv.get("b")?, Some("b1".to_string())); + Ok(()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests set_batch_if_absent_skips` +Expected: FAIL — `no method named set_batch_if_absent`. + +- [ ] **Step 3: Add `BatchGuardOutcome` and `set_batch_if_absent`** + +Add `BatchGuardOutcome` next to the other outcome types (near the top of the module, after `BatchSetOutcome` from Task 1): + +```rust +/// Outcome of a guarded batch write (`set_batch_if_absent`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BatchGuardOutcome { + /// Number of keys newly inserted. + pub written: usize, + /// Number of keys skipped because they already existed. + pub skipped: usize, +} +``` + +Add the method after `set_batch` in `hyperdb-api/src/kv_store.rs`: + +```rust + /// Inserts every absent `(key, value)` pair in one transaction, skipping + /// keys that already exist. All keys are validated before the transaction + /// opens, so an invalid key aborts the whole batch without writing anything. + /// + /// # Errors + /// + /// - [`Error::InvalidName`] if any key is invalid (checked before writing). + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub fn set_batch_if_absent(&self, entries: &[(&str, &str)]) -> Result { + for (key, _) in entries { + validate_kv_name(key, "key")?; + } + self.connection.begin_transaction_raw()?; + let result = (|| { + let mut outcome = BatchGuardOutcome { written: 0, skipped: 0 }; + for (key, value) in entries { + if self.set_if_absent(key, value)? { + outcome.written += 1; + } else { + outcome.skipped += 1; + } + } + Ok(outcome) + })(); + match &result { + Ok(_) => self.connection.commit_raw()?, + Err(_) => { + let _ = self.connection.rollback_raw(); + } + } + result + } +``` + +> Note: `set_if_absent` re-validates each key, which is redundant with the pre-loop validation here but harmless (validation is a cheap in-memory check and keeps `set_if_absent` correct when called standalone). + +- [ ] **Step 4: Extend the re-export** + +In `hyperdb-api/src/lib.rs` (the line changed in Task 1), add `BatchGuardOutcome`: + +```rust +pub use kv_store::{BatchGuardOutcome, BatchSetOutcome, KvStore, SetOutcome}; +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests set_batch_if_absent_skips` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add hyperdb-api/src/kv_store.rs hyperdb-api/src/lib.rs hyperdb-api/tests/kv_store_tests.rs +git commit -m "feat(kv): add sync KvStore::set_batch_if_absent atomic guard" +``` + +### Task 6: async twins of `set_if_absent`, `byte_size`, `entries`, `set_batch_if_absent` + +Mirror Tasks 3–5 in `async_kv_store.rs`. `BatchGuardOutcome` already exists (Task 5). The async `set_batch_if_absent` uses the same mutable-`inner`+`break` loop shape as the async `set_batch` (Task 2), NOT the sync closure form. + +**Files:** +- Modify: `hyperdb-api/src/async_kv_store.rs` (mirror the sync additions) +- Test: `hyperdb-api/tests/async_kv_store_tests.rs` + +**Interfaces:** +- Consumes: `BatchGuardOutcome` (Task 5). +- Produces: `AsyncKvStore::set_if_absent(&self, &str, &str) -> Result`; `byte_size(&self) -> Result`; `entries(&self) -> Result>`; `set_batch_if_absent(&self, &[(&str,&str)]) -> Result`. + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-api/tests/async_kv_store_tests.rs`: + +```rust +#[tokio::test(flavor = "current_thread")] +async fn async_guard_size_and_entries() -> Result<()> { + let (_hyper, conn) = fresh_async_conn("async_kv_guard").await?; + let kv = conn.kv_store("g").await?; + assert!(kv.set_if_absent("k", "first").await?); + assert!(!kv.set_if_absent("k", "second").await?); + assert_eq!(kv.get("k").await?, Some("first".to_string())); + + assert_eq!(kv.byte_size().await?, 5); // "first" + kv.set("z", "hello").await?; // 5 more + assert_eq!(kv.byte_size().await?, 10); + assert_eq!( + kv.entries().await?, + vec![ + ("k".to_string(), "first".to_string()), + ("z".to_string(), "hello".to_string()), + ] + ); + + let out = kv.set_batch_if_absent(&[("k", "x"), ("new", "n1")]).await?; + assert_eq!(out.written, 1); + assert_eq!(out.skipped, 1); + Ok(()) +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test async_kv_store_tests async_guard_size_and_entries` +Expected: FAIL — `no method named set_if_absent`. + +- [ ] **Step 3: Implement the async twins** + +Add to `impl AsyncKvStore`, mirroring the sync bodies with `.await` on each `command_params`/`query_params`. `set_if_absent`: + +```rust + pub async fn set_if_absent(&self, key: &str, value: &str) -> Result { + validate_kv_name(key, "key")?; + let store = self.store_name.as_str(); + let inserted = self + .connection + .command_params( + &format!( + "INSERT INTO {t} (store_name, key, value) \ + SELECT $1, $2, $3 \ + WHERE NOT EXISTS (SELECT 1 FROM {t} WHERE store_name = $4 AND key = $5)", + t = self.table_ref + ), + &[&store, &key, &value, &store, &key], + ) + .await?; + Ok(inserted > 0) + } +``` + +`byte_size` and `entries` (mirror sync; `entries` streams via `next_chunk().await`): + +```rust + pub async fn byte_size(&self) -> Result { + let sql = format!( + "SELECT COALESCE(SUM(OCTET_LENGTH(value)), 0) FROM {} WHERE store_name = $1", + self.table_ref + ); + Ok(self + .connection + .query_params(&sql, &[&self.store_name.as_str()]) + .await? + .scalar::() + .await? + .unwrap_or(0)) + } + + pub async fn entries(&self) -> Result> { + let sql = format!( + "SELECT key, value FROM {} WHERE store_name = $1 ORDER BY key ASC", + self.table_ref + ); + let mut result = self + .connection + .query_params(&sql, &[&self.store_name.as_str()]) + .await?; + let mut entries = Vec::new(); + while let Some(chunk) = result.next_chunk().await? { + for row in &chunk { + if let Some(k) = row.get::(0) { + entries.push((k, row.get::(1).unwrap_or_default())); + } + } + } + Ok(entries) + } +``` + +> Verify the async result API shape against the existing async `keys`/`size` methods before writing — match whether `scalar`/`next_chunk` are `.await`ed in this codebase (the sync twin is synchronous; the async `AsyncQueryResult` methods are `async`). Adjust the `.await` placement to match the neighbours exactly. + +`set_batch_if_absent` — mutable-`inner`+`break` shape (matching async `set_batch` from Task 2): + +```rust + pub async fn set_batch_if_absent(&self, entries: &[(&str, &str)]) -> Result { + for (key, _) in entries { + validate_kv_name(key, "key")?; + } + self.connection.begin_transaction_raw().await?; + let mut inner: Result = Ok(BatchGuardOutcome { written: 0, skipped: 0 }); + for (key, value) in entries { + match self.set_if_absent(key, value).await { + Ok(true) => { + if let Ok(o) = inner.as_mut() { + o.written += 1; + } + } + Ok(false) => { + if let Ok(o) = inner.as_mut() { + o.skipped += 1; + } + } + Err(e) => { + inner = Err(e); + break; + } + } + } + match &inner { + Ok(_) => self.connection.commit_raw().await?, + Err(_) => { + let _ = self.connection.rollback_raw().await; + } + } + inner + } +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test async_kv_store_tests` +Expected: PASS. + +- [ ] **Step 5: Full-crate gate + commit** + +```bash +HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api +cargo clippy -p hyperdb-api --all-targets --all-features -- -D warnings +cargo fmt -p hyperdb-api +git add hyperdb-api/src/async_kv_store.rs hyperdb-api/tests/async_kv_store_tests.rs +git commit -m "feat(kv): add async set_if_absent/byte_size/entries/set_batch_if_absent" +``` + +### Task 7: error.rs — `PermissionDenied` I/O mapping + fix misleading JSON suggestions + +Two independent fidelity fixes, both unit-testable in `error.rs` without a live `hyperd`: (a) an I/O-error → `McpError` mapper that preserves `PermissionDenied` instead of collapsing every file-read error to `FileNotFound`; (b) correct the `suggestion` on the JSON-in-TEXT errors so the LLM is told to cast (`value::json`) rather than to split its statement. + +**Files:** +- Modify: `hyperdb-mcp/src/error.rs` (add `from_io_error`; adjust the `0A000` branch; add a `42601` branch) +- Test: `hyperdb-mcp/src/error.rs` (inline `#[cfg(test)] mod tests`) + +**Interfaces:** +- Produces: `McpError::from_io_error(err: &std::io::Error, context: &str) -> McpError` — `PermissionDenied` → `ErrorCode::PermissionDenied`, `NotFound` → `ErrorCode::FileNotFound`, else `ErrorCode::InternalError`; `context` (e.g. `"value_path"`, `"load_file"`) is prefixed to the message. +- Consumed by: Task 8 (`value_path` read + `load_file` read-site). + +- [ ] **Step 1: Write the failing tests** + +Add (or extend the existing) `#[cfg(test)] mod tests` at the bottom of `hyperdb-mcp/src/error.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn io_error_preserves_permission_denied() { + let e = std::io::Error::from(std::io::ErrorKind::PermissionDenied); + assert_eq!(McpError::from_io_error(&e, "value_path").code, ErrorCode::PermissionDenied); + } + + #[test] + fn io_error_maps_not_found() { + let e = std::io::Error::from(std::io::ErrorKind::NotFound); + assert_eq!(McpError::from_io_error(&e, "value_path").code, ErrorCode::FileNotFound); + } + + #[test] + fn json_value_error_suggests_cast_not_split() { + let err = hyperdb_api::Error::server( + Some("0A000".to_string()), + "function JSON_VALUE is not implemented yet".to_string(), + ); + let mapped = McpError::from(err); + let s = mapped.suggestion.unwrap_or_default(); + assert!(s.contains("::json"), "expected a ::json cast hint, got: {s}"); + assert!(!s.to_lowercase().contains("split"), "must not suggest splitting: {s}"); + } + + #[test] + fn structured_type_error_suggests_cast() { + let err = hyperdb_api::Error::server( + Some("42601".to_string()), + "operator ->> requires a structured data type".to_string(), + ); + let s = McpError::from(err).suggestion.unwrap_or_default(); + assert!(s.contains("::json"), "expected a ::json cast hint, got: {s}"); + } + + #[test] + fn multi_statement_error_still_suggests_split() { + let err = hyperdb_api::Error::server( + Some("0A000".to_string()), + "multi-statement queries are not supported".to_string(), + ); + let s = McpError::from(err).suggestion.unwrap_or_default(); + assert!(s.to_lowercase().contains("one sql statement"), "got: {s}"); + } +} +``` + +> Confirm the exact `Error::server` constructor arity/signature against `hyperdb-api/src/error.rs` (surface map: `Error::server(sqlstate: Option, ...)` at `error.rs:306`). If it takes more args, adjust the test constructor calls to match — the assertions on `.code`/`.suggestion` are what matter. + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `cargo test -p hyperdb-mcp --lib error::tests` +Expected: FAIL — `from_io_error` missing; JSON suggestions still say "split". + +- [ ] **Step 3: Add `from_io_error`** + +Add to `impl McpError` (after `with_suggestion`): + +```rust + /// Maps a filesystem [`std::io::Error`] to an [`McpError`], preserving the + /// distinction between a missing file and a permission problem instead of + /// collapsing both to [`ErrorCode::FileNotFound`]. + #[must_use] + pub fn from_io_error(err: &std::io::Error, context: &str) -> Self { + let code = match err.kind() { + std::io::ErrorKind::PermissionDenied => ErrorCode::PermissionDenied, + std::io::ErrorKind::NotFound => ErrorCode::FileNotFound, + _ => ErrorCode::InternalError, + }; + McpError::new(code, format!("{context}: {err}")) + } +``` + +- [ ] **Step 4: Fix the `0A000` branch and add a `42601` branch** + +Replace the `"0A000" => { ... }` arm (`error.rs:146-150`) so a JSON-feature error steers toward the cast, while a genuine multi-statement error keeps the split hint. Add a `"42601"` arm for the structured-type case: + +```rust + "0A000" => { + // feature_not_supported — could be Hyper's "multi-part + // queries" OR an unimplemented function (e.g. JSON_VALUE). + let lower = err.to_string().to_lowercase(); + if lower.contains("json") { + return McpError::new(ErrorCode::SqlError, err.to_string()).with_suggestion( + "JSON_VALUE is not implemented in this engine. Cast the TEXT value to json first, then use -> / ->> / JSON_EACH, e.g. `SELECT value::json ->> 'field' FROM _hyperdb_kv_store WHERE store_name = '...'`."); + } + return McpError::new(ErrorCode::SqlError, err.to_string()).with_suggestion( + "Hyper only accepts one SQL statement per call. Split your query into separate execute/query calls — one per statement."); + } + "42601" => { + // syntax_error — includes "requires a structured data type" + // when ->/->>' is applied to raw TEXT. Only steer toward the + // JSON cast when the message actually points at that case; + // otherwise leave a generic SQL error (no misleading hint). + let lower = err.to_string().to_lowercase(); + if lower.contains("structured data type") || lower.contains("json") { + return McpError::new(ErrorCode::SqlError, err.to_string()).with_suggestion( + "The -> / ->> operators need a structured type. Cast the TEXT value to json first, e.g. `value::json ->> 'field'`."); + } + return McpError::new(ErrorCode::SqlError, err.to_string()); + } +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `cargo test -p hyperdb-mcp --lib error::tests` +Expected: PASS. + +- [ ] **Step 6: Commit** + +```bash +git add hyperdb-mcp/src/error.rs +git commit -m "fix(mcp): preserve PermissionDenied and steer JSON errors to ::json cast" +``` + +### Task 8: MCP `kv_set` — `created` signal, `overwrite` guard, `value_path`, size warning + +Overhaul the single most-used KV tool. `KvSetParams.value` becomes optional and gains `value_path` (server-side file read, exactly one of the two) and `overwrite` (default `true`; `false` → `set_if_absent`). The response reports `created`, `value_bytes`, and a soft `warning` over 1 MiB. + +**Files:** +- Modify: `hyperdb-mcp/src/server.rs` — `KvSetParams` (`:844-861`), `kv_set` handler (`:3123-3139`); add a shared `KV_SOFT_SIZE_WARN_BYTES` const + a `kv_size_warning(bytes) -> Option` helper near the KV handlers. +- Test: `hyperdb-mcp/tests/kv_tools_tests.rs` + +**Interfaces:** +- Consumes: `KvStore::set` → `SetOutcome` (Task 1), `set_if_absent` (Task 3), `McpError::from_io_error` (Task 7), `attach::validate_input_path` (existing, `attach.rs:672`). +- Produces (const/helper reused by Task 10): `const KV_SOFT_SIZE_WARN_BYTES: usize = 1_048_576;` and `fn kv_size_warning(bytes: usize) -> Option`. + +- [ ] **Step 1: Write the failing tests** + +Add to `hyperdb-mcp/tests/kv_tools_tests.rs`: + +```rust +/// kv_set reports `created` (insert vs overwrite) and `value_bytes`. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_reports_created_and_bytes() -> TestResult { + let h = TestHarness::start(false, false).await?; + let first = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "hello" })).await?; + assert_eq!(structured(&first)["created"], serde_json::json!(true)); + assert_eq!(structured(&first)["value_bytes"], serde_json::json!(5)); + + let second = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "hi" })).await?; + assert_eq!(structured(&second)["created"], serde_json::json!(false)); + h.shutdown().await +} + +/// overwrite:false skips an existing key without clobbering it. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_overwrite_false_guards() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "orig" })).await?; + let guard = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "new", "overwrite": false })).await?; + assert_eq!(structured(&guard)["stored"], serde_json::json!(false)); + assert_eq!(structured(&guard)["existed"], serde_json::json!(true)); + let got = call_tool(&h.client, "kv_get", + serde_json::json!({ "store": "s", "key": "k" })).await?; + assert_eq!(structured(&got)["value"], serde_json::json!("orig")); + h.shutdown().await +} + +/// value_path reads a file's contents; neither/both value+value_path errors. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_value_path_reads_file() -> TestResult { + let h = TestHarness::start(false, false).await?; + let dir = tempfile::TempDir::new()?; + let path = dir.path().join("payload.txt"); + std::fs::write(&path, "from-file")?; + let abs = std::fs::canonicalize(&path)?; + + let set = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "f", "value_path": abs.to_string_lossy() })).await?; + assert!(!is_error(&set), "value_path set failed: {:?}", first_text(&set)); + let got = call_tool(&h.client, "kv_get", + serde_json::json!({ "store": "s", "key": "f" })).await?; + assert_eq!(structured(&got)["value"], serde_json::json!("from-file")); + + // Neither value nor value_path → INVALID_ARGUMENT. + let neither = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "x" })).await?; + assert!(is_error(&neither)); + // Both → INVALID_ARGUMENT. + let both = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "y", "value": "v", "value_path": abs.to_string_lossy() })).await?; + assert!(is_error(&both)); + h.shutdown().await +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests kv_set_` +Expected: FAIL — no `created`/`value_bytes`/`existed` fields; `value_path` unknown field. + +- [ ] **Step 3: Reshape `KvSetParams`** + +Replace the `value` field and add `value_path` + `overwrite` in `hyperdb-mcp/src/server.rs:843-861`: + +```rust +/// Parameters for `kv_set` (write a value under store + key). +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvSetParams { + /// Namespace of the KV store. Created on first write. + pub store: String, + /// Key to write. + pub key: String, + /// Value to store. Any string, including a JSON document. Provide exactly + /// one of `value` or `value_path`. + pub value: Option, + /// Absolute path to a file whose contents become the value (read + /// server-side). Provide exactly one of `value` or `value_path`. Reads any + /// path the server process can read — no sandbox. + pub value_path: Option, + /// When false, do not overwrite an existing key: if the key already exists + /// the write is skipped and the response reports `stored:false, + /// existed:true`. Defaults to true (upsert). + pub overwrite: Option, + /// Target database alias. Omit (or pass `"local"`) to write to the + /// ephemeral primary. Pass `"persistent"` to write to the durable database + /// that survives across sessions. Other values target a user-attached + /// database (must be writable). Each database has its own isolated stores. + pub database: Option, + /// Shorthand for `database: "persistent"`. When true, the value is written + /// to the persistent database. If both `database` and `persist` are set, + /// `database` wins. + pub persist: Option, +} +``` + +- [ ] **Step 4: Add the size-warning const + helper** + +Near the top of the KV-handler region in `server.rs` (module scope, e.g. just before the `impl` block's KV tools, or with the other consts), add: + +```rust +/// Soft threshold (bytes) above which a single KV value triggers a non-fatal +/// `warning` in the write response. The write always succeeds. +const KV_SOFT_SIZE_WARN_BYTES: usize = 1_048_576; + +/// Returns a soft-size advisory when `bytes` exceeds the KV scratchpad +/// threshold, else `None`. Reports the raw byte count (no float division, to +/// stay clear of `cast_precision_loss` under the pedantic lint gate). +fn kv_size_warning(bytes: usize) -> Option { + (bytes > KV_SOFT_SIZE_WARN_BYTES).then(|| { + format!( + "value is {bytes} bytes (> {KV_SOFT_SIZE_WARN_BYTES} soft limit); the KV \ + store is for small scraps — consider load_data or a real table for large payloads" + ) + }) +} +``` + +> `kv_size_warning`/`KV_SOFT_SIZE_WARN_BYTES` are free functions/consts, not methods — place them at module scope so both `kv_set` (Task 8) and `kv_set_many` (Task 10) can call them. If the surrounding code prefers associated items, make them `impl HyperMcpServer` associated fns instead; keep the signature identical. + +- [ ] **Step 5: Rewrite the `kv_set` handler** + +Replace `kv_set` (`server.rs:3123-3139`). Resolve the value from exactly one of `value`/`value_path`, branch on `overwrite`, and build the response with `created`/`value_bytes`/optional `warning`: + +```rust + fn kv_set( + &self, + Parameters(p): Parameters, + ) -> Result { + if let Err(e) = self.check_writable("kv_set") { + return Self::err_content(e); + } + // Exactly one of value / value_path. + let value = match (p.value.as_deref(), p.value_path.as_deref()) { + (Some(v), None) => v.to_string(), + (None, Some(path)) => { + let canonical = match crate::attach::validate_input_path(path, "value_path") { + Ok(c) => c, + Err(e) => return Self::err_content(e), + }; + match std::fs::read_to_string(&canonical) { + Ok(s) => s, + Err(e) => return Self::err_content(McpError::from_io_error(&e, "value_path")), + } + } + (Some(_), Some(_)) => { + return Self::err_content(McpError::new( + ErrorCode::InvalidArgument, + "provide exactly one of `value` or `value_path`, not both", + )); + } + (None, None) => { + return Self::err_content(McpError::new( + ErrorCode::InvalidArgument, + "provide either `value` or `value_path`", + )); + } + }; + let value_bytes = value.len(); + let overwrite = p.overwrite.unwrap_or(true); + + let result = self.with_engine(|engine| { + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; + let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; + if overwrite { + kv.set(&p.key, &value) + .map(|o| (true, o.created)) + .map_err(McpError::from) + } else { + kv.set_if_absent(&p.key, &value) + .map(|written| (written, written)) + .map_err(McpError::from) + } + }); + match result { + Ok((stored, created)) => { + let mut body = json!({ + "stored": stored, + "created": created, + "store": p.store, + "key": p.key, + "value_bytes": value_bytes, + }); + if !stored { + body["existed"] = json!(true); + } + if let Some(w) = kv_size_warning(value_bytes) { + body["warning"] = json!(w); + } + Self::ok_content(body) + } + Err(e) => Self::err_content(e), + } + } +``` + +> `body["existed"] = json!(true)` and `body["warning"] = ...` require `body` to be a mutable `serde_json::Value::Object`; the `json!({...})` macro produces exactly that, and index-assign on a JSON object inserts the key. Confirm `serde_json::json` and `ErrorCode`/`McpError` are already in scope in `server.rs` (they are — used throughout the KV handlers). + +- [ ] **Step 6: Update the `kv_set` tool description** + +Update the `#[tool(description = ...)]` on `kv_set` (`server.rs:3120-3121`) to document the new behavior. Append to the existing description: + +``` + Returns {stored, created, value_bytes}; `created:false` means an existing value was overwritten. Pass overwrite=false to avoid clobbering (skips + returns stored:false, existed:true). Pass value_path= to store a file's contents server-side instead of `value` (exactly one of value/value_path; reads any server-readable path — no sandbox). +``` + +- [ ] **Step 7: Update the existing roundtrip assertion** + +`kv_set_get_roundtrip_and_overwrite` (`kv_tools_tests.rs:185`) asserts `structured(&set)["stored"] == true` — still true. No change needed; the new fields are additive. Confirm in Step 8. + +- [ ] **Step 8: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests` +Expected: PASS (new + existing). + +- [ ] **Step 9: Update the load_file read-site to use `from_io_error` (uniform fix)** + +For the uniform I/O-fidelity fix promised in the spec, route the `load_file` file-read error through the new helper. Locate the site: + +Run: `rg -n "read_to_string|ErrorKind|FileNotFound" hyperdb-mcp/src/server.rs | rg -i "load|read_to_string"` + +At the `load_file` read that maps to `FileNotFound` (surface map: `server.rs:1840-1845`), replace the `.map_err(|e| McpError::new(ErrorCode::FileNotFound, ...))` with `.map_err(|e| McpError::from_io_error(&e, "load_file"))`. If the read is via `validate_input_path` + a subsequent `read_to_string`, only the `read_to_string` mapping changes (canonicalize failures legitimately stay `FileNotFound`). If no such direct `read_to_string` exists in `load_file` (it may delegate to the engine), skip this step and note it in the commit body. + +- [ ] **Step 10: Commit** + +```bash +cargo clippy -p hyperdb-mcp --all-targets --all-features -- -D warnings +cargo fmt -p hyperdb-mcp +git add hyperdb-mcp/src/server.rs hyperdb-mcp/tests/kv_tools_tests.rs +git commit -m "feat(mcp): kv_set reports created/value_bytes, adds overwrite guard + value_path" +``` + + +### Task 9: MCP `kv_size` — add `bytes` field for total value size + +Extend the `kv_size` MCP tool response to report the total byte length of all values in the store (summed from `OCTET_LENGTH(value)`). The existing `size` field (key count) stays unchanged; `bytes` is additive. Backed by the new `KvStore::byte_size()` from Task 4. + +**Files:** +- Modify: `hyperdb-mcp/src/server.rs:3189-3204` (the `kv_size` handler in the `#[tool_router]` impl block); update the `#[tool(description=...)]` at `:3186-3187` to mention `{size, bytes}`. +- Test: `hyperdb-mcp/tests/kv_tools_tests.rs` + +**Interfaces:** +- Consumes: `KvStore::byte_size(&self) -> Result` (Task 4). +- Produces: `kv_size` response JSON `{store, size, bytes}` — `size` remains the key count, `bytes` is the sum of `OCTET_LENGTH` over all values. + +- [ ] **Step 1: Write the failing test** + +Add to `hyperdb-mcp/tests/kv_tools_tests.rs` (after the existing `kv_list_size_and_list_stores` test, around line 248): + +```rust +/// kv_size reports both key count and total value bytes. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_size_reports_bytes() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "abc" })).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "b", "value": "de" })).await?; + + let size = call_tool(&h.client, "kv_size", serde_json::json!({ "store": "s" })).await?; + assert_eq!(structured(&size)["size"], serde_json::json!(2), "two keys"); + assert_eq!(structured(&size)["bytes"], serde_json::json!(5), "3+2=5 bytes"); + h.shutdown().await +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests kv_size_reports_bytes` +Expected: FAIL — no `bytes` field in the response. + +- [ ] **Step 3: Extend the `kv_size` handler to call `byte_size()`** + +Replace the `kv_size` handler body in `hyperdb-mcp/src/server.rs:3189-3204` (the `#[tool_router] impl HyperMcpServer` block). The handler currently calls `kv.size()?` and returns `{store, size}`. Extend the `with_engine` closure to fetch both `size` and `byte_size`, then return `{store, size, bytes}`: + +```rust + fn kv_size( + &self, + Parameters(p): Parameters, + ) -> Result { + let result = self.with_engine(|engine| { + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, false)?; + let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; + let key_count = kv.size().map_err(McpError::from)?; + let value_bytes = kv.byte_size().map_err(McpError::from)?; + Ok(json!({ + "store": p.store, + "size": key_count, + "bytes": value_bytes, + })) + }); + match result { + Ok(val) => Self::ok_content(val), + Err(e) => Self::err_content(e), + } + } +``` + +- [ ] **Step 4: Update the `kv_size` tool description** + +Update the `#[tool(description = ...)]` on `kv_size` (around `server.rs:3186-3187`) to document the `bytes` field. Extend the description to say: + +``` +Returns {store, size, bytes} where `size` is the key count and `bytes` is the total `OCTET_LENGTH` of all values (0 for empty stores). +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests kv_size` +Expected: PASS (new test + existing `kv_list_size_and_list_stores` green). + +- [ ] **Step 6: Commit** + +```bash +cargo clippy -p hyperdb-mcp --all-targets --all-features -- -D warnings +cargo fmt -p hyperdb-mcp +git add hyperdb-mcp/src/server.rs hyperdb-mcp/tests/kv_tools_tests.rs +git commit -m "feat(mcp): kv_size reports total value bytes" +``` + +### Task 10: MCP `kv_set_many` — atomic batch write with outcome reporting + +Add a new MCP tool backed by the atomic `set_batch` (overwrite=true) and `set_batch_if_absent` (overwrite=false) primitives from Tasks 1/5. Batch-wide outcome (`{stored, created, overwritten}` or `{stored, created, skipped}`), total byte count, and per-entry soft-size warnings for oversized values. + +**Files:** +- Modify: `hyperdb-mcp/src/server.rs` — add `KvEntry` and `KvSetManyParams` near `KvSetParams` (`:843`), add a `kv_set_many` handler in the `#[tool_router] impl` beside `kv_set` (`:3123`) +- Test: `hyperdb-mcp/tests/kv_tools_tests.rs` + +**Interfaces:** +- Consumes: `KvStore::set_batch` → `BatchSetOutcome` (Task 1), `set_batch_if_absent` → `BatchGuardOutcome` (Task 5), `KV_SOFT_SIZE_WARN_BYTES` + `kv_size_warning(bytes)` (Task 8), `McpError` + `ErrorCode`. +- Produces: `struct KvEntry { key: String, value: String }`; `struct KvSetManyParams { store: String, entries: Vec, overwrite: Option, database: Option, persist: Option }` and the `kv_set_many` tool returning `{stored, created, overwritten/skipped, total_bytes, [warnings]}`. + +- [ ] **Step 1: Write the failing tests** + +Add to `hyperdb-mcp/tests/kv_tools_tests.rs`: + +```rust +/// kv_set_many writes all entries atomically (overwrite=true default); reports +/// {stored, created, overwritten, total_bytes}; a mixed batch counts correctly. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_many_writes_all() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "1" })).await?; + + let batch = call_tool(&h.client, "kv_set_many", serde_json::json!({ + "store": "s", + "entries": [ + { "key": "a", "value": "10" }, // overwrite + { "key": "b", "value": "20" }, // new + { "key": "c", "value": "30" }, // new + ] + })).await?; + assert!(!is_error(&batch), "kv_set_many failed: {:?}", first_text(&batch)); + assert_eq!(structured(&batch)["stored"], serde_json::json!(3)); + assert_eq!(structured(&batch)["created"], serde_json::json!(2)); + assert_eq!(structured(&batch)["overwritten"], serde_json::json!(1)); + assert_eq!(structured(&batch)["total_bytes"], serde_json::json!(6), "10+20+30 = 6 bytes"); + + let got = call_tool(&h.client, "kv_get", + serde_json::json!({ "store": "s", "key": "a" })).await?; + assert_eq!(structured(&got)["value"], serde_json::json!("10")); + h.shutdown().await +} + +/// kv_set_many with overwrite=false skips existing keys, reports {stored, created, skipped}. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_many_guard_skips_existing() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "orig" })).await?; + + let guard = call_tool(&h.client, "kv_set_many", serde_json::json!({ + "store": "s", + "entries": [ + { "key": "a", "value": "new" }, // skipped + { "key": "b", "value": "b1" }, // written + ], + "overwrite": false + })).await?; + assert!(!is_error(&guard), "kv_set_many guard failed: {:?}", first_text(&guard)); + assert_eq!(structured(&guard)["stored"], serde_json::json!(1)); + assert_eq!(structured(&guard)["created"], serde_json::json!(1)); + assert_eq!(structured(&guard)["skipped"], serde_json::json!(1)); + // total_bytes is the sum of ALL submitted entry values ("new"=3 + "b1"=2), + // an upper bound under overwrite=false: the batch-guard primitive returns + // only counts, not which keys were actually written, so total_bytes cannot + // subtract the skipped entry's bytes. + assert_eq!(structured(&guard)["total_bytes"], serde_json::json!(5), "\"new\"(3) + \"b1\"(2), all submitted"); + + let got = call_tool(&h.client, "kv_get", + serde_json::json!({ "store": "s", "key": "a" })).await?; + assert_eq!(structured(&got)["value"], serde_json::json!("orig"), "existing value untouched"); + h.shutdown().await +} + +/// kv_set_many rejects empty entries with INVALID_ARGUMENT. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_many_empty_batch_errors() -> TestResult { + let h = TestHarness::start(false, false).await?; + let empty = call_tool(&h.client, "kv_set_many", serde_json::json!({ + "store": "s", + "entries": [] + })).await?; + assert!(is_error(&empty), "empty entries must error"); + h.shutdown().await +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests kv_set_many` +Expected: FAIL — `unknown field kv_set_many`. + +- [ ] **Step 3: Add `KvEntry` and `KvSetManyParams`** + +Add near `KvSetParams` in `hyperdb-mcp/src/server.rs` (after `:861`): + +```rust +/// A single key-value pair for batch writes. +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvEntry { + /// Key to write. + pub key: String, + /// Value to store. + pub value: String, +} + +/// Parameters for `kv_set_many` (atomic batch write). +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvSetManyParams { + /// Namespace of the KV store. Created on first write. + pub store: String, + /// Key-value pairs to write atomically. All keys are validated before the + /// transaction opens, so an invalid key aborts the whole batch without + /// writing anything. Empty `entries` is an error. + pub entries: Vec, + /// When false, skip existing keys instead of overwriting them (written + /// entries report `created`, skipped ones report `skipped`). Defaults to + /// true (upsert). + pub overwrite: Option, + /// Target database alias. Omit (or pass `"local"`) to write to the + /// ephemeral primary. Pass `"persistent"` to write to the durable database + /// that survives across sessions. Other values target a user-attached + /// database (must be writable). Each database has its own isolated stores. + pub database: Option, + /// Shorthand for `database: "persistent"`. When true, the batch is written + /// to the persistent database. If both `database` and `persist` are set, + /// `database` wins. + pub persist: Option, +} +``` + +- [ ] **Step 4: Add the `kv_set_many` handler** + +Add in the `#[tool_router] impl HyperMcpServer` block, after the `kv_set` handler (after `:3139`): + +```rust + /// Atomic batch write to the KV scratchpad. + #[tool( + description = "Write multiple KV pairs atomically. All keys validated before the transaction opens, so an invalid key aborts the whole batch. Returns {stored, created, overwritten, total_bytes} when overwrite=true (default); returns {stored, created, skipped, total_bytes} when overwrite=false (guard mode — skips existing keys). Empty `entries` is an error. Omit `database` to write to the ephemeral store; pass \"persistent\" (or persist=true) or an attached alias to write elsewhere." + )] + fn kv_set_many( + &self, + Parameters(p): Parameters, + ) -> Result { + if let Err(e) = self.check_writable("kv_set_many") { + return Self::err_content(e); + } + if p.entries.is_empty() { + return Self::err_content(McpError::new( + ErrorCode::InvalidArgument, + "entries must not be empty", + )); + } + + // Build the &[(&str, &str)] slice for the API crate; collect per-entry + // warnings for oversized values. + let pairs: Vec<(&str, &str)> = p + .entries + .iter() + .map(|e| (e.key.as_str(), e.value.as_str())) + .collect(); + let total_bytes: usize = p.entries.iter().map(|e| e.value.len()).sum(); + let mut warnings: Vec = Vec::new(); + for entry in &p.entries { + if let Some(w) = kv_size_warning(entry.value.len()) { + warnings.push(json!({ "key": entry.key, "warning": w })); + } + } + + // Shape the outcome JSON *inside* each branch so the `with_engine` + // closure returns a single type (`Result`). The two + // batch primitives return different outcome structs (`BatchSetOutcome` + // vs `BatchGuardOutcome`), so a bare `if`/`else` returning both would + // not type-check — a closure, like any block, needs one return type. + // `total_bytes` and `warnings` are engine-independent (computed above), + // so they are spliced into the object after the closure returns. This + // mirrors the single-type-closure pattern used by `kv_list` (Task 11). + let overwrite = p.overwrite.unwrap_or(true); + let result = self.with_engine(|engine| { + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; + let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; + if overwrite { + let o = kv.set_batch(&pairs).map_err(McpError::from)?; + Ok(json!({ + "stored": o.created + o.overwritten, + "created": o.created, + "overwritten": o.overwritten, + })) + } else { + let o = kv.set_batch_if_absent(&pairs).map_err(McpError::from)?; + Ok(json!({ + "stored": o.written, + "created": o.written, + "skipped": o.skipped, + })) + } + }); + + match result { + Ok(mut body) => { + body["total_bytes"] = json!(total_bytes); + if !warnings.is_empty() { + body["warnings"] = json!(warnings); + } + Self::ok_content(body) + } + Err(e) => Self::err_content(e), + } + } +``` + +> The closure returns `Result` in both branches — the `?` on `set_batch`/`set_batch_if_absent` forces the `McpError` error type, and both `Ok(json!(...))` arms produce a `Value`. `total_bytes` is the sum of **all submitted** entry value bytes (see the note on `total_bytes` semantics below), spliced into the object in the `Ok` arm so it is identical across both modes. + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests` +Expected: PASS (new + existing). + +- [ ] **Step 6: Commit** + +```bash +cargo clippy -p hyperdb-mcp --all-targets --all-features -- -D warnings +cargo fmt -p hyperdb-mcp +git add hyperdb-mcp/src/server.rs hyperdb-mcp/tests/kv_tools_tests.rs +git commit -m "feat(mcp): add kv_set_many atomic batch write tool" +``` + +### Task 11: MCP `kv_list` — new `KvListParams` + `values` flag for whole-store reads + +Move `kv_list` off the shared `KvStoreParams` onto its own `KvListParams` so the new `values` flag does not leak into `kv_size`/`kv_pop`/`kv_clear` schemas. When `values:true`, call `kv.entries()` (Task 4) and return `{store, entries:[{key,value},...]}` instead of the keys-only shape. + +**Files:** +- Modify: `hyperdb-mcp/src/server.rs:863-876` (`KvStoreParams` — unchanged); add `KvListParams` after it; `:3169-3184` (`kv_list` handler); update the `kv_list` #[tool(description=...)] to document the `values` flag. +- Test: `hyperdb-mcp/tests/kv_tools_tests.rs` + +**Interfaces:** +- Consumes: `KvStore::entries()` from Task 4 (returns `Vec<(String, String)>` sorted by key). +- Produces: `struct KvListParams { store: String, values: Option, database: Option, persist: Option }`; `kv_list` handler signature changed to `Parameters`. + +- [ ] **Step 1: Write the failing tests** + +Add to `hyperdb-mcp/tests/kv_tools_tests.rs`: + +```rust +/// kv_list default (values absent/false) preserves the keys-only shape. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_list_keys_only_unchanged() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "v1" })).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "b", "value": "v2" })).await?; + let list = call_tool(&h.client, "kv_list", + serde_json::json!({ "store": "s" })).await?; + let body = structured(&list); + assert_eq!(body["store"], serde_json::json!("s")); + assert_eq!(body["count"], serde_json::json!(2)); + assert_eq!(body["keys"], serde_json::json!(["a", "b"])); + h.shutdown().await +} + +/// kv_list with values:true returns entries with both key and value. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_list_values_returns_entries() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "x", "value": "hello" })).await?; + call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "y", "value": "world" })).await?; + let list = call_tool(&h.client, "kv_list", + serde_json::json!({ "store": "s", "values": true })).await?; + let body = structured(&list); + assert_eq!(body["store"], serde_json::json!("s")); + let entries = body["entries"].as_array().expect("entries must be an array"); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0]["key"], serde_json::json!("x")); + assert_eq!(entries[0]["value"], serde_json::json!("hello")); + assert_eq!(entries[1]["key"], serde_json::json!("y")); + assert_eq!(entries[1]["value"], serde_json::json!("world")); + h.shutdown().await +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests kv_list_` +Expected: FAIL — `unknown field values` when `KvStoreParams` is used; the first test may already pass (existing shape) but the second will fail. + +- [ ] **Step 3: Add `KvListParams` after `KvStoreParams`** + +In `hyperdb-mcp/src/server.rs`, after the `KvStoreParams` definition (around `:876`), add: + +```rust +/// Parameters for `kv_list` (enumerate keys, optionally with values). +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvListParams { + /// Namespace of the KV store to list. + pub store: String, + /// When true, return the full `(key, value)` pairs as `entries`; when + /// false or omitted, return only `keys` (the default behavior). Use + /// `values:true` for whole-store reads without N×`kv_get`. + pub values: Option, + /// Target database alias. Omit (or pass `"local"`) for the ephemeral + /// primary. Pass `"persistent"` for the durable database, or a + /// user-attached alias. Each database has its own isolated stores. + pub database: Option, + /// Shorthand for `database: "persistent"`. If both `database` and + /// `persist` are set, `database` wins. + pub persist: Option, +} +``` + +- [ ] **Step 4: Confirm `kv_size`/`kv_pop`/`kv_clear` still use `KvStoreParams`** + +Search `hyperdb-mcp/src/server.rs` for the signatures of `kv_size`, `kv_pop`, and `kv_clear` (around `:3206`, `:3226`, `:3245` — exact line numbers may shift as earlier tasks land). Verify each takes `Parameters`, not `KvListParams`. These three tools MUST NOT gain the `values` field — the split is intentional. No change needed; this is a verification step. + +- [ ] **Step 5: Reshape the `kv_list` handler** + +Replace the `kv_list` handler (`server.rs:3169-3184`) to accept `KvListParams` and branch on `p.values`: + +```rust + fn kv_list( + &self, + Parameters(p): Parameters, + ) -> Result { + let with_values = p.values.unwrap_or(false); + let result = self.with_engine(|engine| { + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, false)?; + let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; + if with_values { + kv.entries().map(|pairs| (Some(pairs), None)) + } else { + kv.keys().map(|keys| (None, Some(keys))) + } + .map_err(McpError::from) + }); + match result { + Ok((Some(entries), None)) => { + let arr: Vec = entries + .into_iter() + .map(|(k, v)| json!({ "key": k, "value": v })) + .collect(); + Self::ok_content(json!({ "store": p.store, "entries": arr })) + } + Ok((None, Some(keys))) => { + Self::ok_content(json!({ "store": p.store, "count": keys.len(), "keys": keys })) + } + Ok(_) => unreachable!("exactly one of entries/keys is Some"), + Err(e) => Self::err_content(e), + } + } +``` + +> The `(Some(entries), None)` / `(None, Some(keys))` tuple shape avoids cloning or materializing both; the match arm reconstructs the correct response JSON. `Value` and `json!` are already in scope from the existing KV handlers. The keys-only response shape is preserved verbatim: `{store, count, keys}`. + +- [ ] **Step 6: Update the `kv_list` tool description** + +Locate the `#[tool(description = ...)]` attribute on `kv_list` (a few lines above `:3169`). Append to the existing description: + +``` + Pass values=true to return full (key, value) pairs as an `entries` array instead of just keys — useful for reading a whole store without N×kv_get. +``` + +- [ ] **Step 7: Run tests to verify they pass** + +Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-mcp --test kv_tools_tests` +Expected: PASS (new tests green; existing `kv_list_then_delete_and_list_again` still green). + +- [ ] **Step 8: Verify the existing `kv_list` integration test still passes** + +Confirm `kv_list_then_delete_and_list_again` (`kv_tools_tests.rs:207-222`) still passes unchanged — it calls `kv_list` with no `values` parameter, so it gets the old shape. No code change needed; this is a verification step (covered by Step 7's full-crate run). + +- [ ] **Step 9: Commit** + +```bash +cargo clippy -p hyperdb-mcp --all-targets --all-features -- -D warnings +cargo fmt -p hyperdb-mcp +git add hyperdb-mcp/src/server.rs hyperdb-mcp/tests/kv_tools_tests.rs +git commit -m "feat(mcp): add values flag to kv_list for whole-store reads" +``` + +### Task 12: Docs + readme structural test — JSON queries, `::numeric` gotcha, batch/value_path/overwrite/values flags + +This is a docs task driven by the structural test. Add `kv_set_many` to the tool-name enforcement list, then update the KV README section to document the new capabilities and the two SQL quirks LLMs hit during dogfooding. + +**Files:** +- Modify: `hyperdb-mcp/tests/readme_tests.rs:56-63` (tool-name array) +- Modify: `hyperdb-mcp/src/readme.rs:187-208` (KV section) +- Test: `hyperdb-mcp/tests/readme_tests.rs` (structural test `readme_mentions_every_tool`) + +**Interfaces:** +- Consumes: `kv_set_many` (Task 10), `value_path`/`overwrite` on `kv_set` (Task 8), `values` flag on `kv_list` (Task 11), byte reporting on `kv_set`/`kv_size` (Tasks 8/9). +- Produces: updated README text covering the new surface + JSON-query/numeric cast docs. + +- [ ] **Step 1: Add `kv_set_many` to the tool-name array** + +In `hyperdb-mcp/tests/readme_tests.rs`, add `"kv_set_many"` to the `tools` array (line 56–64). Insert it in the natural position after `kv_set`: + +```rust + let tools = [ + "query", + "query_data", + "query_file", + "execute", + "load_file", + "load_files", + "load_data", + "load_iceberg", + "describe", + "sample", + "inspect_file", + "status", + "export", + "chart", + "copy_query", + "save_query", + "delete_query", + "set_table_metadata", + "attach_database", + "detach_database", + "list_attached_databases", + "watch_directory", + "unwatch_directory", + "kv_get", + "kv_set", + "kv_set_many", + "kv_delete", + "kv_list", + "kv_list_stores", + "kv_size", + "kv_pop", + "kv_clear", + "get_readme", + ]; +``` + +- [ ] **Step 2: Run the test to confirm it fails** + +Run: `cargo test -p hyperdb-mcp --test readme_tests readme_mentions_every_tool` +Expected: FAIL — `README missing mention of kv_set_many`. + +- [ ] **Step 3: Update the KV section in readme.rs** + +Replace `hyperdb-mcp/src/readme.rs:187-208` (the entire `### Key-value store` section up to and including the paragraph after `kv_clear`, stopping just before `### Introspection`). Preserve the section header and list structure; add the new tool, flags, and SQL notes: + +```rust +### Key-value store (scratchpad) +- `kv_set` — save a variable / state / summary / JSON string under a + store + key. Returns `{stored, created, value_bytes}`. Pass + `overwrite: false` to skip writes that would clobber an existing key + (response: `{stored: false, existed: true}`). Pass + `value_path: ` to store a file's contents server-side + instead of inlining `value` (exactly one of `value` / `value_path`; + reads any path the server process can read — no sandbox). +- `kv_set_many` — atomic batch write. Pass an `entries` array of + `{key, value}` objects. All keys validated up front; an invalid key + aborts the whole batch without writing anything. `overwrite: false` + skips existing keys within the batch. Returns + `{stored, created, overwritten, total_bytes}` (or `skipped` instead + of `overwritten` under `overwrite: false`). +- `kv_get` — read a value by store + key. +- `kv_delete` — delete a key. +- `kv_list` — list keys in a store. Pass `values: true` to return + `{entries: [{key, value}, ...]}` instead of `{keys: [...]}` — reads + the whole store in one call (eliminates N×`kv_get`). +- `kv_list_stores` — list store namespaces that hold data in a database. +- `kv_size` — count keys and total value bytes in a store. Returns + `{size, bytes}`. +- `kv_pop` — destructively read-and-remove the lowest-keyed entry + (lexicographic key order, not insertion order). +- `kv_clear` — delete all keys in a store. + +Every kv_* tool takes the same optional `database` parameter as the data +tools. Omit it and the store lives in the EPHEMERAL database (lost on +restart); pass `\"persistent\"` (or `persist: true`) to persist across +restarts, or any attached alias to target that database. Each database +has its own isolated set of stores. Enrich analytical tables with KV +metadata via LEFT JOIN — always filter `kv.store_name = ''` +to avoid row multiplication, and keep the KV table in the same database +as the joined table. See the `hyper://schema/kv` resource for the join +template, and `KV store vs. a custom table` above for when to reach for +this instead of a real table. + +**Querying JSON in a KV value:** Values are TEXT. To query JSON +structure, cast first: `SELECT value::json ->> 'field' FROM +_hyperdb_kv_store WHERE store_name = '...'`. The `->` / `->>` / +`JSON_EACH(...)` operators work AFTER the `::json` cast. Applying `->` +or `->>` to raw TEXT fails with SQLSTATE 42601 ("requires a structured +data type"). `JSON_VALUE(...)` is not implemented in this engine — use +the `::json` cast instead. + +**::numeric truncation gotcha:** A bare `::numeric` cast defaults to +scale 0 and truncates decimal places. Example: `41.54178215::numeric` +→ `42`. Always specify precision and scale: `::numeric(20,10)`. +``` + +- [ ] **Step 4: Run the test to confirm it passes** + +Run: `cargo test -p hyperdb-mcp --test readme_tests readme_mentions_every_tool` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +cargo fmt -p hyperdb-mcp +git add hyperdb-mcp/src/readme.rs hyperdb-mcp/tests/readme_tests.rs +git commit -m "docs(mcp): document kv JSON queries, ::numeric gotcha, batch/value_path/overwrite/values" +``` + +### Task 13: Append per-crate changelog bullets (BREAKING + additions) + +Per AGENTS.md rule 8, release-please owns the root `CHANGELOG.md`, all `Cargo.toml` versions, and `.release-please-manifest.json` — contributors only append per-crate `## [Unreleased]` bullets. The 0.6.1 → 0.7.0 bump is carried by the `feat!:` commits from Tasks 1–2; this task documents the user-visible changes in each crate's own `CHANGELOG.md`. + +**Files:** +- Modify: `hyperdb-api/CHANGELOG.md:8-18` (`## [Unreleased]` section; create if absent) +- Modify: `hyperdb-mcp/CHANGELOG.md:8-53` (`## [Unreleased]` section — runs from the `## [Unreleased]` heading down to the blank line before `## [0.5.0]`; create if absent) + +**Interfaces:** +- Consumes: `SetOutcome`, `BatchSetOutcome`, `BatchGuardOutcome` (Tasks 1,5); `set_if_absent`, `byte_size`, `entries`, `set_batch_if_absent` (Tasks 3–6); `kv_set_many` (Task 10); `kv_set.value_path`/`overwrite`, `kv_list.values`, `kv_size.bytes` (Tasks 8,9,11); error fixes (Task 7). +- Produces: no code; append bullets to each crate's `## [Unreleased]` section. + +- [ ] **Step 1: Read both CHANGELOG files** + +Run: `rg -n "## \[Unreleased\]" hyperdb-api/CHANGELOG.md hyperdb-mcp/CHANGELOG.md` + +Expected: `hyperdb-api/CHANGELOG.md:8:## [Unreleased]`, `hyperdb-mcp/CHANGELOG.md:8:## [Unreleased]`. Both have the heading. Confirm current bullet count and style before appending. + +- [ ] **Step 2: Append to `hyperdb-api/CHANGELOG.md`** + +Replace the `## [Unreleased]` section (currently lines 8–18) with: + +```markdown +## [Unreleased] + +### Changed + +- **BREAKING:** `KvStore::set`, `KvStore::set_as`, and `KvStore::set_batch` (plus their `AsyncKvStore` twins) now return `SetOutcome` or `BatchSetOutcome` instead of `Result<()>`, reporting whether each write created a new key or overwrote an existing one. The `created` signal eliminates silent data loss when an LLM accidentally clobbers existing KV data. Pre-0.7.0 callers that ignored the `Result` (statement-position `set("k","v")?;`) still compile unchanged; callers that bound the return (`let _ = set(...)?;`) must destructure or ignore the outcome. Released as 0.7.0 under pre-1.0 semver (the minor slot is the breaking slot). + +### Added + +- `KvStore::set_if_absent` / `AsyncKvStore::set_if_absent` — guarded write that inserts only if the key is absent (no check-then-write race; single `INSERT ... WHERE NOT EXISTS`). Returns `true` if written, `false` if the key already existed (nothing written). +- `KvStore::set_batch_if_absent` / `AsyncKvStore::set_batch_if_absent` — atomic batch variant of `set_if_absent`, returning `BatchGuardOutcome { written, skipped }`. All keys are validated before the transaction opens; an invalid key aborts the whole batch. +- `KvStore::byte_size` / `AsyncKvStore::byte_size` — returns the total byte length of all values in the store (`SUM(OCTET_LENGTH(value))`); 0 for an empty store. +- `KvStore::entries` / `AsyncKvStore::entries` — returns all `(key, value)` pairs sorted by key ascending, materializing the whole store. Intended for small scratchpad stores. +- `SetOutcome`, `BatchSetOutcome`, `BatchGuardOutcome` — public outcome types re-exported from `hyperdb_api` (sync + async twins). +- Key-value store API: `Connection::kv_store` / `AsyncConnection::kv_store` returning + `KvStore` / `AsyncKvStore` handles over a fixed `_hyperdb_kv_store` table, with + `get`/`set`/`get_as`/`set_as`/`delete`/`exists`/`size`/`keys`/`pop`/`clear`/`set_batch`, + plus `kv_list_stores`. Adds the `Error::Serialization` variant. +- `Connection::kv_store_in(database, name)` / `kv_list_stores_in(database)` (plus the + `AsyncConnection` twins) to open and enumerate KV stores in a specific attached + database. The database name is identifier-escaped internally. +``` + +The existing KV bullets (lines 12–18 in the current file) are appended at the end because they were under `## [Unreleased]` even though they shipped in 0.6.0/0.6.1 (a promotion miss). Keep them as-is (they're already correct) and prepend the new BREAKING + additions above them. + +- [ ] **Step 3: Append to `hyperdb-mcp/CHANGELOG.md`** + +Replace the `## [Unreleased]` section (currently lines 8–26) with: + +```markdown +## [Unreleased] + +### Added + +- **`kv_set_many` tool** — atomic batch write accepting an array of `{key, value}` entries. Validates all keys before opening the transaction; an invalid key aborts the whole batch without writing anything. Default behavior (`overwrite` absent or `true`) reports `{stored, created, overwritten, total_bytes}`; with `overwrite: false`, existing keys are skipped (not errors) and the response reports `{stored, created: 0, skipped, total_bytes}`. Each oversized entry (> 1 MiB) adds a keyed `warning` to a `warnings` array. +- **`kv_set` — `value_path` parameter** — absolute path to a file whose contents become the value (read server-side). Provide exactly one of `value` or `value_path`; neither or both is `INVALID_ARGUMENT`. Reads any path the server process can read (same posture as `load_file` — no sandbox), with I/O errors preserved (`PermissionDenied` → `ErrorCode::PermissionDenied`, not collapsed to `FileNotFound`). +- **`kv_set` — `overwrite` parameter** (default `true`) — when `false`, skips the write if the key already exists (calls `set_if_absent` instead of `set`), returning `{stored: false, created: false, existed: true}` with the original value unchanged. Eliminates silent data-loss from accidental overwrites. +- **`kv_set` — `created` and `value_bytes` in response** — `created: true` means the key was newly inserted, `false` means an existing value was overwritten. `value_bytes` reports the UTF-8 byte length of the written value. +- **`kv_set` — soft size warning** — values exceeding 1 MiB trigger a non-fatal `warning` field in the response steering the LLM toward `load_data` or a real table for large payloads. The write always succeeds. +- **`kv_size` — `bytes` field** — response now includes `{store, size, bytes}` where `size` is the key count (unchanged) and `bytes` is `SUM(OCTET_LENGTH(value))` (0 for an empty store). +- **`kv_list` — `values` flag** (default `false`) — when `true`, the response includes `{store, entries: [{key, value}, ...]}` instead of `{store, keys: [...]}`, eliminating the N×`kv_get` read pattern for whole-store materialization. Backed by a single `SELECT key, value` streamed to completion; acceptable for small scratchpad stores. +- **Key-value scratchpad tools** — eight new MCP tools (`kv_set`, `kv_get`, + `kv_delete`, `kv_list`, `kv_list_stores`, `kv_size`, `kv_pop`, `kv_clear`) + let an LLM stash variables, state, summaries, or JSON strings under a + `store`/`key` namespace without creating a table. Each takes the same + optional `database`/`persist` routing as the data tools; **stores are + ephemeral by default** (lost on server restart) and persist only when + routed to `"persistent"` (or `persist: true`) or an attached alias. Each + database keeps its own isolated set of stores. The mutating tools + (`kv_set`, `kv_delete`, `kv_pop`, `kv_clear`) are disabled in read-only + mode; the readers (`kv_get`, `kv_list`, `kv_size`, `kv_list_stores`) + always work. +- **`hyper://schema/kv` resource** describing the `_hyperdb_kv_store` + backing table, its indexless shape, the ephemeral-vs-persistent + durability rule, per-database isolation, and the `LEFT JOIN` pattern for + enriching analytical tables with KV metadata. + +### Fixed + +- **I/O error fidelity on `value_path` and `load_file`.** File-read errors now preserve `PermissionDenied` → `ErrorCode::PermissionDenied` instead of collapsing every I/O failure to `FileNotFound`. A missing file still maps to `FileNotFound`; any other I/O error becomes a generic `InternalError` with the OS message. (Implemented via `McpError::from_io_error` in `error.rs`.) +- **Misleading JSON-error `suggestion` text corrected.** The "requires a structured data type" (`42601`) and `JSON_VALUE`-not-implemented (`0A000`) errors previously suggested splitting the statement, which was wrong; they now advise casting the TEXT value to `json` first (`value::json ->> 'field'`), the actual fix. Applies narrowly to JSON-related cases; other `42601` syntax errors carry a generic message without a misleading hint. +- **Caller-fixable argument errors now return `INVALID_ARGUMENT`, not + `INTERNAL_ERROR`.** An invalid identifier from the `hyperdb-api` layer — + such as a KV `store`/`key` containing a disallowed byte or exceeding the + 512-byte limit — previously fell through to the catch-all `INTERNAL_ERROR` + code, mislabeling a validation failure the caller can fix as a server-side + bug. These (`InvalidName`, `InvalidTableDefinition`) now map to + `INVALID_ARGUMENT`, which carries a self-correction suggestion. The + human-readable message (naming the offending byte or the length) is + unchanged. +- **README `--read-only` flag table no longer claims Hyper-format export is + disabled.** The `--read-only` row wrongly listed "Hyper-format export" among + the disabled operations, contradicting both the actual behavior (`export` + has no read-only gate — a `.hyper` export is a harmless read-only file copy + and stays allowed) and the same document's Read-Only Mode "Allowed" list. + Docs-only correction; no behavior change. +- **TCP keepalive on the `hyperd` connection.** Connections to `hyperd` now + enable TCP keepalive (60s idle, 10s interval, ~90s to declare a dead peer) + instead of relying on the OS default 2-hour idle timeout. Without it, a + long-lived idle connection that went half-open — laptop sleep, a network + blip, or a `hyperd` that vanished without sending a FIN — would make the + next query (including the `status` tool) block for up to two hours instead + of failing fast and reconnecting. This is most visible since the daemon + became resident-by-default in 0.5.0, which made long-lived idle connections + the norm. (Fixed in `hyperdb-api-core` for both the sync and async clients.) +``` + +The existing KV bullets (lines 12–26) are kept at the end because they already shipped in 0.6.0/0.6.1 and correctly describe the base surface. The new bullets prepended above them document the 0.7.0 additions. + +- [ ] **Step 4: Verify the edits** + +Run: `rg -n "Unreleased" hyperdb-api/CHANGELOG.md hyperdb-mcp/CHANGELOG.md` + +Expected: both files show `## [Unreleased]` at line 8, followed by the new bullets. Eyeball the structure (### Changed / ### Added / ### Fixed headings match [Keep a Changelog](https://keepachangelog.com/)). + +- [ ] **Step 5: Commit** + +```bash +git add hyperdb-api/CHANGELOG.md hyperdb-mcp/CHANGELOG.md +git commit -m "$(cat <<'EOF' +docs(changelog): note KV breaking changes and MCP additions for 0.7.0 + +Per-crate CHANGELOG.md bullets only — release-please owns the root +CHANGELOG.md, all Cargo.toml versions, and .release-please-manifest.json. +The 0.6.1 → 0.7.0 bump is carried by the `feat!:` commits in this branch. + +hyperdb-api: BREAKING — set/set_as/set_batch return SetOutcome/BatchSetOutcome +instead of Result<()>; added set_if_absent, set_batch_if_absent, byte_size, +entries (sync + async twins). + +hyperdb-mcp: added kv_set_many tool, value_path + overwrite on kv_set, values +flag on kv_list, byte reporting on kv_set/kv_size; fixed PermissionDenied +I/O-error mapping and misleading JSON-error suggestion (now advises ::json cast +instead of statement split). +EOF +)" +``` diff --git a/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md b/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md new file mode 100644 index 0000000..60d9421 --- /dev/null +++ b/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md @@ -0,0 +1,429 @@ +# KV Store — LLM-First MCP Ergonomics (Issue #192) — Design + +**Status:** Approved (design) +**Date:** 2026-07-10 +**Author:** Stefan Steiner (with Claude) +**Issue:** [tableau/hyper-api-rust#192](https://github.com/tableau/hyper-api-rust/issues/192) +**Base branch:** `main` @ `d2ab4fc` (KV work #182/#185/#188/#189 all merged; released in `v0.6.1`) + +## Context + +The `hyperdb-api` KV store (sync `KvStore` / async `AsyncKvStore` over a shared +`_hyperdb_kv_store` table) and its MCP tool surface (`kv_set`, `kv_get`, +`kv_list`, `kv_size`, `kv_delete`, `kv_pop`, `kv_clear`, `kv_list_stores`) +shipped in `v0.6.0`/`v0.6.1`. A subsequent Claude Code session used the KV store +as a working scratchpad and filed five friction points that make the tools +harder for an LLM to drive than they should be. This design fixes all five, plus +two adjacent LLM-ergonomics gaps surfaced during firsthand dogfooding of the +live MCP on 2026-07-10. + +The guiding goal (user's words): *"make the MCP very flexible and powerful for +LLMs — the best database MCP ever, that LLMs have few issues trying to run."* + +### The five issue items (all reproduced firsthand) + +1. **`value_path`** — no way to store a file's contents as a KV value without the + LLM first reading the whole file into its own context and pasting it back. +2. **Insert-vs-overwrite signal** — `kv_set` returns `{"stored":true}` whether it + created a new key or silently clobbered an existing one. This is a *silent + data-loss* footgun (the "sleeper"): an LLM cannot tell it just overwrote data, + and has no way to *prevent* the overwrite. +3. **Byte-size reporting** — `kv_size` returns only a key *count*, so an LLM has + no signal about how much data it is accumulating. +4. **Batch ops** — writing N keys means N `kv_set` calls; reading a whole store's + values means `kv_list` (keys only) followed by N `kv_get` calls. Chatty in + both directions. +5. **JSON-query + `::numeric` docs** — nothing LLM-facing documents how to query + JSON stored in a TEXT value (`value::json` + `->`/`->>`/`JSON_EACH`), nor the + `::numeric` scale-0 truncation gotcha. Both cost real trial-and-error. + +### Two adjacent gaps (in scope per design decision) + +- **Error-code fidelity** — every file-read I/O error collapses to + `ErrorCode::FileNotFound`, even a genuine `PermissionDenied` + (`hyperdb-mcp/src/error.rs:36`). +- **Misleading error `suggestion` text** — the "requires a structured data type" + / `JSON_VALUE`-not-implemented errors carry a `suggestion` that tells the LLM + to split its statement, which is wrong; the real fix is the `value::json` cast. + +### Verified facts this design rests on + +- `KvStore::set` / `AsyncKvStore::set` are **public and published** (re-exported + at `hyperdb-api/src/lib.rs:203,217`; present at tags `v0.6.0` and `v0.6.1`; + `hyperdb-api = "0.6.1"` is live on crates.io). Reshaping `set()`'s return type + is therefore a **genuine breaking change** — accepted deliberately (see Version + Impact). The KV store has seen little-to-no external use yet, so the breakage + cost is judged low. +- The `created`-vs-overwritten bit is **already computed and discarded**: `upsert` + captures the UPDATE rows-count at `hyperdb-api/src/kv_store.rs:203` and returns + `Ok(())`. Threading it up is free. +- `set_batch` (`kv_store.rs:412`, `async_kv_store.rs:339`) already exists, + validates-all-then-writes-in-one-transaction, and loops `upsert` per entry + (`kv_store.rs:418`) — so a per-entry `created` bool aggregates naturally into a + batch outcome. It is **not** currently surfaced as an MCP tool. +- `attach::validate_input_path(path, kind)` (`hyperdb-mcp/src/attach.rs:672`) + already enforces absolute + canonicalized + no-`..`, with the same + arbitrary-file-read posture as `load_file` (no sandbox/allowlist). Reused for + `value_path`. +- The CHANGELOG mis-files the KV API under `## [Unreleased]` even though it + shipped in `0.6.0`/`0.6.1` — a promotion miss, corrected as part of this work. + +## Goals + +- Fix all five issue items so an LLM can use the KV store with fewer round-trips + and no silent data loss. +- Give the LLM a way to *prevent* an accidental overwrite, not just observe one. +- Keep every change consistent across the sync/async twins and the MCP layer. +- Improve error fidelity and docs so the LLM makes fewer wrong guesses. + +## Non-Goals + +- **No hard cap on value size.** Size reporting is observational + a soft, + non-fatal warning. The write always succeeds. +- **No `kv_get_many` with an explicit key list.** `values:true` on `kv_list` + covers whole-store reads; a keyed multi-get is speculative until demanded. +- **No value-column schema change.** The `value` column stays unconstrained + `TEXT`. +- **No broader MCP-surface audit** beyond the KV tools and the two named error + touch-ups. + +--- + +## Version Impact + +One intentional breaking change (`set()` / `set_batch()` return types) → +workspace **0.6.1 → 0.7.0**. + +**How the bump actually happens (release-please owns it — do NOT hand-edit +versions).** This repo is driven by release-please (`release-please-config.json`, +`release-type: simple`, `bump-minor-pre-major: true`, `skip-github-release: +true`). Version numbers live behind `# x-release-please-start-version` markers in +each crate's `Cargo.toml` and are bumped by the tool via `extra-files`; the root +`CHANGELOG.md` is generated by the tool from Conventional Commits. Contributors +therefore **must not** hand-edit `Cargo.toml` versions, the workspace version, or +the root `CHANGELOG.md`. Instead: + +- Land the breaking change as a **`feat!:`** commit (or a body with a + `BREAKING CHANGE:` footer). Under `bump-minor-pre-major`, release-please turns + a breaking change on a `0.x` line into a **minor** bump — `0.6.1 → 0.7.0` — + which is exactly the intended target. +- release-please then opens a `chore(main): release 0.7.0` PR that bumps every + `Cargo.toml` marker + workspace version and regenerates the root changelog. +- Per **AGENTS.md rule 8**, contributors still hand-append bullets to each + affected crate's **own** `CHANGELOG.md` under `## [Unreleased]`; the maintainer + promotes those to a dated heading at release time. That is the only changelog + editing this work does. +- Because `skip-github-release: true`, the `vX.Y.Z` git tag and GitHub Release + are created **by hand** after the release PR merges + (`docs/GITHUB_OPERATIONS.md:228`) — this is the manual v0.7.0 step. + +Under pre-1.0 semver the minor slot is the breaking slot; downstream users on +`^0.6` will not auto-upgrade and must opt in — acceptable given negligible +external KV usage to date. + +--- + +## Design + +### 1 · Insert-vs-overwrite signal + write guard (#2) + +#### API layer (`hyperdb-api`) — breaking + +Introduce two small public outcome types (both crates, re-exported from +`hyperdb-api`): + +```rust +/// Outcome of a single KV write. +pub struct SetOutcome { + /// `true` if the key did not previously exist (insert); + /// `false` if an existing value was overwritten. + pub created: bool, +} + +/// Outcome of a batch KV write. +pub struct BatchSetOutcome { + pub created: usize, // keys newly inserted + pub overwritten: usize, // keys whose prior value was replaced +} +``` + +Signature changes (sync + async twins): + +```rust +pub fn set(&self, key: &str, value: &str) -> Result // was Result<()> +pub fn set_as(&self, key: &str, value: &T) -> Result // was Result<()> +pub fn set_batch(&self, entries: &[(&str,&str)]) -> Result // was Result<()> +``` + +Internal `upsert` changes from `Result<()>` to `Result` (returns +`created`): the UPDATE rows-count at `kv_store.rs:203` already distinguishes the +two paths — `updated == 0` means the conditional INSERT ran (created), otherwise +it was an overwrite. `set_batch` accumulates the per-entry bool into +`BatchSetOutcome`. + +**Write guard — new non-breaking method (sync + async):** + +```rust +/// Inserts `value` only if `key` is absent. Returns `true` if written, +/// `false` if the key already existed (nothing written). +pub fn set_if_absent(&self, key: &str, value: &str) -> Result +``` + +Implemented with the conditional-INSERT half of the existing upsert idiom +(`INSERT ... WHERE NOT EXISTS`), so there is **no** check-then-write race: a +single statement decides. Returns whether a row was inserted. + +#### MCP layer (`hyperdb-mcp`) + +`kv_set` response gains `created` (and `value_bytes`, see §3): + +```json +{ "stored": true, "created": true, "store": "s", "key": "k", "value_bytes": 42 } +``` + +`KvSetParams` gains `overwrite: Option` (default `true`): + +- `overwrite` absent/`true` → current behavior via `set()`, now reporting + `created`. +- `overwrite:false` → calls `set_if_absent`. If the key already existed, the + write is **skipped** and the response is: + ```json + { "stored": false, "created": false, "existed": true, "store": "s", "key": "k" } + ``` + +The tool description documents `overwrite:false` as the way to avoid clobbering +existing data. + +### 2 · `value_path` — server-side file read for `kv_set` (#1) + +`KvSetParams`: + +- `value` becomes `Option` (was `String`). +- add `value_path: Option`. +- **Exactly one** of `value` / `value_path` must be provided. Neither, or both, + is an `INVALID_ARGUMENT` error with a clear message. + +Behavior when `value_path` is given: + +1. `attach::validate_input_path(&path, "value_path")` → absolute, canonicalized, + no `..`. +2. `std::fs::read_to_string(&path)` → the value. I/O errors map through the + improved error path (§5): `PermissionDenied` → `ErrorCode::PermissionDenied`, + missing → `FileNotFound`, else a generic internal error. +3. Store the file's contents exactly as if passed via `value`; response includes + `value_bytes` and the soft-size warning (§3). + +**Security posture (documented, not silently inherited):** identical to +`load_file` — reads any path the *server process* can read; no sandbox or +allowlist. The tool description and README state this explicitly so an operator +running the MCP with broad filesystem access understands the exposure. + +### 3 · Byte-size reporting + soft warning (#3) + +#### API layer + +Add to both twins: + +```rust +/// Total bytes of all values in this store (`SUM(OCTET_LENGTH(value))`). +/// Returns 0 for an empty store. +pub fn byte_size(&self) -> Result +``` + +`OCTET_LENGTH` (not `LENGTH`) so the number reflects encoded bytes, matching the +`value_bytes` reported on writes. `NULL`-value rows contribute 0 via +`COALESCE(SUM(OCTET_LENGTH(value)), 0)`. + +#### MCP layer + +- `kv_size` response gains `bytes` (key `size` = count, unchanged): + ```json + { "store": "s", "size": 12, "bytes": 8451 } + ``` +- **Soft warning (never blocks a write):** `kv_set` and `kv_set_many` include a + non-fatal `warning` field when a single value's byte length exceeds + **1 MiB (1_048_576 bytes)**: + ```json + "warning": "value is 2.1 MiB; the KV store is intended for small scraps — + consider load_data or a real table for large payloads" + ``` + The write still succeeds; `warning` is purely advisory. `value_bytes` is + computed as `value.len()` (UTF-8 byte length) at the MCP layer. + +### 4 · Batch ops (#4) + +#### New MCP tool: `kv_set_many` + +Backed by a new `KvSetManyParams` struct: + +```rust +struct KvSetManyParams { + store: String, + entries: Vec, // KvEntry { key: String, value: String } + overwrite: Option, // default true; false → all-or-per-key skip + database: Option, + persist: Option, +} +``` + +- Wraps the existing atomic `set_batch` (validate-all-then-one-transaction — an + invalid key aborts the whole batch, writing nothing). +- `overwrite:false` → uses a batch variant built on `set_if_absent` per entry + within the one transaction; existing keys are skipped, not errors. +- Response: + ```json + { "stored": 5, "created": 3, "overwritten": 2, "total_bytes": 1234 } + ``` + (Under `overwrite:false`, `overwritten` is replaced by `skipped`.) +- `total_bytes` = sum of `value_bytes` for the entries actually written; the + soft-size warning fires per oversized entry (collected into a `warnings` array + keyed by offending key). + +#### Read-all: `values` flag on `kv_list` + +`kv_list` moves off the shared `KvStoreParams` onto its **own** `KvListParams` +(so the new flag does not leak into `kv_size`/`kv_pop`/`kv_clear` schemas): + +```rust +struct KvListParams { + store: String, + values: Option, // default false → keys only (current behavior) + database: Option, + persist: Option, +} +``` + +- `values` absent/`false` → `{ "store": "s", "keys": ["a","b"] }` (unchanged). +- `values:true` → `{ "store": "s", "entries": [{ "key":"a","value":"..." }, ...] }`, + eliminating the N×`kv_get` read pattern. Backed by the existing full-store + read path (a `SELECT key, value` streamed to completion; the store is a + scratchpad, so full materialization is acceptable and matches `keys()` + semantics today). + +`kv_size`, `kv_pop`, `kv_clear` continue to share `KvStoreParams` unchanged. + +### 5 · Docs + error polish (#5 + adjacent) + +#### LLM-facing docs — added to BOTH `hyperdb-mcp/src/readme.rs` and the `KV_SCHEMA_RESOURCE` (`hyper://schema/kv`) + +- **Querying JSON in a KV value.** Values are TEXT; to query JSON structure, + cast first: `SELECT value::json ->> 'field' FROM _hyperdb_kv_store WHERE ...`. + Call out explicitly: + - `->` / `->>` / `JSON_EACH(...)` work **after** `::json`. + - `->>` applied to raw TEXT fails with `42601` ("requires a structured data + type"). + - `JSON_VALUE(...)` is **not implemented** in this engine — use the `::json` + cast instead. +- **`::numeric` truncation gotcha.** A bare `::numeric` cast defaults to scale 0 + and truncates (`41.54178215::numeric → 42`). Always specify precision/scale: + `::numeric(20,10)`. (Reproduced live during dogfooding.) + +#### Error-code fidelity (`hyperdb-mcp/src/error.rs:36` and the `value_path`/`load_file` read paths) + +Map `std::io::ErrorKind::PermissionDenied` → `ErrorCode::PermissionDenied` +instead of collapsing every file-read error to `FileNotFound`. `NotFound` stays +`FileNotFound`; anything else maps to a generic internal error. Applies to the +new `value_path` read and the existing `load_file` path so the fix is uniform. + +#### Misleading `suggestion` text + +Where the "requires a structured data type" (`42601`) / `JSON_VALUE`-not- +implemented errors currently suggest splitting the statement, replace the +suggestion with the correct remedy: cast the TEXT value to `json` first +(`value::json ->> '...'`). Scope this narrowly to the KV/JSON case so unrelated +`42601` errors are not mis-suggested — if a precise trigger can't be isolated, +prefer a general, non-misleading suggestion over a specific wrong one. + +--- + +## Testing + +All tests run under `HYPERD_PATH=~/dev/bin/hyperd` (real `hyperd` subprocess). + +**API layer (`hyperdb-api`, sync + async):** +- `set` returns `created:true` on first write, `created:false` on overwrite. +- `set_batch` returns correct `{created, overwritten}` for a mixed batch. +- `set_if_absent` returns `true` then `false`; second call does not change the + stored value. +- `byte_size` = 0 empty; matches `SUM(OCTET_LENGTH)` after known writes; unaffected + by key count vs. value length. + +**MCP layer (`hyperdb-mcp/tests/kv_tools_tests.rs`, via the local `TestHarness`):** +- `kv_set` response carries `created` correctly across insert then overwrite. +- `kv_set` with `overwrite:false` on an existing key → `{stored:false, existed:true}`, + value unchanged. +- `kv_set` with `value_path` reads a temp file's contents; with a bad path → + correct error code; with neither/both of `value`/`value_path` → + `INVALID_ARGUMENT`. +- `value_path` on an unreadable file → `PermissionDenied` (not `FileNotFound`) + where the platform allows constructing that case. +- `kv_size` returns `bytes` matching known content. +- oversized value (> 1 MiB) → write succeeds AND `warning` present. +- `kv_set_many` writes all entries atomically; correct `{created, overwritten}`; + invalid key aborts the whole batch. +- `kv_list` default → keys only; `values:true` → `entries` with values. +- restart-durability test still passes for the new fields on a persistent store. + +**Structural / hygiene:** +- `hyperdb-mcp/tests/readme_tests.rs` — add `kv_set_many` to the asserted + tool-name list (`readme_tests.rs:56-63`) so README coverage is enforced. +- `cargo clippy --workspace --all-targets --all-features -- -D warnings` and + `cargo fmt --all --check` clean (matching CI's exact command + toolchain). + +**Changelog (per-crate `## [Unreleased]` bullets ONLY — release-please owns the +root changelog + all version numbers; see Version Impact):** +- Append to `hyperdb-api/CHANGELOG.md` under `## [Unreleased]`: `### Changed` + (BREAKING: `set`/`set_as`/`set_batch` now return `SetOutcome`/`BatchSetOutcome`) + and `### Added` (`set_if_absent`, `byte_size`, `SetOutcome`/`BatchSetOutcome`). +- Append to `hyperdb-mcp/CHANGELOG.md` under `## [Unreleased]`: `### Added` + (`kv_set_many`, `value_path`, `overwrite`, `values` on `kv_list`, byte + reporting) and `### Fixed` (PermissionDenied mapping, misleading suggestion). +- Do **not** hand-edit `Cargo.toml` versions, the workspace version, or the root + `CHANGELOG.md` — the breaking change is carried by a `feat!:` commit and + release-please performs the `0.6.1 → 0.7.0` bump. (The pre-existing + mis-filing of released KV entries under a crate's `## [Unreleased]` is a + cosmetic pre-existing issue; correct it opportunistically only if it does not + expand scope.) + +--- + +## Risks & Mitigations + +- **Breaking `set()` return type.** Mitigated by: negligible external usage, + single intentional bump to `0.7.0`, explicit BREAKING changelog note. `set_as` + is bumped in the same wave to keep the write API internally consistent. +- **`value_path` arbitrary file read.** Same posture as `load_file` (accepted + project-wide); mitigated by explicit documentation of the exposure rather than + a false sense of sandboxing. +- **`values:true` on a huge store** materializes all values in one response. + Acceptable for a scratchpad; the soft-size warnings on write already steer + large payloads toward real tables. Not adding pagination now (YAGNI) — revisit + if a large-store complaint appears. +- **Narrow-suggestion regression.** Rewriting the `42601` suggestion risks + mis-firing on unrelated structured-type errors; mitigated by scoping the new + suggestion to the JSON-cast case and preferring a general non-misleading + message if a precise trigger can't be isolated. + +--- + +## Files Touched (inventory) + +- `hyperdb-api/src/kv_store.rs` — `SetOutcome`/`BatchSetOutcome`, `set`/`set_as`/ + `set_batch`/`upsert` signatures, `set_if_absent`, `byte_size`. +- `hyperdb-api/src/async_kv_store.rs` — async twins of all the above. +- `hyperdb-api/src/lib.rs` — re-export the two new outcome types. +- `hyperdb-api/CHANGELOG.md` — append `## [Unreleased]` bullets only (no version + headings; release-please generates the root changelog + version numbers). +- `hyperdb-mcp/src/server.rs` — `KvSetParams` (`value` optional, `value_path`, + `overwrite`), `KvListParams` (new, `values`), `KvSetManyParams`/`KvEntry` + (new), `kv_set`/`kv_list`/`kv_size` handlers, new `kv_set_many` tool, size + warning + `value_bytes`. +- `hyperdb-mcp/src/error.rs` — `PermissionDenied` mapping. +- `hyperdb-mcp/src/readme.rs` — JSON-query + `::numeric` docs, `kv_set_many`, + `value_path`/`overwrite`/`values` documentation, `value_path` security note. +- `hyperdb-mcp/tests/kv_tools_tests.rs` — new coverage listed above. +- `hyperdb-mcp/tests/readme_tests.rs` — add `kv_set_many` to tool-name assertion. +- `hyperdb-mcp/CHANGELOG.md` — append `## [Unreleased]` Added/Fixed bullets only. +- **NOT touched by hand:** `Cargo.toml` versions, workspace version, root + `CHANGELOG.md`, `.release-please-manifest.json` — all owned by release-please. + The `0.6.1 → 0.7.0` bump is carried by a `feat!:` commit. From 397c20742c9555fa3e23e8a991dce34498b0183e Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:10:37 -0700 Subject: [PATCH 02/20] docs(kv): reconcile plan/spec with vet findings (#192) Apply the 6 confirmed adversarial-review findings from the plan-vet phase: - F1: fold the breaking-change MCP caller fix (server.rs:3136 Ok(())->Ok(_)) into Task 1's commit + add cargo build -p hyperdb-mcp gate and README refresh (Step 7b), so hyperdb-mcp compiles through Tasks 2-7 instead of only at Task 8. - F2: revert the silent require_writable true->false flip in the kv_size (Task 9) and kv_list (Task 11) rewrites, restoring reader parity with kv_get/kv_list_stores. - F3: correct all three Error::server test calls to the real 4-arg signature server(Option, impl Into, Option, Option). - F4: Task 13 Step 3 CHANGELOG range 8-26 -> 8-53 so the existing hyperdb-mcp ### Fixed bullets are carried into the replacement, not duplicated/orphaned. - F5: sync spec 4 total_bytes to the implemented all-submitted upper-bound semantics (BatchGuardOutcome returns counts only, not which keys were written). - F6: README refresh gap folded into Task 1 Step 7b; premise corrected. No feature-scope change; these make the plan compilable and internally consistent. --- .../plans/2026-07-11-kv-mcp-llm-ergonomics.md | 49 ++++++++++++++----- ...2026-07-10-kv-mcp-llm-ergonomics-design.md | 13 +++-- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md b/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md index 6d30745..83b2915 100644 --- a/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md +++ b/docs/superpowers/plans/2026-07-11-kv-mcp-llm-ergonomics.md @@ -218,19 +218,40 @@ to: pub use kv_store::{BatchSetOutcome, KvStore, SetOutcome}; ``` -- [ ] **Step 7: Fix in-crate `set()` callers (compile break)** +- [ ] **Step 7: Fix cross-crate `set()` callers (compile break — MUST be in this commit)** -The only in-crate consumers are tests. `hyperdb-api/tests/kv_store_tests.rs:67-69` calls `kv.set("k","v1")?;` in statement position — these still compile (the `Result` is just `?`-unwrapped and the `SetOutcome` dropped). No change needed unless a test binds the return. Confirm with the build in Step 8. +In-workspace consumers of the reshaped methods are: the two KV test files, `hyperdb-api/tests/kv_store_in_tests.rs`, the `[[example]]` bench `hyperdb-api/benches/kv_benchmark.rs`, `hyperdb-api/README.md`, and — critically — the **`hyperdb-mcp` crate**. All the API-crate callers use `set`/`set_batch` in statement position, so they still compile (the `Result` is `?`-unwrapped and the outcome dropped); the bench and `kv_store_in_tests.rs` are covered by Task 6 Step 5's `--all-targets`/`cargo test -p hyperdb-api` gate; the README refresh is folded into Step 7b below. + +**The one caller that breaks compilation is the MCP `kv_set` handler** (`hyperdb-mcp/src/server.rs:3133-3136`), which today does `kv.set(...).map_err(McpError::from)` as the closure tail and then `match result { Ok(()) => ... }`. The instant `set()` returns `Result`, `with_engine`'s inferred `R` becomes `SetOutcome` and the `Ok(())` arm is a hard type error (E0308) — breaking the whole `hyperdb-mcp` crate until Task 8. Because this is a `feat(kv)!` break, the caller fix belongs in **this** commit (an API break must leave every in-workspace caller compiling). Make the minimal one-arm edit now; Task 8 does the full handler overhaul: + +```rust + match result { + Ok(_) => Self::ok_content(json!({ "stored": true, "store": p.store, "key": p.key })), + Err(e) => Self::err_content(e), + } +``` + +(Change only `Ok(())` → `Ok(_)` on `server.rs:3136`; leave the response body as-is for now. There is no `set_batch`/`set_as`/`set_if_absent` caller in `hyperdb-mcp`, so this is the only site.) + +- [ ] **Step 7b: Refresh the README usage example** + +`hyperdb-api/README.md` (KV example, ~lines 409-417) calls `kv.set(...)` / `kv.set_as(...)` / `kv.set_batch(...)` in statement position. The blocks are NOT doctested (`lib.rs` does not `include_str!` the README), so they still compile — but they no longer showcase the new return values the whole change is about. Update the example to bind the outcome, e.g. `let outcome = kv.set("theme", "dark")?; assert!(outcome.created);` and a `set_batch` line reading `BatchSetOutcome { created, overwritten }`. This is the one consumer no gate compiles, so it must be updated by hand. - [ ] **Step 8: Run tests to verify they pass** Run: `HYPERD_PATH=~/dev/bin/hyperd cargo test -p hyperdb-api --test kv_store_tests` Expected: PASS (new tests green; existing `set_then_get_and_overwrite` still green). +Then confirm the cross-crate caller fix kept `hyperdb-mcp` compiling: + +Run: `cargo build -p hyperdb-mcp` +Expected: SUCCESS (the `Ok(_)` arm from Step 7 resolves the `SetOutcome` return; no E0308). + - [ ] **Step 9: Commit** ```bash -git add hyperdb-api/src/kv_store.rs hyperdb-api/src/lib.rs hyperdb-api/tests/kv_store_tests.rs +git add hyperdb-api/src/kv_store.rs hyperdb-api/src/lib.rs hyperdb-api/tests/kv_store_tests.rs \ + hyperdb-api/README.md hyperdb-mcp/src/server.rs git commit -m "feat(kv)!: sync set/set_as/set_batch return SetOutcome/BatchSetOutcome" ``` @@ -882,7 +903,9 @@ mod tests { fn json_value_error_suggests_cast_not_split() { let err = hyperdb_api::Error::server( Some("0A000".to_string()), - "function JSON_VALUE is not implemented yet".to_string(), + "function JSON_VALUE is not implemented yet", + None, + None, ); let mapped = McpError::from(err); let s = mapped.suggestion.unwrap_or_default(); @@ -894,7 +917,9 @@ mod tests { fn structured_type_error_suggests_cast() { let err = hyperdb_api::Error::server( Some("42601".to_string()), - "operator ->> requires a structured data type".to_string(), + "operator ->> requires a structured data type", + None, + None, ); let s = McpError::from(err).suggestion.unwrap_or_default(); assert!(s.contains("::json"), "expected a ::json cast hint, got: {s}"); @@ -904,7 +929,9 @@ mod tests { fn multi_statement_error_still_suggests_split() { let err = hyperdb_api::Error::server( Some("0A000".to_string()), - "multi-statement queries are not supported".to_string(), + "multi-statement queries are not supported", + None, + None, ); let s = McpError::from(err).suggestion.unwrap_or_default(); assert!(s.to_lowercase().contains("one sql statement"), "got: {s}"); @@ -912,7 +939,7 @@ mod tests { } ``` -> Confirm the exact `Error::server` constructor arity/signature against `hyperdb-api/src/error.rs` (surface map: `Error::server(sqlstate: Option, ...)` at `error.rs:306`). If it takes more args, adjust the test constructor calls to match — the assertions on `.code`/`.suggestion` are what matter. +> The `Error::server` constructor is the confirmed 4-arg form `server(sqlstate: Option, message: impl Into, detail: Option, hint: Option)` (`error.rs:306`) — the three calls above already match it (`&str` message via `impl Into`; `detail`/`hint` = `None`). The assertions on `.code`/`.suggestion` are what the test verifies. - [ ] **Step 2: Run tests to verify they fail** @@ -1281,7 +1308,7 @@ Replace the `kv_size` handler body in `hyperdb-mcp/src/server.rs:3189-3204` (the Parameters(p): Parameters, ) -> Result { let result = self.with_engine(|engine| { - let db = self.resolve_db(engine, p.database.as_deref(), p.persist, false)?; + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; let key_count = kv.size().map_err(McpError::from)?; let value_bytes = kv.byte_size().map_err(McpError::from)?; @@ -1649,7 +1676,7 @@ Replace the `kv_list` handler (`server.rs:3169-3184`) to accept `KvListParams` a ) -> Result { let with_values = p.values.unwrap_or(false); let result = self.with_engine(|engine| { - let db = self.resolve_db(engine, p.database.as_deref(), p.persist, false)?; + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; if with_values { kv.entries().map(|pairs| (Some(pairs), None)) @@ -1880,7 +1907,7 @@ The existing KV bullets (lines 12–18 in the current file) are appended at the - [ ] **Step 3: Append to `hyperdb-mcp/CHANGELOG.md`** -Replace the `## [Unreleased]` section (currently lines 8–26) with: +Replace the `## [Unreleased]` section (currently lines 8–53 — the entire section, running from the `## [Unreleased]` heading through the last TCP-keepalive `### Fixed` bullet, ending just before the blank line preceding `## [0.5.0]`) with: ```markdown ## [Unreleased] @@ -1940,7 +1967,7 @@ Replace the `## [Unreleased]` section (currently lines 8–26) with: the norm. (Fixed in `hyperdb-api-core` for both the sync and async clients.) ``` -The existing KV bullets (lines 12–26) are kept at the end because they already shipped in 0.6.0/0.6.1 and correctly describe the base surface. The new bullets prepended above them document the 0.7.0 additions. +This replaces the *entire* current `## [Unreleased]` section (lines 8–53), so the block above reproduces everything that must survive: the existing KV `### Added` bullets (current lines 12–26) AND all three existing `### Fixed` bullets (current lines 28–53 — caller-fixable argument errors, README `--read-only` correction, TCP keepalive). Those are kept verbatim because they already shipped/were staged and correctly describe the base surface; the new 0.7.0 bullets are prepended within each heading. Because the replacement spans the whole section (not just lines 8–26), the existing `### Fixed` block is *moved into* the new block rather than orphaned below it — replacing only 8–26 would duplicate the three Fixed bullets. - [ ] **Step 4: Verify the edits** diff --git a/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md b/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md index 60d9421..1c75614 100644 --- a/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md +++ b/docs/superpowers/specs/2026-07-10-kv-mcp-llm-ergonomics-design.md @@ -275,9 +275,16 @@ struct KvSetManyParams { { "stored": 5, "created": 3, "overwritten": 2, "total_bytes": 1234 } ``` (Under `overwrite:false`, `overwritten` is replaced by `skipped`.) -- `total_bytes` = sum of `value_bytes` for the entries actually written; the - soft-size warning fires per oversized entry (collected into a `warnings` array - keyed by offending key). +- `total_bytes` = sum of `value_bytes` over **all submitted entries** (the UTF-8 + byte length of every `value` in the request), computed at the MCP layer before + the batch runs. Under `overwrite:false` this is an **upper bound** on the bytes + actually persisted, because `set_batch_if_absent` returns only aggregate counts + (`written`/`skipped`), not which keys were written — the MCP layer cannot know + which specific values were skipped, so it reports the total of what was + submitted rather than a figure it cannot compute. Under the default + (`overwrite:true`) every submitted entry is written, so `total_bytes` is exact. + The soft-size warning fires per oversized entry (collected into a `warnings` + array keyed by offending key). #### Read-all: `values` flag on `kv_list` From 5aeab78c92c5b7f3897692da6ff4aea9943375fa Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:17:56 -0700 Subject: [PATCH 03/20] feat(kv)!: sync set/set_as/set_batch return SetOutcome/BatchSetOutcome --- hyperdb-api/README.md | 10 +++-- hyperdb-api/src/kv_store.rs | 61 +++++++++++++++++++++++------ hyperdb-api/src/lib.rs | 2 +- hyperdb-api/tests/kv_store_tests.rs | 27 +++++++++++++ hyperdb-mcp/src/server.rs | 2 +- 5 files changed, 84 insertions(+), 18 deletions(-) diff --git a/hyperdb-api/README.md b/hyperdb-api/README.md index 63e31ab..9996ad8 100644 --- a/hyperdb-api/README.md +++ b/hyperdb-api/README.md @@ -406,15 +406,19 @@ fn main() -> Result<()> { let conn = Connection::new(&hyper, "app.hyper", CreateMode::CreateIfNotExists)?; let kv = conn.kv_store("settings")?; - kv.set("theme", "dark")?; + let outcome = kv.set("theme", "dark")?; + assert!(outcome.created); assert_eq!(kv.get("theme")?, Some("dark".to_string())); // Typed values via serde (stored as JSON). - kv.set_as("retries", &3u32)?; + let outcome = kv.set_as("retries", &3u32)?; + assert!(outcome.created); assert_eq!(kv.get_as::("retries")?, Some(3)); // Write many entries atomically in one transaction. - kv.set_batch(&[("host", "localhost"), ("port", "7483")])?; + let batch_outcome = kv.set_batch(&[("host", "localhost"), ("port", "7483")])?; + assert_eq!(batch_outcome.created, 2); + assert_eq!(batch_outcome.overwritten, 0); // Other operations: exists, delete, size, keys, clear, and pop // (remove-and-return the lowest-ordered key). diff --git a/hyperdb-api/src/kv_store.rs b/hyperdb-api/src/kv_store.rs index f141c04..f9fc578 100644 --- a/hyperdb-api/src/kv_store.rs +++ b/hyperdb-api/src/kv_store.rs @@ -93,6 +93,23 @@ pub(crate) fn kv_target_prefix(database: &str) -> Result { use crate::connection::Connection; +/// Outcome of a single KV write. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SetOutcome { + /// `true` if the key did not previously exist (insert); `false` if an + /// existing value was overwritten. + pub created: bool, +} + +/// Outcome of a batch KV write. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BatchSetOutcome { + /// Number of keys newly inserted. + pub created: usize, + /// Number of keys whose prior value was replaced. + pub overwritten: usize, +} + /// A handle to one named key-value store, backed by `KV_TABLE`. /// /// Borrows its [`Connection`] for the handle's lifetime (`'conn`), matching @@ -181,24 +198,29 @@ impl<'conn> KvStore<'conn> { Ok(row.and_then(|r| r.get::(0))) } - /// Sets `key` to `value`, inserting or overwriting (upsert). + /// Sets `key` to `value`, inserting or overwriting (upsert). Returns + /// [`SetOutcome`] indicating whether the key was newly created. /// /// # Errors /// /// - [`Error::InvalidName`] if `key` is invalid. /// - [`Error::FeatureNotSupported`] on gRPC transport. /// - [`Error::Server`] if the `UPDATE`/`INSERT` fails. - pub fn set(&self, key: &str, value: &str) -> Result<()> { + pub fn set(&self, key: &str, value: &str) -> Result { validate_kv_name(key, "key")?; - self.upsert(key, value) + Ok(SetOutcome { + created: self.upsert(key, value)?, + }) } /// UPDATE-then-conditional-INSERT upsert. Assumes `key` is validated. + /// Returns `true` if the row was newly inserted (created), `false` if an + /// existing value was overwritten. /// /// Hyper has no `ON CONFLICT`; this mirrors the proven `_table_catalog` /// idiom. The conditional INSERT uses distinct placeholders (`$4`/`$5`) /// so it is unambiguous under the extended-query protocol. - fn upsert(&self, key: &str, value: &str) -> Result<()> { + fn upsert(&self, key: &str, value: &str) -> Result { let store = self.store_name.as_str(); let updated = self.connection.command_params( &format!( @@ -218,7 +240,7 @@ impl<'conn> KvStore<'conn> { &[&store, &key, &value, &store, &key], )?; } - Ok(()) + Ok(updated == 0) } /// Deserializes the JSON-encoded value for `key` into `T`. @@ -239,17 +261,20 @@ impl<'conn> KvStore<'conn> { } } - /// Serializes `value` to JSON and stores it under `key` (upsert). + /// Serializes `value` to JSON and stores it under `key` (upsert). Returns + /// [`SetOutcome`] indicating whether the key was newly created. /// /// # Errors /// /// - [`Error::InvalidName`] if `key` is invalid. /// - [`Error::Serialization`] if `value` cannot be serialized to JSON. /// - [`Error::FeatureNotSupported`] / [`Error::Server`] as for [`set`](Self::set). - pub fn set_as(&self, key: &str, value: &T) -> Result<()> { + pub fn set_as(&self, key: &str, value: &T) -> Result { validate_kv_name(key, "key")?; let json = serde_json::to_string(value).map_err(|e| Error::serialization(e.to_string()))?; - self.upsert(key, &json) + Ok(SetOutcome { + created: self.upsert(key, &json)?, + }) } /// Deletes `key`; returns `true` if a row was removed. @@ -400,7 +425,9 @@ impl<'conn> KvStore<'conn> { Ok(Some((key, value))) } - /// Upserts every `(key, value)` pair in one transaction. + /// Upserts every `(key, value)` pair in one transaction. Returns + /// [`BatchSetOutcome`] reporting how many keys were newly inserted vs. + /// overwritten. /// /// All keys are validated before the transaction opens, so an invalid key /// aborts the whole batch without writing anything. @@ -409,19 +436,27 @@ impl<'conn> KvStore<'conn> { /// /// - [`Error::InvalidName`] if any key is invalid (checked before writing). /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. - pub fn set_batch(&self, entries: &[(&str, &str)]) -> Result<()> { + pub fn set_batch(&self, entries: &[(&str, &str)]) -> Result { for (key, _) in entries { validate_kv_name(key, "key")?; } self.connection.begin_transaction_raw()?; let result = (|| { + let mut outcome = BatchSetOutcome { + created: 0, + overwritten: 0, + }; for (key, value) in entries { - self.upsert(key, value)?; + if self.upsert(key, value)? { + outcome.created += 1; + } else { + outcome.overwritten += 1; + } } - Ok(()) + Ok(outcome) })(); match &result { - Ok(()) => self.connection.commit_raw()?, + Ok(_) => self.connection.commit_raw()?, Err(_) => { let _ = self.connection.rollback_raw(); } diff --git a/hyperdb-api/src/lib.rs b/hyperdb-api/src/lib.rs index 043c86f..192df36 100644 --- a/hyperdb-api/src/lib.rs +++ b/hyperdb-api/src/lib.rs @@ -214,7 +214,7 @@ pub use prepared::PreparedStatement; pub use async_transaction::AsyncTransaction; pub use hyperdb_api_core::client::{Notice, NoticeReceiver}; pub use inserter::{ChunkSender, ColumnMapping, InsertChunk, Inserter, IntoValue, MappedInserter}; -pub use kv_store::KvStore; +pub use kv_store::{BatchSetOutcome, KvStore, SetOutcome}; pub use names::{ escape_name, escape_sql_path, escape_string_literal, DatabaseName, Name, SchemaName, TableName, }; diff --git a/hyperdb-api/tests/kv_store_tests.rs b/hyperdb-api/tests/kv_store_tests.rs index 638cdad..b647c1d 100644 --- a/hyperdb-api/tests/kv_store_tests.rs +++ b/hyperdb-api/tests/kv_store_tests.rs @@ -204,3 +204,30 @@ fn set_batch_rejects_invalid_key_before_writing() -> Result<()> { assert_eq!(kv.size()?, 0); Ok(()) } + +#[test] +fn set_reports_created_then_overwritten() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("outcome")?; + let first = kv.set("k", "v1")?; + assert!( + first.created, + "first write of a key must report created=true" + ); + let second = kv.set("k", "v2")?; + assert!(!second.created, "overwrite must report created=false"); + assert_eq!(kv.get("k")?, Some("v2".to_string())); + Ok(()) +} + +#[test] +fn set_batch_reports_created_and_overwritten() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("batch_outcome")?; + kv.set("a", "1")?; // pre-existing → will be overwritten + let out = kv.set_batch(&[("a", "10"), ("b", "20"), ("c", "30")])?; + assert_eq!(out.created, 2, "b and c are new"); + assert_eq!(out.overwritten, 1, "a existed"); + assert_eq!(kv.get("a")?, Some("10".to_string())); + Ok(()) +} diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index 86c1da4..ecbbf74 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -3133,7 +3133,7 @@ impl HyperMcpServer { kv.set(&p.key, &p.value).map_err(McpError::from) }); match result { - Ok(()) => Self::ok_content(json!({ "stored": true, "store": p.store, "key": p.key })), + Ok(_) => Self::ok_content(json!({ "stored": true, "store": p.store, "key": p.key })), Err(e) => Self::err_content(e), } } From 89a7767b80f0420691a65dcfbe27bcf49f31dd2c Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:23:45 -0700 Subject: [PATCH 04/20] =?UTF-8?q?feat(kv)!:=20async=20twin=20=E2=80=94=20r?= =?UTF-8?q?eshape=20AsyncKvStore=20set/set=5Fas/set=5Fbatch?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the sync return-type changes (SetOutcome/BatchSetOutcome) in the async twin to maintain the sync/async lockstep invariant (AGENTS.md). Changes: - AsyncKvStore::upsert now returns Result (created) - AsyncKvStore::set returns Result - AsyncKvStore::set_as returns Result - AsyncKvStore::set_batch returns Result - Import SetOutcome/BatchSetOutcome from crate::kv_store - Update existing test calls to ignore outcomes with `let _` - Add async_set_reports_created_and_batch_outcome test All async KV tests pass (5/5) and sync tests remain green (14/14). --- hyperdb-api/src/async_kv_store.rs | 54 ++++++++++++++++------- hyperdb-api/tests/async_kv_store_tests.rs | 30 +++++++++---- 2 files changed, 59 insertions(+), 25 deletions(-) diff --git a/hyperdb-api/src/async_kv_store.rs b/hyperdb-api/src/async_kv_store.rs index 0724ca4..024692f 100644 --- a/hyperdb-api/src/async_kv_store.rs +++ b/hyperdb-api/src/async_kv_store.rs @@ -5,7 +5,9 @@ use crate::async_connection::AsyncConnection; use crate::error::{Error, Result}; -use crate::kv_store::{kv_create_table_sql, kv_target_prefix, validate_kv_name, KV_TABLE}; +use crate::kv_store::{ + kv_create_table_sql, kv_target_prefix, validate_kv_name, BatchSetOutcome, SetOutcome, KV_TABLE, +}; /// A handle to one named key-value store over an [`AsyncConnection`]. /// @@ -97,22 +99,27 @@ impl<'conn> AsyncKvStore<'conn> { Ok(row.and_then(|r| r.get::(0))) } - /// Sets `key` to `value` (upsert). + /// Sets `key` to `value`, inserting or overwriting (upsert). Returns + /// [`SetOutcome`] indicating whether the key was newly created. /// /// # Errors /// /// See [`KvStore::set`](crate::KvStore::set). - pub async fn set(&self, key: &str, value: &str) -> Result<()> { + pub async fn set(&self, key: &str, value: &str) -> Result { validate_kv_name(key, "key")?; - self.upsert(key, value).await + Ok(SetOutcome { + created: self.upsert(key, value).await?, + }) } /// UPDATE-then-conditional-INSERT upsert. Assumes `key` is validated. + /// Returns `true` if the row was newly inserted (created), `false` if an + /// existing value was overwritten. /// /// Mirrors [`KvStore::upsert`](crate::KvStore); the conditional INSERT uses /// distinct placeholders (`$4`/`$5`) so it is unambiguous under the /// extended-query protocol. - async fn upsert(&self, key: &str, value: &str) -> Result<()> { + async fn upsert(&self, key: &str, value: &str) -> Result { let updated = self .connection .command_params( @@ -142,7 +149,7 @@ impl<'conn> AsyncKvStore<'conn> { ) .await?; } - Ok(()) + Ok(updated == 0) } /// Deserializes the JSON value for `key` into `T`; `None` if absent. @@ -159,15 +166,18 @@ impl<'conn> AsyncKvStore<'conn> { } } - /// Serializes `value` to JSON and stores it under `key` (upsert). + /// Serializes `value` to JSON and stores it under `key` (upsert). Returns + /// [`SetOutcome`] indicating whether the key was newly created. /// /// # Errors /// /// See [`KvStore::set_as`](crate::KvStore::set_as). - pub async fn set_as(&self, key: &str, value: &T) -> Result<()> { + pub async fn set_as(&self, key: &str, value: &T) -> Result { validate_kv_name(key, "key")?; let json = serde_json::to_string(value).map_err(|e| Error::serialization(e.to_string()))?; - self.upsert(key, &json).await + Ok(SetOutcome { + created: self.upsert(key, &json).await?, + }) } /// Deletes `key`; returns `true` if a row was removed. @@ -328,7 +338,9 @@ impl<'conn> AsyncKvStore<'conn> { Ok(Some((key, value))) } - /// Upserts every `(key, value)` pair in one transaction. + /// Upserts every `(key, value)` pair in one transaction. Returns + /// [`BatchSetOutcome`] reporting how many keys were newly inserted vs. + /// overwritten. /// /// All keys are validated before the transaction opens, so an invalid key /// aborts the whole batch without writing anything. @@ -336,20 +348,28 @@ impl<'conn> AsyncKvStore<'conn> { /// # Errors /// /// See [`KvStore::set_batch`](crate::KvStore::set_batch). - pub async fn set_batch(&self, entries: &[(&str, &str)]) -> Result<()> { + pub async fn set_batch(&self, entries: &[(&str, &str)]) -> Result { for (key, _) in entries { validate_kv_name(key, "key")?; } self.connection.begin_transaction_raw().await?; - let mut inner: Result<()> = Ok(()); - for (key, value) in entries { - if let Err(e) = self.upsert(key, value).await { - inner = Err(e); - break; + let inner: Result = async { + let mut outcome = BatchSetOutcome { + created: 0, + overwritten: 0, + }; + for (key, value) in entries { + if self.upsert(key, value).await? { + outcome.created += 1; + } else { + outcome.overwritten += 1; + } } + Ok(outcome) } + .await; match &inner { - Ok(()) => self.connection.commit_raw().await?, + Ok(_) => self.connection.commit_raw().await?, Err(_) => { let _ = self.connection.rollback_raw().await; } diff --git a/hyperdb-api/tests/async_kv_store_tests.rs b/hyperdb-api/tests/async_kv_store_tests.rs index 854bc77..470eb7d 100644 --- a/hyperdb-api/tests/async_kv_store_tests.rs +++ b/hyperdb-api/tests/async_kv_store_tests.rs @@ -35,22 +35,22 @@ async fn async_kv_full_surface() -> Result<()> { let kv = conn.kv_store("cfg").await?; assert_eq!(kv.get("missing").await?, None); - kv.set("k", "v1").await?; - kv.set("k", "v2").await?; + let _ = kv.set("k", "v1").await?; + let _ = kv.set("k", "v2").await?; assert_eq!(kv.get("k").await?, Some("v2".to_string())); let p = Profile { name: "ada".into(), level: 7, }; - kv.set_as("p", &p).await?; + let _ = kv.set_as("p", &p).await?; assert_eq!(kv.get_as::("p").await?, Some(p)); assert!(matches!( kv.get_as::("k").await, Err(Error::Serialization(_)) )); - kv.set_batch(&[("a", "1"), ("b", "2")]).await?; + let _ = kv.set_batch(&[("a", "1"), ("b", "2")]).await?; assert_eq!(kv.size().await?, 4); assert_eq!(kv.keys().await?, vec!["a", "b", "k", "p"]); assert!(kv.exists("a").await?); @@ -72,8 +72,8 @@ async fn async_kv_full_surface() -> Result<()> { async fn async_list_stores_and_validation() -> Result<()> { let (_hyper, conn) = fresh_async_conn("async_kv_list").await?; assert!(conn.kv_list_stores().await?.is_empty()); - conn.kv_store("alpha").await?.set("k", "1").await?; - conn.kv_store("beta").await?.set("k", "2").await?; + let _ = conn.kv_store("alpha").await?.set("k", "1").await?; + let _ = conn.kv_store("beta").await?.set("k", "2").await?; let mut stores = conn.kv_list_stores().await?; stores.sort(); assert_eq!(stores, vec!["alpha", "beta"]); @@ -123,9 +123,23 @@ async fn async_store_isolation() -> Result<()> { let a = conn.kv_store("alpha").await?; let b = conn.kv_store("beta").await?; // Same key in two stores resolves to each store's own value. - a.set("k", "from_alpha").await?; - b.set("k", "from_beta").await?; + let _ = a.set("k", "from_alpha").await?; + let _ = b.set("k", "from_beta").await?; assert_eq!(a.get("k").await?, Some("from_alpha".to_string())); assert_eq!(b.get("k").await?, Some("from_beta".to_string())); Ok(()) } + +#[tokio::test(flavor = "current_thread")] +async fn async_set_reports_created_and_batch_outcome() -> Result<()> { + let (_hyper, conn) = fresh_async_conn("async_kv_outcome").await?; + let kv = conn.kv_store("outcome").await?; + assert!(kv.set("k", "v1").await?.created); + assert!(!kv.set("k", "v2").await?.created); + + kv.set("a", "1").await?; // pre-existing + let out = kv.set_batch(&[("a", "10"), ("b", "20")]).await?; + assert_eq!(out.created, 1); + assert_eq!(out.overwritten, 1); + Ok(()) +} From 6af06495121ba78d1273d714c599ed8b9a8a7113 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:26:02 -0700 Subject: [PATCH 05/20] feat(kv)!: async set/set_as/set_batch return SetOutcome/BatchSetOutcome --- hyperdb-api/src/async_kv_store.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hyperdb-api/src/async_kv_store.rs b/hyperdb-api/src/async_kv_store.rs index 024692f..0dc061d 100644 --- a/hyperdb-api/src/async_kv_store.rs +++ b/hyperdb-api/src/async_kv_store.rs @@ -353,7 +353,7 @@ impl<'conn> AsyncKvStore<'conn> { validate_kv_name(key, "key")?; } self.connection.begin_transaction_raw().await?; - let inner: Result = async { + let result = async { let mut outcome = BatchSetOutcome { created: 0, overwritten: 0, @@ -368,13 +368,13 @@ impl<'conn> AsyncKvStore<'conn> { Ok(outcome) } .await; - match &inner { + match &result { Ok(_) => self.connection.commit_raw().await?, Err(_) => { let _ = self.connection.rollback_raw().await; } } - inner + result } } From 4f1ca85fabec62fb225a70e7a922f669d6053711 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:31:18 -0700 Subject: [PATCH 06/20] feat(kv): add sync KvStore::set_if_absent write guard --- hyperdb-api/src/kv_store.rs | 26 ++++++++++++++++++++++++++ hyperdb-api/tests/kv_store_tests.rs | 20 ++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/hyperdb-api/src/kv_store.rs b/hyperdb-api/src/kv_store.rs index f9fc578..2070bb7 100644 --- a/hyperdb-api/src/kv_store.rs +++ b/hyperdb-api/src/kv_store.rs @@ -277,6 +277,32 @@ impl<'conn> KvStore<'conn> { }) } + /// Inserts `value` under `key` only if `key` is absent. + /// + /// Returns `true` if a row was written, `false` if the key already existed + /// (in which case nothing is written). A single `INSERT ... WHERE NOT + /// EXISTS` statement decides, so there is no check-then-write race. + /// + /// # Errors + /// + /// - [`Error::InvalidName`] if `key` is invalid. + /// - [`Error::FeatureNotSupported`] on gRPC transport. + /// - [`Error::Server`] if the `INSERT` fails. + pub fn set_if_absent(&self, key: &str, value: &str) -> Result { + validate_kv_name(key, "key")?; + let store = self.store_name.as_str(); + let inserted = self.connection.command_params( + &format!( + "INSERT INTO {t} (store_name, key, value) \ + SELECT $1, $2, $3 \ + WHERE NOT EXISTS (SELECT 1 FROM {t} WHERE store_name = $4 AND key = $5)", + t = self.table_ref + ), + &[&store, &key, &value, &store, &key], + )?; + Ok(inserted > 0) + } + /// Deletes `key`; returns `true` if a row was removed. /// /// # Errors diff --git a/hyperdb-api/tests/kv_store_tests.rs b/hyperdb-api/tests/kv_store_tests.rs index b647c1d..d1ed7fb 100644 --- a/hyperdb-api/tests/kv_store_tests.rs +++ b/hyperdb-api/tests/kv_store_tests.rs @@ -231,3 +231,23 @@ fn set_batch_reports_created_and_overwritten() -> Result<()> { assert_eq!(kv.get("a")?, Some("10".to_string())); Ok(()) } + +#[test] +fn set_if_absent_guards_existing_key() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("guard")?; + assert!( + kv.set_if_absent("k", "first")?, + "absent key must be written" + ); + assert!( + !kv.set_if_absent("k", "second")?, + "present key must be skipped" + ); + assert_eq!( + kv.get("k")?, + Some("first".to_string()), + "value must be unchanged" + ); + Ok(()) +} From aea9ea54d261e8f76368acd83c028374e8b1b6bc Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:34:46 -0700 Subject: [PATCH 07/20] feat(kv): add sync KvStore::byte_size and entries --- hyperdb-api/src/async_kv_store.rs | 46 +++++++++++++++++++++++ hyperdb-api/src/kv_store.rs | 45 ++++++++++++++++++++++ hyperdb-api/tests/async_kv_store_tests.rs | 19 ++++++++++ hyperdb-api/tests/kv_store_tests.rs | 19 ++++++++++ 4 files changed, 129 insertions(+) diff --git a/hyperdb-api/src/async_kv_store.rs b/hyperdb-api/src/async_kv_store.rs index 0dc061d..3dfefd9 100644 --- a/hyperdb-api/src/async_kv_store.rs +++ b/hyperdb-api/src/async_kv_store.rs @@ -241,6 +241,52 @@ impl<'conn> AsyncKvStore<'conn> { .unwrap_or(0)) } + /// Returns the total byte size of all values in this store (0 if empty). + /// + /// # Errors + /// + /// See [`KvStore::byte_size`](crate::KvStore::byte_size). + pub async fn byte_size(&self) -> Result { + let sql = format!( + "SELECT COALESCE(SUM(OCTET_LENGTH(value)), 0) FROM {} WHERE store_name = $1", + self.table_ref + ); + Ok(self + .connection + .query_params(&sql, &[&self.store_name.as_str()]) + .await? + .scalar::() + .await? + .unwrap_or(0)) + } + + /// Returns this store's `(key, value)` pairs, sorted by key ascending. + /// + /// Materializes the whole store — intended for small scratchpad stores. + /// + /// # Errors + /// + /// See [`KvStore::entries`](crate::KvStore::entries). + pub async fn entries(&self) -> Result> { + let sql = format!( + "SELECT key, value FROM {} WHERE store_name = $1 ORDER BY key ASC", + self.table_ref + ); + let mut result = self + .connection + .query_params(&sql, &[&self.store_name.as_str()]) + .await?; + let mut entries = Vec::new(); + while let Some(chunk) = result.next_chunk().await? { + for row in &chunk { + if let Some(k) = row.get::(0) { + entries.push((k, row.get::(1).unwrap_or_default())); + } + } + } + Ok(entries) + } + /// Returns this store's keys, sorted ascending. /// /// # Errors diff --git a/hyperdb-api/src/kv_store.rs b/hyperdb-api/src/kv_store.rs index 2070bb7..b44cfd2 100644 --- a/hyperdb-api/src/kv_store.rs +++ b/hyperdb-api/src/kv_store.rs @@ -383,6 +383,51 @@ impl<'conn> KvStore<'conn> { Ok(keys) } + /// Returns the total byte length of all values in this store + /// (`SUM(OCTET_LENGTH(value))`). Returns 0 for an empty store; `NULL` + /// values contribute 0 via `COALESCE`. + /// + /// # Errors + /// + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub fn byte_size(&self) -> Result { + let sql = format!( + "SELECT COALESCE(SUM(OCTET_LENGTH(value)), 0) FROM {} WHERE store_name = $1", + self.table_ref + ); + Ok(self + .connection + .query_params(&sql, &[&self.store_name.as_str()])? + .scalar::()? + .unwrap_or(0)) + } + + /// Returns this store's `(key, value)` pairs, sorted by key ascending. + /// + /// Materializes the whole store — intended for small scratchpad stores. + /// + /// # Errors + /// + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub fn entries(&self) -> Result> { + let sql = format!( + "SELECT key, value FROM {} WHERE store_name = $1 ORDER BY key ASC", + self.table_ref + ); + let mut result = self + .connection + .query_params(&sql, &[&self.store_name.as_str()])?; + let mut entries = Vec::new(); + while let Some(chunk) = result.next_chunk()? { + for row in &chunk { + if let Some(k) = row.get::(0) { + entries.push((k, row.get::(1).unwrap_or_default())); + } + } + } + Ok(entries) + } + /// Deletes every key in this store; returns the number removed. /// /// The shared backing table survives; only this store's rows are removed. diff --git a/hyperdb-api/tests/async_kv_store_tests.rs b/hyperdb-api/tests/async_kv_store_tests.rs index 470eb7d..d8200bc 100644 --- a/hyperdb-api/tests/async_kv_store_tests.rs +++ b/hyperdb-api/tests/async_kv_store_tests.rs @@ -143,3 +143,22 @@ async fn async_set_reports_created_and_batch_outcome() -> Result<()> { assert_eq!(out.overwritten, 1); Ok(()) } + +#[tokio::test(flavor = "current_thread")] +async fn async_byte_size_and_entries() -> Result<()> { + let (_hyper, conn) = fresh_async_conn("async_kv_sized").await?; + let kv = conn.kv_store("sized").await?; + assert_eq!(kv.byte_size().await?, 0, "empty store has 0 bytes"); + kv.set("a", "hello").await?; // 5 bytes + kv.set("b", "worlds").await?; // 6 bytes + assert_eq!(kv.byte_size().await?, 11, "sum of OCTET_LENGTH"); + assert_eq!( + kv.entries().await?, + vec![ + ("a".to_string(), "hello".to_string()), + ("b".to_string(), "worlds".to_string()), + ], + "entries sorted by key with values" + ); + Ok(()) +} diff --git a/hyperdb-api/tests/kv_store_tests.rs b/hyperdb-api/tests/kv_store_tests.rs index d1ed7fb..826fd0b 100644 --- a/hyperdb-api/tests/kv_store_tests.rs +++ b/hyperdb-api/tests/kv_store_tests.rs @@ -251,3 +251,22 @@ fn set_if_absent_guards_existing_key() -> Result<()> { ); Ok(()) } + +#[test] +fn byte_size_and_entries() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("sized")?; + assert_eq!(kv.byte_size()?, 0, "empty store has 0 bytes"); + kv.set("a", "hello")?; // 5 bytes + kv.set("b", "worlds")?; // 6 bytes + assert_eq!(kv.byte_size()?, 11, "sum of OCTET_LENGTH"); + assert_eq!( + kv.entries()?, + vec![ + ("a".to_string(), "hello".to_string()), + ("b".to_string(), "worlds".to_string()), + ], + "entries sorted by key with values" + ); + Ok(()) +} From 4a88aa3b284c5843ec3a573417d7a35f15a93b20 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:41:34 -0700 Subject: [PATCH 08/20] feat(kv): add sync KvStore::set_batch_if_absent atomic guard --- hyperdb-api/src/kv_store.rs | 45 +++++++++++++++++++++++++++++ hyperdb-api/src/lib.rs | 2 +- hyperdb-api/tests/kv_store_tests.rs | 17 +++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/hyperdb-api/src/kv_store.rs b/hyperdb-api/src/kv_store.rs index b44cfd2..e1f6e0a 100644 --- a/hyperdb-api/src/kv_store.rs +++ b/hyperdb-api/src/kv_store.rs @@ -110,6 +110,15 @@ pub struct BatchSetOutcome { pub overwritten: usize, } +/// Outcome of a guarded batch write (`set_batch_if_absent`). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BatchGuardOutcome { + /// Number of keys newly inserted. + pub written: usize, + /// Number of keys skipped because they already existed. + pub skipped: usize, +} + /// A handle to one named key-value store, backed by `KV_TABLE`. /// /// Borrows its [`Connection`] for the handle's lifetime (`'conn`), matching @@ -534,6 +543,42 @@ impl<'conn> KvStore<'conn> { } result } + + /// Inserts every absent `(key, value)` pair in one transaction, skipping + /// keys that already exist. All keys are validated before the transaction + /// opens, so an invalid key aborts the whole batch without writing anything. + /// + /// # Errors + /// + /// - [`Error::InvalidName`] if any key is invalid (checked before writing). + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub fn set_batch_if_absent(&self, entries: &[(&str, &str)]) -> Result { + for (key, _) in entries { + validate_kv_name(key, "key")?; + } + self.connection.begin_transaction_raw()?; + let result = (|| { + let mut outcome = BatchGuardOutcome { + written: 0, + skipped: 0, + }; + for (key, value) in entries { + if self.set_if_absent(key, value)? { + outcome.written += 1; + } else { + outcome.skipped += 1; + } + } + Ok(outcome) + })(); + match &result { + Ok(_) => self.connection.commit_raw()?, + Err(_) => { + let _ = self.connection.rollback_raw(); + } + } + result + } } impl Connection { diff --git a/hyperdb-api/src/lib.rs b/hyperdb-api/src/lib.rs index 192df36..97f4e1b 100644 --- a/hyperdb-api/src/lib.rs +++ b/hyperdb-api/src/lib.rs @@ -214,7 +214,7 @@ pub use prepared::PreparedStatement; pub use async_transaction::AsyncTransaction; pub use hyperdb_api_core::client::{Notice, NoticeReceiver}; pub use inserter::{ChunkSender, ColumnMapping, InsertChunk, Inserter, IntoValue, MappedInserter}; -pub use kv_store::{BatchSetOutcome, KvStore, SetOutcome}; +pub use kv_store::{BatchGuardOutcome, BatchSetOutcome, KvStore, SetOutcome}; pub use names::{ escape_name, escape_sql_path, escape_string_literal, DatabaseName, Name, SchemaName, TableName, }; diff --git a/hyperdb-api/tests/kv_store_tests.rs b/hyperdb-api/tests/kv_store_tests.rs index 826fd0b..a8ccd4f 100644 --- a/hyperdb-api/tests/kv_store_tests.rs +++ b/hyperdb-api/tests/kv_store_tests.rs @@ -270,3 +270,20 @@ fn byte_size_and_entries() -> Result<()> { ); Ok(()) } + +#[test] +fn set_batch_if_absent_skips_existing() -> Result<()> { + let tc = TestConnection::new()?; + let kv = tc.connection.kv_store("batch_guard")?; + kv.set("a", "orig")?; // pre-existing → must be skipped + let out = kv.set_batch_if_absent(&[("a", "new"), ("b", "b1"), ("c", "c1")])?; + assert_eq!(out.written, 2, "b and c are new"); + assert_eq!(out.skipped, 1, "a existed"); + assert_eq!( + kv.get("a")?, + Some("orig".to_string()), + "existing value untouched" + ); + assert_eq!(kv.get("b")?, Some("b1".to_string())); + Ok(()) +} From d116b125a92d9819f1e07b0e4a1e094b581312bc Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:46:28 -0700 Subject: [PATCH 09/20] feat(kv): add set_if_absent and set_batch_if_absent to AsyncKvStore Restores sync/async twin lockstep (AGENTS.md rule). The sync KvStore received `set_if_absent` (iteration 3) and `set_batch_if_absent` (iteration 5), but AsyncKvStore was missing both. This adds: - `AsyncKvStore::set_if_absent` (line 195): conditional-INSERT guard (INSERT ... WHERE NOT EXISTS), mirroring the sync version. - `AsyncKvStore::set_batch_if_absent` (line 464): transaction-wrapped batch insert that skips existing keys, returning BatchGuardOutcome (written/skipped counts). Both methods use the same SQL patterns as their sync twins. All KV tests (sync + async) pass. Fixes CRITICAL-001 (sync/async drift). Closes iteration 6 (plan Task 6). --- hyperdb-api/src/async_kv_store.rs | 69 ++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/hyperdb-api/src/async_kv_store.rs b/hyperdb-api/src/async_kv_store.rs index 3dfefd9..3cee89f 100644 --- a/hyperdb-api/src/async_kv_store.rs +++ b/hyperdb-api/src/async_kv_store.rs @@ -6,7 +6,8 @@ use crate::async_connection::AsyncConnection; use crate::error::{Error, Result}; use crate::kv_store::{ - kv_create_table_sql, kv_target_prefix, validate_kv_name, BatchSetOutcome, SetOutcome, KV_TABLE, + kv_create_table_sql, kv_target_prefix, validate_kv_name, BatchGuardOutcome, BatchSetOutcome, + SetOutcome, KV_TABLE, }; /// A handle to one named key-value store over an [`AsyncConnection`]. @@ -180,6 +181,35 @@ impl<'conn> AsyncKvStore<'conn> { }) } + /// Inserts `value` under `key` only if `key` is absent. + /// + /// Returns `true` if a row was written, `false` if the key already existed + /// (in which case nothing is written). A single `INSERT ... WHERE NOT + /// EXISTS` statement decides, so there is no check-then-write race. + /// + /// # Errors + /// + /// - [`Error::InvalidName`] if `key` is invalid. + /// - [`Error::FeatureNotSupported`] on gRPC transport. + /// - [`Error::Server`] if the `INSERT` fails. + pub async fn set_if_absent(&self, key: &str, value: &str) -> Result { + validate_kv_name(key, "key")?; + let store = self.store_name.as_str(); + let inserted = self + .connection + .command_params( + &format!( + "INSERT INTO {t} (store_name, key, value) \ + SELECT $1, $2, $3 \ + WHERE NOT EXISTS (SELECT 1 FROM {t} WHERE store_name = $4 AND key = $5)", + t = self.table_ref + ), + &[&store, &key, &value, &store, &key], + ) + .await?; + Ok(inserted > 0) + } + /// Deletes `key`; returns `true` if a row was removed. /// /// # Errors @@ -422,6 +452,43 @@ impl<'conn> AsyncKvStore<'conn> { } result } + + /// Inserts every absent `(key, value)` pair in one transaction, skipping + /// keys that already exist. All keys are validated before the transaction + /// opens, so an invalid key aborts the whole batch without writing anything. + /// + /// # Errors + /// + /// - [`Error::InvalidName`] if any key is invalid (checked before writing). + /// - [`Error::FeatureNotSupported`] / [`Error::Server`]. + pub async fn set_batch_if_absent(&self, entries: &[(&str, &str)]) -> Result { + for (key, _) in entries { + validate_kv_name(key, "key")?; + } + self.connection.begin_transaction_raw().await?; + let result = async { + let mut outcome = BatchGuardOutcome { + written: 0, + skipped: 0, + }; + for (key, value) in entries { + if self.set_if_absent(key, value).await? { + outcome.written += 1; + } else { + outcome.skipped += 1; + } + } + Ok(outcome) + } + .await; + match &result { + Ok(_) => self.connection.commit_raw().await?, + Err(_) => { + let _ = self.connection.rollback_raw().await; + } + } + result + } } impl AsyncConnection { From a16681378b7e71a9ee8821aedd1f4ebdd4373160 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:50:13 -0700 Subject: [PATCH 10/20] feat(kv): add async set_if_absent/byte_size/entries/set_batch_if_absent --- hyperdb-api/src/async_kv_store.rs | 35 ++++++++++++++--------- hyperdb-api/tests/async_kv_store_tests.rs | 25 ++++++++++++++++ 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/hyperdb-api/src/async_kv_store.rs b/hyperdb-api/src/async_kv_store.rs index 3cee89f..30e7971 100644 --- a/hyperdb-api/src/async_kv_store.rs +++ b/hyperdb-api/src/async_kv_store.rs @@ -466,28 +466,35 @@ impl<'conn> AsyncKvStore<'conn> { validate_kv_name(key, "key")?; } self.connection.begin_transaction_raw().await?; - let result = async { - let mut outcome = BatchGuardOutcome { - written: 0, - skipped: 0, - }; - for (key, value) in entries { - if self.set_if_absent(key, value).await? { - outcome.written += 1; - } else { - outcome.skipped += 1; + let mut inner: Result = Ok(BatchGuardOutcome { + written: 0, + skipped: 0, + }); + for (key, value) in entries { + match self.set_if_absent(key, value).await { + Ok(true) => { + if let Ok(o) = inner.as_mut() { + o.written += 1; + } + } + Ok(false) => { + if let Ok(o) = inner.as_mut() { + o.skipped += 1; + } + } + Err(e) => { + inner = Err(e); + break; } } - Ok(outcome) } - .await; - match &result { + match &inner { Ok(_) => self.connection.commit_raw().await?, Err(_) => { let _ = self.connection.rollback_raw().await; } } - result + inner } } diff --git a/hyperdb-api/tests/async_kv_store_tests.rs b/hyperdb-api/tests/async_kv_store_tests.rs index d8200bc..c572205 100644 --- a/hyperdb-api/tests/async_kv_store_tests.rs +++ b/hyperdb-api/tests/async_kv_store_tests.rs @@ -162,3 +162,28 @@ async fn async_byte_size_and_entries() -> Result<()> { ); Ok(()) } + +#[tokio::test(flavor = "current_thread")] +async fn async_guard_size_and_entries() -> Result<()> { + let (_hyper, conn) = fresh_async_conn("async_kv_guard").await?; + let kv = conn.kv_store("g").await?; + assert!(kv.set_if_absent("k", "first").await?); + assert!(!kv.set_if_absent("k", "second").await?); + assert_eq!(kv.get("k").await?, Some("first".to_string())); + + assert_eq!(kv.byte_size().await?, 5); // "first" + kv.set("z", "hello").await?; // 5 more + assert_eq!(kv.byte_size().await?, 10); + assert_eq!( + kv.entries().await?, + vec![ + ("k".to_string(), "first".to_string()), + ("z".to_string(), "hello".to_string()), + ] + ); + + let out = kv.set_batch_if_absent(&[("k", "x"), ("new", "n1")]).await?; + assert_eq!(out.written, 1); + assert_eq!(out.skipped, 1); + Ok(()) +} From e1acbcde9cf09308acd7d805745877939fb2be9d Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 12:54:20 -0700 Subject: [PATCH 11/20] fix(mcp): preserve PermissionDenied and steer JSON errors to ::json cast --- hyperdb-mcp/src/error.rs | 103 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/hyperdb-mcp/src/error.rs b/hyperdb-mcp/src/error.rs index e483b65..84ee148 100644 --- a/hyperdb-mcp/src/error.rs +++ b/hyperdb-mcp/src/error.rs @@ -88,6 +88,19 @@ impl McpError { self.suggestion = Some(suggestion.into()); self } + + /// Maps a filesystem [`std::io::Error`] to an [`McpError`], preserving the + /// distinction between a missing file and a permission problem instead of + /// collapsing both to [`ErrorCode::FileNotFound`]. + #[must_use] + pub fn from_io_error(err: &std::io::Error, context: &str) -> Self { + let code = match err.kind() { + std::io::ErrorKind::PermissionDenied => ErrorCode::PermissionDenied, + std::io::ErrorKind::NotFound => ErrorCode::FileNotFound, + _ => ErrorCode::InternalError, + }; + McpError::new(code, format!("{context}: {err}")) + } } impl std::fmt::Display for McpError { @@ -144,10 +157,28 @@ impl From for McpError { "A value could not be parsed into its column type. Retry with a partial schema override forcing TEXT for the offending column, e.g. schema: {\"Id\": \"TEXT\"}, and cast in SQL as needed."); } "0A000" => { - // feature_not_supported — Hyper's "Multi-part queries" + // feature_not_supported — could be Hyper's "multi-part + // queries" OR an unimplemented function (e.g. JSON_VALUE). + let lower = err.to_string().to_lowercase(); + if lower.contains("json") { + return McpError::new(ErrorCode::SqlError, err.to_string()).with_suggestion( + "JSON_VALUE is not implemented in this engine. Cast the TEXT value to json first, then use -> / ->> / JSON_EACH, e.g. `SELECT value::json ->> 'field' FROM _hyperdb_kv_store WHERE store_name = '...'`."); + } return McpError::new(ErrorCode::SqlError, err.to_string()).with_suggestion( "Hyper only accepts one SQL statement per call. Split your query into separate execute/query calls — one per statement."); } + "42601" => { + // syntax_error — includes "requires a structured data type" + // when ->/->>' is applied to raw TEXT. Only steer toward the + // JSON cast when the message actually points at that case; + // otherwise leave a generic SQL error (no misleading hint). + let lower = err.to_string().to_lowercase(); + if lower.contains("structured data type") || lower.contains("json") { + return McpError::new(ErrorCode::SqlError, err.to_string()).with_suggestion( + "The -> / ->> operators need a structured type. Cast the TEXT value to json first, e.g. `value::json ->> 'field'`."); + } + return McpError::new(ErrorCode::SqlError, err.to_string()); + } _ => {} // fall through to message-based classification } } @@ -288,3 +319,73 @@ fn is_resource_busy(msg: &str) -> bool { || lower.contains("already in use") || lower.contains("file is locked") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn io_error_preserves_permission_denied() { + let e = std::io::Error::from(std::io::ErrorKind::PermissionDenied); + assert_eq!( + McpError::from_io_error(&e, "value_path").code, + ErrorCode::PermissionDenied + ); + } + + #[test] + fn io_error_maps_not_found() { + let e = std::io::Error::from(std::io::ErrorKind::NotFound); + assert_eq!( + McpError::from_io_error(&e, "value_path").code, + ErrorCode::FileNotFound + ); + } + + #[test] + fn json_value_error_suggests_cast_not_split() { + let err = hyperdb_api::Error::server( + Some("0A000".to_string()), + "function JSON_VALUE is not implemented yet", + None, + None, + ); + let mapped = McpError::from(err); + let s = mapped.suggestion.unwrap_or_default(); + assert!( + s.contains("::json"), + "expected a ::json cast hint, got: {s}" + ); + assert!( + !s.to_lowercase().contains("split"), + "must not suggest splitting: {s}" + ); + } + + #[test] + fn structured_type_error_suggests_cast() { + let err = hyperdb_api::Error::server( + Some("42601".to_string()), + "operator ->> requires a structured data type", + None, + None, + ); + let s = McpError::from(err).suggestion.unwrap_or_default(); + assert!( + s.contains("::json"), + "expected a ::json cast hint, got: {s}" + ); + } + + #[test] + fn multi_statement_error_still_suggests_split() { + let err = hyperdb_api::Error::server( + Some("0A000".to_string()), + "multi-statement queries are not supported", + None, + None, + ); + let s = McpError::from(err).suggestion.unwrap_or_default(); + assert!(s.to_lowercase().contains("one sql statement"), "got: {s}"); + } +} From fd89084afdbfb3ffac41ae4ee255cc2312c9c557 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:00:20 -0700 Subject: [PATCH 12/20] feat(mcp): kv_set reports created/value_bytes, adds overwrite guard + value_path --- hyperdb-mcp/src/server.rs | 114 ++++++++++++++++++++++------ hyperdb-mcp/tests/kv_tools_tests.rs | 94 +++++++++++++++++++++++ 2 files changed, 184 insertions(+), 24 deletions(-) diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index ecbbf74..eeb8f20 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -845,10 +845,19 @@ pub struct KvKeyParams { pub struct KvSetParams { /// Namespace of the KV store. Created on first write. pub store: String, - /// Key to write. Overwrites any existing value for this key (upsert). + /// Key to write. pub key: String, - /// Value to store. Any string, including a JSON document. - pub value: String, + /// Value to store. Any string, including a JSON document. Provide exactly + /// one of `value` or `value_path`. + pub value: Option, + /// Absolute path to a file whose contents become the value (read + /// server-side). Provide exactly one of `value` or `value_path`. Reads any + /// path the server process can read — no sandbox. + pub value_path: Option, + /// When false, do not overwrite an existing key: if the key already exists + /// the write is skipped and the response reports `stored:false, + /// existed:true`. Defaults to true (upsert). + pub overwrite: Option, /// Target database alias. Omit (or pass `"local"`) to write to the /// ephemeral primary. Pass `"persistent"` to write to the durable database /// that survives across sessions. Other values target a user-attached @@ -1177,6 +1186,23 @@ impl HyperMcpServer { Ok(Some(resolved)) } + /// Soft threshold (bytes) above which a single KV value triggers a non-fatal + /// `warning` in the write response. The write always succeeds. + const KV_SOFT_SIZE_WARN_BYTES: usize = 1_048_576; + + /// Returns a soft-size advisory when `bytes` exceeds the KV scratchpad + /// threshold, else `None`. Reports the raw byte count (no float division, to + /// stay clear of `cast_precision_loss` under the pedantic lint gate). + fn kv_size_warning(bytes: usize) -> Option { + (bytes > Self::KV_SOFT_SIZE_WARN_BYTES).then(|| { + format!( + "value is {bytes} bytes (> {} soft limit); the KV \ + store is for small scraps — consider load_data or a real table for large payloads", + Self::KV_SOFT_SIZE_WARN_BYTES + ) + }) + } + /// Opens a KV store handle on the engine's connection, targeting the /// resolved `database` (`None` = the ephemeral primary's default location, /// `Some(alias)` = the persistent DB or an attached alias). @@ -1667,12 +1693,8 @@ impl HyperMcpServer { }; let ingest_result = if let Some(ref json_path) = params.json_extract_path { - let raw = std::fs::read_to_string(¶ms.path).map_err(|e| { - McpError::new( - ErrorCode::FileNotFound, - format!("Cannot read file '{}': {e}", params.path), - ) - })?; + let raw = std::fs::read_to_string(¶ms.path) + .map_err(|e| McpError::from_io_error(&e, "load_file"))?; let extracted = crate::ingest::extract_json_path(&raw, json_path)?; let array_text = crate::ingest::normalize_json_or_jsonl(&extracted)?; let mut result = ingest_json(engine, &array_text, &opts)?; @@ -1837,12 +1859,8 @@ impl HyperMcpServer { }; let ingest_result = if let Some(ref json_path) = params.json_extract_path { - let raw = std::fs::read_to_string(¶ms.path).map_err(|e| { - McpError::new( - ErrorCode::FileNotFound, - format!("Cannot read file '{}': {e}", params.path), - ) - })?; + let raw = std::fs::read_to_string(¶ms.path) + .map_err(|e| McpError::from_io_error(&e, "load_file"))?; let extracted = crate::ingest::extract_json_path(&raw, json_path)?; let array_text = crate::ingest::normalize_json_or_jsonl(&extracted)?; let mut result = ingest_json(engine, &array_text, &opts)?; @@ -2864,12 +2882,8 @@ impl HyperMcpServer { let sample_rows = params.sample_rows.unwrap_or(5).clamp(1, 50) as usize; let result = if let Some(ref json_path) = params.json_extract_path { (|| -> Result<_, McpError> { - let raw = std::fs::read_to_string(¶ms.path).map_err(|e| { - McpError::new( - ErrorCode::FileNotFound, - format!("Cannot read file '{}': {e}", params.path), - ) - })?; + let raw = std::fs::read_to_string(¶ms.path) + .map_err(|e| McpError::from_io_error(&e, "load_file"))?; let file_size = std::fs::metadata(¶ms.path).map_or(0, |m| m.len()); let extracted = crate::ingest::extract_json_path(&raw, json_path)?; crate::inspect::inspect_json_from_text(&extracted, file_size, sample_rows) @@ -3118,7 +3132,7 @@ impl HyperMcpServer { /// Save a value under store + key (upsert). Ephemeral unless routed. #[tool( - description = "KV scratchpad. Save a variable, state, summary, or JSON config under store + key to remember later without creating a database table. Overwrites any existing value (upsert). IMPORTANT: without `database` the value is written to the EPHEMERAL database and is LOST when the server restarts. To persist across restarts, pass database=\"persistent\" (or persist=true)." + description = "KV scratchpad. Save a variable, state, summary, or JSON config under store + key to remember later without creating a database table. IMPORTANT: without `database` the value is written to the EPHEMERAL database and is LOST when the server restarts. To persist across restarts, pass database=\"persistent\" (or persist=true). Returns {stored, created, value_bytes}; `created:false` means an existing value was overwritten. Pass overwrite=false to avoid clobbering (skips + returns stored:false, existed:true). Pass value_path= to store a file's contents server-side instead of `value` (exactly one of value/value_path; reads any server-readable path — no sandbox)." )] fn kv_set( &self, @@ -3127,13 +3141,65 @@ impl HyperMcpServer { if let Err(e) = self.check_writable("kv_set") { return Self::err_content(e); } + // Exactly one of value / value_path. + let value = match (p.value.as_deref(), p.value_path.as_deref()) { + (Some(v), None) => v.to_string(), + (None, Some(path)) => { + let canonical = match crate::attach::validate_input_path(path, "value_path") { + Ok(c) => c, + Err(e) => return Self::err_content(e), + }; + match std::fs::read_to_string(&canonical) { + Ok(s) => s, + Err(e) => return Self::err_content(McpError::from_io_error(&e, "value_path")), + } + } + (Some(_), Some(_)) => { + return Self::err_content(McpError::new( + ErrorCode::InvalidArgument, + "provide exactly one of `value` or `value_path`, not both", + )); + } + (None, None) => { + return Self::err_content(McpError::new( + ErrorCode::InvalidArgument, + "provide either `value` or `value_path`", + )); + } + }; + let value_bytes = value.len(); + let overwrite = p.overwrite.unwrap_or(true); + let result = self.with_engine(|engine| { let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; - kv.set(&p.key, &p.value).map_err(McpError::from) + if overwrite { + kv.set(&p.key, &value) + .map(|o| (true, o.created)) + .map_err(McpError::from) + } else { + kv.set_if_absent(&p.key, &value) + .map(|written| (written, written)) + .map_err(McpError::from) + } }); match result { - Ok(_) => Self::ok_content(json!({ "stored": true, "store": p.store, "key": p.key })), + Ok((stored, created)) => { + let mut body = json!({ + "stored": stored, + "created": created, + "store": p.store, + "key": p.key, + "value_bytes": value_bytes, + }); + if !stored { + body["existed"] = json!(true); + } + if let Some(w) = Self::kv_size_warning(value_bytes) { + body["warning"] = json!(w); + } + Self::ok_content(body) + } Err(e) => Self::err_content(e), } } diff --git a/hyperdb-mcp/tests/kv_tools_tests.rs b/hyperdb-mcp/tests/kv_tools_tests.rs index 16cb8f9..f5e7c1f 100644 --- a/hyperdb-mcp/tests/kv_tools_tests.rs +++ b/hyperdb-mcp/tests/kv_tools_tests.rs @@ -622,3 +622,97 @@ async fn kv_persistent_value_survives_server_restart() -> TestResult { h2.shutdown().await } + +/// kv_set reports `created` (insert vs overwrite) and `value_bytes`. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_reports_created_and_bytes() -> TestResult { + let h = TestHarness::start(false, false).await?; + let first = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "hello" }), + ) + .await?; + assert_eq!(structured(&first)["created"], serde_json::json!(true)); + assert_eq!(structured(&first)["value_bytes"], serde_json::json!(5)); + + let second = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "hi" }), + ) + .await?; + assert_eq!(structured(&second)["created"], serde_json::json!(false)); + h.shutdown().await +} + +/// overwrite:false skips an existing key without clobbering it. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_overwrite_false_guards() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "orig" }), + ) + .await?; + let guard = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "k", "value": "new", "overwrite": false }), + ) + .await?; + assert_eq!(structured(&guard)["stored"], serde_json::json!(false)); + assert_eq!(structured(&guard)["existed"], serde_json::json!(true)); + let got = call_tool( + &h.client, + "kv_get", + serde_json::json!({ "store": "s", "key": "k" }), + ) + .await?; + assert_eq!(structured(&got)["value"], serde_json::json!("orig")); + h.shutdown().await +} + +/// value_path reads a file's contents; neither/both value+value_path errors. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_value_path_reads_file() -> TestResult { + let h = TestHarness::start(false, false).await?; + let dir = tempfile::TempDir::new()?; + let path = dir.path().join("payload.txt"); + std::fs::write(&path, "from-file")?; + let abs = std::fs::canonicalize(&path)?; + + let set = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "f", "value_path": abs.to_string_lossy() }), + ) + .await?; + assert!( + !is_error(&set), + "value_path set failed: {:?}", + first_text(&set) + ); + let got = call_tool( + &h.client, + "kv_get", + serde_json::json!({ "store": "s", "key": "f" }), + ) + .await?; + assert_eq!(structured(&got)["value"], serde_json::json!("from-file")); + + // Neither value nor value_path → INVALID_ARGUMENT. + let neither = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "x" }), + ) + .await?; + assert!(is_error(&neither)); + // Both → INVALID_ARGUMENT. + let both = call_tool(&h.client, "kv_set", + serde_json::json!({ "store": "s", "key": "y", "value": "v", "value_path": abs.to_string_lossy() })).await?; + assert!(is_error(&both)); + h.shutdown().await +} From 4d357c9e8d5addbddbbc43d734dd620bc95455fb Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:05:54 -0700 Subject: [PATCH 13/20] test(mcp): complete kv_set test coverage for value_bytes and large-value warning Two critical test gaps identified in iteration 8 review: 1. kv_set_reports_created_and_bytes: Added missing assertion for second.value_bytes = 2. Test name promised both created and value_bytes checks but only verified created=false on the overwrite path. 2. New test kv_set_large_value_warns: Verifies kv_set returns a warning when value_bytes exceeds KV_SOFT_SIZE_WARN_BYTES (1_048_576). Boundary check confirms exactly 1MB triggers no warning, 1MB+1 does. Validates warning text contains "1048576" and mentions "soft limit"/"recommended". All 15 KV tests pass; full workspace clippy+fmt green. Refs: #192 iteration 8 blocking review findings ITER8-001, ITER8-002 --- hyperdb-mcp/tests/kv_tools_tests.rs | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/hyperdb-mcp/tests/kv_tools_tests.rs b/hyperdb-mcp/tests/kv_tools_tests.rs index f5e7c1f..ea40e77 100644 --- a/hyperdb-mcp/tests/kv_tools_tests.rs +++ b/hyperdb-mcp/tests/kv_tools_tests.rs @@ -643,6 +643,48 @@ async fn kv_set_reports_created_and_bytes() -> TestResult { ) .await?; assert_eq!(structured(&second)["created"], serde_json::json!(false)); + assert_eq!(structured(&second)["value_bytes"], serde_json::json!(2)); + h.shutdown().await +} + +/// kv_set warns when value_bytes exceeds the soft limit (1MB). +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_large_value_warns() -> TestResult { + let h = TestHarness::start(false, false).await?; + + // At exactly 1 MB boundary: no warning + let one_mb = "x".repeat(1_048_576); + let at_limit = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "at_limit", "value": one_mb }), + ) + .await?; + assert_eq!(structured(&at_limit)["stored"], serde_json::json!(true)); + assert!(structured(&at_limit)["warning"].is_null()); + + // Above 1 MB: warning fires + let over_mb = "x".repeat(1_048_577); + let over_limit = call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "over_limit", "value": over_mb }), + ) + .await?; + let structured_over = structured(&over_limit); + assert_eq!(structured_over["stored"], serde_json::json!(true)); + let warning = structured_over["warning"] + .as_str() + .expect("warning should be a string"); + assert!( + warning.contains("1048576"), + "warning should mention the byte limit; got: {warning}" + ); + assert!( + warning.contains("soft limit") || warning.contains("recommended"), + "warning should mention soft limit or recommended; got: {warning}" + ); + h.shutdown().await } From 0967c05eb7f27667ad9e1f792bc039ad332a8d06 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:07:37 -0700 Subject: [PATCH 14/20] feat(mcp): kv_size reports total value bytes --- hyperdb-mcp/src/server.rs | 12 +++++++++--- hyperdb-mcp/tests/kv_tools_tests.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index eeb8f20..bc5856a 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -3273,7 +3273,7 @@ impl HyperMcpServer { /// Count the keys in a scratchpad store. #[tool( - description = "Return the number of keys in a KV scratchpad store. Omit `database` for the ephemeral store, or route with \"persistent\"/persist=true/an attached alias." + description = "Returns {store, size, bytes} where `size` is the key count and `bytes` is the total `OCTET_LENGTH` of all values (0 for empty stores). Omit `database` for the ephemeral store, or route with \"persistent\"/persist=true/an attached alias." )] fn kv_size( &self, @@ -3282,10 +3282,16 @@ impl HyperMcpServer { let result = self.with_engine(|engine| { let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; - kv.size().map_err(McpError::from) + let key_count = kv.size().map_err(McpError::from)?; + let value_bytes = kv.byte_size().map_err(McpError::from)?; + Ok(json!({ + "store": p.store, + "size": key_count, + "bytes": value_bytes, + })) }); match result { - Ok(size) => Self::ok_content(json!({ "store": p.store, "size": size })), + Ok(val) => Self::ok_content(val), Err(e) => Self::err_content(e), } } diff --git a/hyperdb-mcp/tests/kv_tools_tests.rs b/hyperdb-mcp/tests/kv_tools_tests.rs index ea40e77..4742917 100644 --- a/hyperdb-mcp/tests/kv_tools_tests.rs +++ b/hyperdb-mcp/tests/kv_tools_tests.rs @@ -247,6 +247,33 @@ async fn kv_list_size_and_list_stores() -> TestResult { h.shutdown().await } +/// kv_size reports both key count and total value bytes. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_size_reports_bytes() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "abc" }), + ) + .await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "b", "value": "de" }), + ) + .await?; + + let size = call_tool(&h.client, "kv_size", serde_json::json!({ "store": "s" })).await?; + assert_eq!(structured(&size)["size"], serde_json::json!(2), "two keys"); + assert_eq!( + structured(&size)["bytes"], + serde_json::json!(5), + "3+2=5 bytes" + ); + h.shutdown().await +} + /// delete returns `{deleted:true}` when the key existed and /// `{deleted:false}` on a second delete (idempotent, not an error). #[tokio::test(flavor = "multi_thread", worker_threads = 2)] From 34f15f7b0f067d47932ef48d3434b9f584c5d8c2 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:12:40 -0700 Subject: [PATCH 15/20] feat(mcp): add kv_set_many atomic batch write tool --- hyperdb-mcp/src/server.rs | 107 ++++++++++++++++++++++++ hyperdb-mcp/tests/kv_tools_tests.rs | 122 ++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index bc5856a..94103e1 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -869,6 +869,39 @@ pub struct KvSetParams { pub persist: Option, } +/// A single key-value pair for batch writes. +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvEntry { + /// Key to write. + pub key: String, + /// Value to store. + pub value: String, +} + +/// Parameters for `kv_set_many` (atomic batch write). +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvSetManyParams { + /// Namespace of the KV store. Created on first write. + pub store: String, + /// Key-value pairs to write atomically. All keys are validated before the + /// transaction opens, so an invalid key aborts the whole batch without + /// writing anything. Empty `entries` is an error. + pub entries: Vec, + /// When false, skip existing keys instead of overwriting them (written + /// entries report `created`, skipped ones report `skipped`). Defaults to + /// true (upsert). + pub overwrite: Option, + /// Target database alias. Omit (or pass `"local"`) to write to the + /// ephemeral primary. Pass `"persistent"` to write to the durable database + /// that survives across sessions. Other values target a user-attached + /// database (must be writable). Each database has its own isolated stores. + pub database: Option, + /// Shorthand for `database: "persistent"`. When true, the batch is written + /// to the persistent database. If both `database` and `persist` are set, + /// `database` wins. + pub persist: Option, +} + /// Parameters for store-scoped KV tools (`kv_list`, `kv_size`, `kv_pop`, /// `kv_clear`). #[derive(Debug, Deserialize, JsonSchema)] @@ -3204,6 +3237,80 @@ impl HyperMcpServer { } } + /// Atomic batch write to the KV scratchpad. + #[tool( + description = "Write multiple KV pairs atomically. All keys validated before the transaction opens, so an invalid key aborts the whole batch. Returns {stored, created, overwritten, total_bytes} when overwrite=true (default); returns {stored, created, skipped, total_bytes} when overwrite=false (guard mode — skips existing keys). Empty `entries` is an error. Omit `database` to write to the ephemeral store; pass \"persistent\" (or persist=true) or an attached alias to write elsewhere." + )] + fn kv_set_many( + &self, + Parameters(p): Parameters, + ) -> Result { + if let Err(e) = self.check_writable("kv_set_many") { + return Self::err_content(e); + } + if p.entries.is_empty() { + return Self::err_content(McpError::new( + ErrorCode::InvalidArgument, + "entries must not be empty", + )); + } + + // Build the &[(&str, &str)] slice for the API crate; collect per-entry + // warnings for oversized values. + let pairs: Vec<(&str, &str)> = p + .entries + .iter() + .map(|e| (e.key.as_str(), e.value.as_str())) + .collect(); + let total_bytes: usize = p.entries.iter().map(|e| e.value.len()).sum(); + let mut warnings: Vec = Vec::new(); + for entry in &p.entries { + if let Some(w) = Self::kv_size_warning(entry.value.len()) { + warnings.push(json!({ "key": entry.key, "warning": w })); + } + } + + // Shape the outcome JSON *inside* each branch so the `with_engine` + // closure returns a single type (`Result`). The two + // batch primitives return different outcome structs (`BatchSetOutcome` + // vs `BatchGuardOutcome`), so a bare `if`/`else` returning both would + // not type-check — a closure, like any block, needs one return type. + // `total_bytes` and `warnings` are engine-independent (computed above), + // so they are spliced into the object after the closure returns. This + // mirrors the single-type-closure pattern used by `kv_list` (Task 11). + let overwrite = p.overwrite.unwrap_or(true); + let result = self.with_engine(|engine| { + let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; + let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; + if overwrite { + let o = kv.set_batch(&pairs).map_err(McpError::from)?; + Ok(json!({ + "stored": o.created + o.overwritten, + "created": o.created, + "overwritten": o.overwritten, + })) + } else { + let o = kv.set_batch_if_absent(&pairs).map_err(McpError::from)?; + Ok(json!({ + "stored": o.written, + "created": o.written, + "skipped": o.skipped, + })) + } + }); + + match result { + Ok(mut body) => { + body["total_bytes"] = json!(total_bytes); + if !warnings.is_empty() { + body["warnings"] = json!(warnings); + } + Self::ok_content(body) + } + Err(e) => Self::err_content(e), + } + } + /// Delete a key from the scratchpad. #[tool( description = "Delete a key from the KV scratchpad. Returns {deleted: true} when the key existed, {deleted: false} otherwise (no error). Omit `database` for the ephemeral store, or route with \"persistent\"/persist=true/an attached alias." diff --git a/hyperdb-mcp/tests/kv_tools_tests.rs b/hyperdb-mcp/tests/kv_tools_tests.rs index 4742917..1393f96 100644 --- a/hyperdb-mcp/tests/kv_tools_tests.rs +++ b/hyperdb-mcp/tests/kv_tools_tests.rs @@ -785,3 +785,125 @@ async fn kv_set_value_path_reads_file() -> TestResult { assert!(is_error(&both)); h.shutdown().await } + +/// kv_set_many writes all entries atomically (overwrite=true default); reports +/// {stored, created, overwritten, total_bytes}; a mixed batch counts correctly. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_many_writes_all() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "1" }), + ) + .await?; + + let batch = call_tool( + &h.client, + "kv_set_many", + serde_json::json!({ + "store": "s", + "entries": [ + { "key": "a", "value": "10" }, // overwrite + { "key": "b", "value": "20" }, // new + { "key": "c", "value": "30" }, // new + ] + }), + ) + .await?; + assert!( + !is_error(&batch), + "kv_set_many failed: {:?}", + first_text(&batch) + ); + assert_eq!(structured(&batch)["stored"], serde_json::json!(3)); + assert_eq!(structured(&batch)["created"], serde_json::json!(2)); + assert_eq!(structured(&batch)["overwritten"], serde_json::json!(1)); + assert_eq!( + structured(&batch)["total_bytes"], + serde_json::json!(6), + "10+20+30 = 6 bytes" + ); + + let got = call_tool( + &h.client, + "kv_get", + serde_json::json!({ "store": "s", "key": "a" }), + ) + .await?; + assert_eq!(structured(&got)["value"], serde_json::json!("10")); + h.shutdown().await +} + +/// kv_set_many with overwrite=false skips existing keys, reports {stored, created, skipped}. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_many_guard_skips_existing() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "orig" }), + ) + .await?; + + let guard = call_tool( + &h.client, + "kv_set_many", + serde_json::json!({ + "store": "s", + "entries": [ + { "key": "a", "value": "new" }, // skipped + { "key": "b", "value": "b1" }, // written + ], + "overwrite": false + }), + ) + .await?; + assert!( + !is_error(&guard), + "kv_set_many guard failed: {:?}", + first_text(&guard) + ); + assert_eq!(structured(&guard)["stored"], serde_json::json!(1)); + assert_eq!(structured(&guard)["created"], serde_json::json!(1)); + assert_eq!(structured(&guard)["skipped"], serde_json::json!(1)); + // total_bytes is the sum of ALL submitted entry values ("new"=3 + "b1"=2), + // an upper bound under overwrite=false: the batch-guard primitive returns + // only counts, not which keys were actually written, so total_bytes cannot + // subtract the skipped entry's bytes. + assert_eq!( + structured(&guard)["total_bytes"], + serde_json::json!(5), + "\"new\"(3) + \"b1\"(2), all submitted" + ); + + let got = call_tool( + &h.client, + "kv_get", + serde_json::json!({ "store": "s", "key": "a" }), + ) + .await?; + assert_eq!( + structured(&got)["value"], + serde_json::json!("orig"), + "existing value untouched" + ); + h.shutdown().await +} + +/// kv_set_many rejects empty entries with INVALID_ARGUMENT. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_set_many_empty_batch_errors() -> TestResult { + let h = TestHarness::start(false, false).await?; + let empty = call_tool( + &h.client, + "kv_set_many", + serde_json::json!({ + "store": "s", + "entries": [] + }), + ) + .await?; + assert!(is_error(&empty), "empty entries must error"); + h.shutdown().await +} From f0b35275a13bdb2289f58dd8f24e2321d1764e7a Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:20:04 -0700 Subject: [PATCH 16/20] feat(mcp): add values flag to kv_list for whole-store reads --- hyperdb-mcp/src/server.rs | 43 ++++++++++++++++++--- hyperdb-mcp/tests/kv_tools_tests.rs | 59 +++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index 94103e1..e7c575f 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -902,8 +902,7 @@ pub struct KvSetManyParams { pub persist: Option, } -/// Parameters for store-scoped KV tools (`kv_list`, `kv_size`, `kv_pop`, -/// `kv_clear`). +/// Parameters for store-scoped KV tools (`kv_size`, `kv_pop`, `kv_clear`). #[derive(Debug, Deserialize, JsonSchema)] pub struct KvStoreParams { /// Namespace of the KV store to operate on. @@ -917,6 +916,24 @@ pub struct KvStoreParams { pub persist: Option, } +/// Parameters for `kv_list` (enumerate keys, optionally with values). +#[derive(Debug, Deserialize, JsonSchema)] +pub struct KvListParams { + /// Namespace of the KV store to list. + pub store: String, + /// When true, return the full `(key, value)` pairs as `entries`; when + /// false or omitted, return only `keys` (the default behavior). Use + /// `values:true` for whole-store reads without N×`kv_get`. + pub values: Option, + /// Target database alias. Omit (or pass `"local"`) for the ephemeral + /// primary. Pass `"persistent"` for the durable database, or a + /// user-attached alias. Each database has its own isolated stores. + pub database: Option, + /// Shorthand for `database: "persistent"`. If both `database` and + /// `persist` are set, `database` wins. + pub persist: Option, +} + /// Parameters for `kv_list_stores` (enumerate every store in a database). #[derive(Debug, Deserialize, JsonSchema)] pub struct KvListStoresParams { @@ -3337,21 +3354,35 @@ impl HyperMcpServer { /// List all keys in a scratchpad store, sorted ascending. #[tool( - description = "List all keys in a KV scratchpad store, sorted ascending. Omit `database` for the ephemeral store, or route with \"persistent\"/persist=true/an attached alias." + description = "List all keys in a KV scratchpad store, sorted ascending. Omit `database` for the ephemeral store, or route with \"persistent\"/persist=true/an attached alias. Pass values=true to return full (key, value) pairs as an `entries` array instead of just keys — useful for reading a whole store without N×kv_get." )] fn kv_list( &self, - Parameters(p): Parameters, + Parameters(p): Parameters, ) -> Result { + let with_values = p.values.unwrap_or(false); let result = self.with_engine(|engine| { let db = self.resolve_db(engine, p.database.as_deref(), p.persist, true)?; let kv = Self::kv_open(engine, db.as_deref(), &p.store)?; - kv.keys().map_err(McpError::from) + if with_values { + kv.entries().map(|pairs| (Some(pairs), None)) + } else { + kv.keys().map(|keys| (None, Some(keys))) + } + .map_err(McpError::from) }); match result { - Ok(keys) => { + Ok((Some(entries), None)) => { + let arr: Vec = entries + .into_iter() + .map(|(k, v)| json!({ "key": k, "value": v })) + .collect(); + Self::ok_content(json!({ "store": p.store, "entries": arr })) + } + Ok((None, Some(keys))) => { Self::ok_content(json!({ "store": p.store, "count": keys.len(), "keys": keys })) } + Ok(_) => unreachable!("exactly one of entries/keys is Some"), Err(e) => Self::err_content(e), } } diff --git a/hyperdb-mcp/tests/kv_tools_tests.rs b/hyperdb-mcp/tests/kv_tools_tests.rs index 1393f96..b1b76e7 100644 --- a/hyperdb-mcp/tests/kv_tools_tests.rs +++ b/hyperdb-mcp/tests/kv_tools_tests.rs @@ -247,6 +247,65 @@ async fn kv_list_size_and_list_stores() -> TestResult { h.shutdown().await } +/// kv_list default (values absent/false) preserves the keys-only shape. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_list_keys_only_unchanged() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "a", "value": "v1" }), + ) + .await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "b", "value": "v2" }), + ) + .await?; + let list = call_tool(&h.client, "kv_list", serde_json::json!({ "store": "s" })).await?; + let body = structured(&list); + assert_eq!(body["store"], serde_json::json!("s")); + assert_eq!(body["count"], serde_json::json!(2)); + assert_eq!(body["keys"], serde_json::json!(["a", "b"])); + h.shutdown().await +} + +/// kv_list with values:true returns entries with both key and value. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn kv_list_values_returns_entries() -> TestResult { + let h = TestHarness::start(false, false).await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "x", "value": "hello" }), + ) + .await?; + call_tool( + &h.client, + "kv_set", + serde_json::json!({ "store": "s", "key": "y", "value": "world" }), + ) + .await?; + let list = call_tool( + &h.client, + "kv_list", + serde_json::json!({ "store": "s", "values": true }), + ) + .await?; + let body = structured(&list); + assert_eq!(body["store"], serde_json::json!("s")); + let entries = body["entries"] + .as_array() + .expect("entries must be an array"); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0]["key"], serde_json::json!("x")); + assert_eq!(entries[0]["value"], serde_json::json!("hello")); + assert_eq!(entries[1]["key"], serde_json::json!("y")); + assert_eq!(entries[1]["value"], serde_json::json!("world")); + h.shutdown().await +} + /// kv_size reports both key count and total value bytes. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn kv_size_reports_bytes() -> TestResult { From 3e3854976f8124149c440815a371c0b239ef69e0 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:25:32 -0700 Subject: [PATCH 17/20] docs(mcp): document kv JSON queries, ::numeric gotcha, batch/value_path/overwrite/values --- hyperdb-mcp/src/readme.rs | 32 ++++++++++++++++++++++++++++--- hyperdb-mcp/tests/readme_tests.rs | 1 + 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/hyperdb-mcp/src/readme.rs b/hyperdb-mcp/src/readme.rs index 9bccdee..c5fdefd 100644 --- a/hyperdb-mcp/src/readme.rs +++ b/hyperdb-mcp/src/readme.rs @@ -186,12 +186,26 @@ watcher targets the alias; call `unwatch_directory` first. ### Key-value store (scratchpad) - `kv_set` — save a variable / state / summary / JSON string under a - store + key (upsert). + store + key. Returns `{stored, created, value_bytes}`. Pass + `overwrite: false` to skip writes that would clobber an existing key + (response: `{stored: false, existed: true}`). Pass + `value_path: ` to store a file's contents server-side + instead of inlining `value` (exactly one of `value` / `value_path`; + reads any path the server process can read — no sandbox). +- `kv_set_many` — atomic batch write. Pass an `entries` array of + `{key, value}` objects. All keys validated up front; an invalid key + aborts the whole batch without writing anything. `overwrite: false` + skips existing keys within the batch. Returns + `{stored, created, overwritten, total_bytes}` (or `skipped` instead + of `overwritten` under `overwrite: false`). - `kv_get` — read a value by store + key. - `kv_delete` — delete a key. -- `kv_list` — list keys in a store. +- `kv_list` — list keys in a store. Pass `values: true` to return + `{entries: [{key, value}, ...]}` instead of `{keys: [...]}` — reads + the whole store in one call (eliminates N×`kv_get`). - `kv_list_stores` — list store namespaces that hold data in a database. -- `kv_size` — count keys in a store. +- `kv_size` — count keys and total value bytes in a store. Returns + `{size, bytes}`. - `kv_pop` — destructively read-and-remove the lowest-keyed entry (lexicographic key order, not insertion order). - `kv_clear` — delete all keys in a store. @@ -207,6 +221,18 @@ as the joined table. See the `hyper://schema/kv` resource for the join template, and `KV store vs. a custom table` above for when to reach for this instead of a real table. +**Querying JSON in a KV value:** Values are TEXT. To query JSON +structure, cast first: `SELECT value::json ->> 'field' FROM +_hyperdb_kv_store WHERE store_name = '...'`. The `->` / `->>` / +`JSON_EACH(...)` operators work AFTER the `::json` cast. Applying `->` +or `->>` to raw TEXT fails with SQLSTATE 42601 (\"requires a structured +data type\"). `JSON_VALUE(...)` is not implemented in this engine — use +the `::json` cast instead. + +**::numeric truncation gotcha:** A bare `::numeric` cast defaults to +scale 0 and truncates decimal places. Example: `41.54178215::numeric` +→ `42`. Always specify precision and scale: `::numeric(20,10)`. + ### Introspection - `get_readme` — this document. Call once at the start of a session. diff --git a/hyperdb-mcp/tests/readme_tests.rs b/hyperdb-mcp/tests/readme_tests.rs index a21d7bc..cea64dd 100644 --- a/hyperdb-mcp/tests/readme_tests.rs +++ b/hyperdb-mcp/tests/readme_tests.rs @@ -55,6 +55,7 @@ fn readme_mentions_every_tool() { "unwatch_directory", "kv_get", "kv_set", + "kv_set_many", "kv_delete", "kv_list", "kv_list_stores", From fff01112a98b38580e257033ae4861eda60e054b Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 13:47:56 -0700 Subject: [PATCH 18/20] docs(changelog): note KV breaking changes and MCP additions for 0.7.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per-crate CHANGELOG.md bullets only — release-please owns the root CHANGELOG.md, all Cargo.toml versions, and .release-please-manifest.json. The 0.6.1 → 0.7.0 bump is carried by the `feat!:` commits in this branch. hyperdb-api: BREAKING — set/set_as/set_batch return SetOutcome/BatchSetOutcome instead of Result<()>; added set_if_absent, set_batch_if_absent, byte_size, entries (sync + async twins). hyperdb-mcp: added kv_set_many tool, value_path + overwrite on kv_set, values flag on kv_list, byte reporting on kv_set/kv_size; fixed PermissionDenied I/O-error mapping and misleading JSON-error suggestion (now advises ::json cast instead of statement split). --- hyperdb-api/CHANGELOG.md | 9 +++++++++ hyperdb-mcp/CHANGELOG.md | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/hyperdb-api/CHANGELOG.md b/hyperdb-api/CHANGELOG.md index a201666..2149659 100644 --- a/hyperdb-api/CHANGELOG.md +++ b/hyperdb-api/CHANGELOG.md @@ -7,8 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Changed + +- **BREAKING:** `KvStore::set`, `KvStore::set_as`, and `KvStore::set_batch` (plus their `AsyncKvStore` twins) now return `SetOutcome` or `BatchSetOutcome` instead of `Result<()>`, reporting whether each write created a new key or overwrote an existing one. The `created` signal eliminates silent data loss when an LLM accidentally clobbers existing KV data. Pre-0.7.0 callers that ignored the `Result` (statement-position `set("k","v")?;`) still compile unchanged; callers that bound the return (`let _ = set(...)?;`) must destructure or ignore the outcome. Released as 0.7.0 under pre-1.0 semver (the minor slot is the breaking slot). + ### Added +- `KvStore::set_if_absent` / `AsyncKvStore::set_if_absent` — guarded write that inserts only if the key is absent (no check-then-write race; single `INSERT ... WHERE NOT EXISTS`). Returns `true` if written, `false` if the key already existed (nothing written). +- `KvStore::set_batch_if_absent` / `AsyncKvStore::set_batch_if_absent` — atomic batch variant of `set_if_absent`, returning `BatchGuardOutcome { written, skipped }`. All keys are validated before the transaction opens; an invalid key aborts the whole batch. +- `KvStore::byte_size` / `AsyncKvStore::byte_size` — returns the total byte length of all values in the store (`SUM(OCTET_LENGTH(value))`); 0 for an empty store. +- `KvStore::entries` / `AsyncKvStore::entries` — returns all `(key, value)` pairs sorted by key ascending, materializing the whole store. Intended for small scratchpad stores. +- `SetOutcome`, `BatchSetOutcome`, `BatchGuardOutcome` — public outcome types re-exported from `hyperdb_api` (sync + async twins). - Key-value store API: `Connection::kv_store` / `AsyncConnection::kv_store` returning `KvStore` / `AsyncKvStore` handles over a fixed `_hyperdb_kv_store` table, with `get`/`set`/`get_as`/`set_as`/`delete`/`exists`/`size`/`keys`/`pop`/`clear`/`set_batch`, diff --git a/hyperdb-mcp/CHANGELOG.md b/hyperdb-mcp/CHANGELOG.md index cefbbf2..e68b648 100644 --- a/hyperdb-mcp/CHANGELOG.md +++ b/hyperdb-mcp/CHANGELOG.md @@ -9,6 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Added +- **`kv_set_many` tool** — atomic batch write accepting an array of `{key, value}` entries. Validates all keys before opening the transaction; an invalid key aborts the whole batch without writing anything. Default behavior (`overwrite` absent or `true`) reports `{stored, created, overwritten, total_bytes}`; with `overwrite: false`, existing keys are skipped (not errors) and the response reports `{stored, created: 0, skipped, total_bytes}`. Each oversized entry (> 1 MiB) adds a keyed `warning` to a `warnings` array. +- **`kv_set` — `value_path` parameter** — absolute path to a file whose contents become the value (read server-side). Provide exactly one of `value` or `value_path`; neither or both is `INVALID_ARGUMENT`. Reads any path the server process can read (same posture as `load_file` — no sandbox), with I/O errors preserved (`PermissionDenied` → `ErrorCode::PermissionDenied`, not collapsed to `FileNotFound`). +- **`kv_set` — `overwrite` parameter** (default `true`) — when `false`, skips the write if the key already exists (calls `set_if_absent` instead of `set`), returning `{stored: false, created: false, existed: true}` with the original value unchanged. Eliminates silent data-loss from accidental overwrites. +- **`kv_set` — `created` and `value_bytes` in response** — `created: true` means the key was newly inserted, `false` means an existing value was overwritten. `value_bytes` reports the UTF-8 byte length of the written value. +- **`kv_set` — soft size warning** — values exceeding 1 MiB trigger a non-fatal `warning` field in the response steering the LLM toward `load_data` or a real table for large payloads. The write always succeeds. +- **`kv_size` — `bytes` field** — response now includes `{store, size, bytes}` where `size` is the key count (unchanged) and `bytes` is `SUM(OCTET_LENGTH(value))` (0 for an empty store). +- **`kv_list` — `values` flag** (default `false`) — when `true`, the response includes `{store, entries: [{key, value}, ...]}` instead of `{store, keys: [...]}`, eliminating the N×`kv_get` read pattern for whole-store materialization. Backed by a single `SELECT key, value` streamed to completion; acceptable for small scratchpad stores. - **Key-value scratchpad tools** — eight new MCP tools (`kv_set`, `kv_get`, `kv_delete`, `kv_list`, `kv_list_stores`, `kv_size`, `kv_pop`, `kv_clear`) let an LLM stash variables, state, summaries, or JSON strings under a @@ -27,6 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Fixed +- **I/O error fidelity on `value_path` and `load_file`.** File-read errors now preserve `PermissionDenied` → `ErrorCode::PermissionDenied` instead of collapsing every I/O failure to `FileNotFound`. A missing file still maps to `FileNotFound`; any other I/O error becomes a generic `InternalError` with the OS message. (Implemented via `McpError::from_io_error` in `error.rs`.) +- **Misleading JSON-error `suggestion` text corrected.** The "requires a structured data type" (`42601`) and `JSON_VALUE`-not-implemented (`0A000`) errors previously suggested splitting the statement, which was wrong; they now advise casting the TEXT value to `json` first (`value::json ->> 'field'`), the actual fix. Applies narrowly to JSON-related cases; other `42601` syntax errors carry a generic message without a misleading hint. - **Caller-fixable argument errors now return `INVALID_ARGUMENT`, not `INTERNAL_ERROR`.** An invalid identifier from the `hyperdb-api` layer — such as a KV `store`/`key` containing a disallowed byte or exceeding the From 0fda8ece5ac0e6065c2b797fd9fe26d6221279e9 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 14:23:28 -0700 Subject: [PATCH 19/20] fix(mcp): #192 cap kv_set value_path file size before reading Reject a value_path file larger than 64 MiB against its metadata BEFORE reading it into memory, so a stray path to a multi-gigabyte file returns INVALID_ARGUMENT instead of OOMing the server by slurping the whole thing into a single KV TEXT value. The soft 1 MiB warning still fires post-write for smaller-but-large values; this adds a hard, enforced ceiling for the file-backed path. Pure check_value_path_size helper is unit-tested at the boundary (at-cap passes, cap+1 rejects) without materializing a huge fixture. --- hyperdb-mcp/CHANGELOG.md | 2 +- hyperdb-mcp/src/server.rs | 63 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/hyperdb-mcp/CHANGELOG.md b/hyperdb-mcp/CHANGELOG.md index e68b648..80bfab6 100644 --- a/hyperdb-mcp/CHANGELOG.md +++ b/hyperdb-mcp/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Added - **`kv_set_many` tool** — atomic batch write accepting an array of `{key, value}` entries. Validates all keys before opening the transaction; an invalid key aborts the whole batch without writing anything. Default behavior (`overwrite` absent or `true`) reports `{stored, created, overwritten, total_bytes}`; with `overwrite: false`, existing keys are skipped (not errors) and the response reports `{stored, created: 0, skipped, total_bytes}`. Each oversized entry (> 1 MiB) adds a keyed `warning` to a `warnings` array. -- **`kv_set` — `value_path` parameter** — absolute path to a file whose contents become the value (read server-side). Provide exactly one of `value` or `value_path`; neither or both is `INVALID_ARGUMENT`. Reads any path the server process can read (same posture as `load_file` — no sandbox), with I/O errors preserved (`PermissionDenied` → `ErrorCode::PermissionDenied`, not collapsed to `FileNotFound`). +- **`kv_set` — `value_path` parameter** — absolute path to a file whose contents become the value (read server-side). Provide exactly one of `value` or `value_path`; neither or both is `INVALID_ARGUMENT`. Reads any path the server process can read (same posture as `load_file` — no sandbox), with I/O errors preserved (`PermissionDenied` → `ErrorCode::PermissionDenied`, not collapsed to `FileNotFound`). A hard 64 MiB size cap is enforced against the file's metadata *before* reading, so a stray path to a huge file is rejected with `INVALID_ARGUMENT` instead of being slurped into memory. - **`kv_set` — `overwrite` parameter** (default `true`) — when `false`, skips the write if the key already exists (calls `set_if_absent` instead of `set`), returning `{stored: false, created: false, existed: true}` with the original value unchanged. Eliminates silent data-loss from accidental overwrites. - **`kv_set` — `created` and `value_bytes` in response** — `created: true` means the key was newly inserted, `false` means an existing value was overwritten. `value_bytes` reports the UTF-8 byte length of the written value. - **`kv_set` — soft size warning** — values exceeding 1 MiB trigger a non-fatal `warning` field in the response steering the LLM toward `load_data` or a real table for large payloads. The write always succeeds. diff --git a/hyperdb-mcp/src/server.rs b/hyperdb-mcp/src/server.rs index e7c575f..7b72edf 100644 --- a/hyperdb-mcp/src/server.rs +++ b/hyperdb-mcp/src/server.rs @@ -1253,6 +1253,31 @@ impl HyperMcpServer { }) } + /// Hard cap (bytes) on a `value_path` file. Unlike the soft warning, this is + /// enforced: a file above the cap is rejected *before* it is read, so a + /// stray path to a multi-gigabyte file can't OOM the server by slurping the + /// whole thing into a single TEXT value. Generous enough (64 MiB) that any + /// legitimate scratchpad value passes. + const KV_VALUE_PATH_MAX_BYTES: u64 = 64 * 1024 * 1024; + + /// Rejects a `value_path` file whose size exceeds [`Self::KV_VALUE_PATH_MAX_BYTES`]. + /// `Ok(())` means the file is small enough to read into memory. Kept as a + /// pure helper so the bound is unit-testable without materializing a + /// multi-megabyte fixture on disk. + fn check_value_path_size(bytes: u64) -> Result<(), McpError> { + if bytes > Self::KV_VALUE_PATH_MAX_BYTES { + return Err(McpError::new( + ErrorCode::InvalidArgument, + format!( + "value_path file is {bytes} bytes (> {} hard limit); the KV store \ + is for small scraps — use load_file or a real table for large payloads", + Self::KV_VALUE_PATH_MAX_BYTES + ), + )); + } + Ok(()) + } + /// Opens a KV store handle on the engine's connection, targeting the /// resolved `database` (`None` = the ephemeral primary's default location, /// `Some(alias)` = the persistent DB or an attached alias). @@ -3182,7 +3207,7 @@ impl HyperMcpServer { /// Save a value under store + key (upsert). Ephemeral unless routed. #[tool( - description = "KV scratchpad. Save a variable, state, summary, or JSON config under store + key to remember later without creating a database table. IMPORTANT: without `database` the value is written to the EPHEMERAL database and is LOST when the server restarts. To persist across restarts, pass database=\"persistent\" (or persist=true). Returns {stored, created, value_bytes}; `created:false` means an existing value was overwritten. Pass overwrite=false to avoid clobbering (skips + returns stored:false, existed:true). Pass value_path= to store a file's contents server-side instead of `value` (exactly one of value/value_path; reads any server-readable path — no sandbox)." + description = "KV scratchpad. Save a variable, state, summary, or JSON config under store + key to remember later without creating a database table. IMPORTANT: without `database` the value is written to the EPHEMERAL database and is LOST when the server restarts. To persist across restarts, pass database=\"persistent\" (or persist=true). Returns {stored, created, value_bytes}; `created:false` means an existing value was overwritten. Pass overwrite=false to avoid clobbering (skips + returns stored:false, existed:true). Pass value_path= to store a file's contents server-side instead of `value` (exactly one of value/value_path; reads any server-readable path — no sandbox; files over 64 MiB are rejected before reading)." )] fn kv_set( &self, @@ -3199,6 +3224,16 @@ impl HyperMcpServer { Ok(c) => c, Err(e) => return Self::err_content(e), }; + // Enforce the hard size cap on the file's metadata BEFORE reading, + // so a huge path is rejected instead of slurped into memory. + match std::fs::metadata(&canonical) { + Ok(m) => { + if let Err(e) = Self::check_value_path_size(m.len()) { + return Self::err_content(e); + } + } + Err(e) => return Self::err_content(McpError::from_io_error(&e, "value_path")), + } match std::fs::read_to_string(&canonical) { Ok(s) => s, Err(e) => return Self::err_content(McpError::from_io_error(&e, "value_path")), @@ -5377,3 +5412,29 @@ mod validate_execute_batch_tests { validate_execute_batch(&s(&["INSERT INTO t VALUES (1);"])).unwrap(); } } + +#[cfg(test)] +mod kv_value_path_size_tests { + use super::*; + + #[test] + fn accepts_file_at_or_below_cap() { + // A zero-byte file, a modest file, and exactly-at-the-cap all pass. + HyperMcpServer::check_value_path_size(0).unwrap(); + HyperMcpServer::check_value_path_size(1024).unwrap(); + HyperMcpServer::check_value_path_size(HyperMcpServer::KV_VALUE_PATH_MAX_BYTES).unwrap(); + } + + #[test] + fn rejects_file_above_cap() { + let err = + HyperMcpServer::check_value_path_size(HyperMcpServer::KV_VALUE_PATH_MAX_BYTES + 1) + .unwrap_err(); + assert_eq!(err.code, ErrorCode::InvalidArgument); + assert!( + err.message.contains("hard limit"), + "message should name the hard limit: {}", + err.message + ); + } +} From f7b577d80b03060d634a1b67885f1d59ca590174 Mon Sep 17 00:00:00 2001 From: Stefan Steiner Date: Sat, 11 Jul 2026 14:25:09 -0700 Subject: [PATCH 20/20] docs(kv): #192 correct changelog/rustdoc accuracy from review sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Final-sweep review reconciliation — doc-only, no behavior change: - hyperdb-mcp CHANGELOG: kv_set_many under overwrite:false returns `created` = keys newly inserted, NOT the literal `created: 0` the bullet claimed (the code returns o.written; kv_set_many_guard test asserts created==1). Also note total_bytes is an upper bound. - hyperdb-api CHANGELOG: `let _ = set(...)?;` already compiles unchanged — it is NOT a breaking case; name the genuinely-breaking shapes instead. Drop the hardcoded "0.7.0" that presumed the version release-please will assign. - set_if_absent rustdoc (sync + async twins): qualify "no check-then-write race" with the cross-process caveat (no unique constraint on (store_name,key); exact within one process/the serialized MCP daemon), mirroring kv_pop's wording. - readme.rs: note the 64 MiB value_path cap and total_bytes upper-bound semantics for the LLM. --- hyperdb-api/CHANGELOG.md | 2 +- hyperdb-api/src/async_kv_store.rs | 7 ++++++- hyperdb-api/src/kv_store.rs | 7 ++++++- hyperdb-mcp/CHANGELOG.md | 2 +- hyperdb-mcp/src/readme.rs | 7 +++++-- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/hyperdb-api/CHANGELOG.md b/hyperdb-api/CHANGELOG.md index 2149659..7d598db 100644 --- a/hyperdb-api/CHANGELOG.md +++ b/hyperdb-api/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Changed -- **BREAKING:** `KvStore::set`, `KvStore::set_as`, and `KvStore::set_batch` (plus their `AsyncKvStore` twins) now return `SetOutcome` or `BatchSetOutcome` instead of `Result<()>`, reporting whether each write created a new key or overwrote an existing one. The `created` signal eliminates silent data loss when an LLM accidentally clobbers existing KV data. Pre-0.7.0 callers that ignored the `Result` (statement-position `set("k","v")?;`) still compile unchanged; callers that bound the return (`let _ = set(...)?;`) must destructure or ignore the outcome. Released as 0.7.0 under pre-1.0 semver (the minor slot is the breaking slot). +- **BREAKING:** `KvStore::set`, `KvStore::set_as`, and `KvStore::set_batch` (plus their `AsyncKvStore` twins) now return `SetOutcome` or `BatchSetOutcome` instead of `Result<()>`, reporting whether each write created a new key or overwrote an existing one. The `created` signal eliminates silent data loss when an LLM accidentally clobbers existing KV data. Callers that ignored the `Result` (statement-position `set("k","v")?;`) — including `let _ = set(...)?;` — still compile unchanged. The genuinely breaking cases are callers that named the unit return (`let x: () = set(...)?;`) or that returned `set(...)` where a `Result<()>` was expected; these now see `SetOutcome`/`BatchSetOutcome` and must adapt. Under pre-1.0 semver the minor slot is the breaking slot, so this lands in the next minor release. ### Added diff --git a/hyperdb-api/src/async_kv_store.rs b/hyperdb-api/src/async_kv_store.rs index 30e7971..a18951a 100644 --- a/hyperdb-api/src/async_kv_store.rs +++ b/hyperdb-api/src/async_kv_store.rs @@ -185,7 +185,12 @@ impl<'conn> AsyncKvStore<'conn> { /// /// Returns `true` if a row was written, `false` if the key already existed /// (in which case nothing is written). A single `INSERT ... WHERE NOT - /// EXISTS` statement decides, so there is no check-then-write race. + /// EXISTS` statement decides, so there is no check-then-write race within a + /// connection. The backing table has no unique constraint on + /// `(store_name, key)`, so two *separate* processes writing the same key to + /// a shared persistent store concurrently could both pass `NOT EXISTS` and + /// double-insert; within a single process (including the MCP daemon, which + /// serializes engine access) the guard is exact. /// /// # Errors /// diff --git a/hyperdb-api/src/kv_store.rs b/hyperdb-api/src/kv_store.rs index e1f6e0a..2055562 100644 --- a/hyperdb-api/src/kv_store.rs +++ b/hyperdb-api/src/kv_store.rs @@ -290,7 +290,12 @@ impl<'conn> KvStore<'conn> { /// /// Returns `true` if a row was written, `false` if the key already existed /// (in which case nothing is written). A single `INSERT ... WHERE NOT - /// EXISTS` statement decides, so there is no check-then-write race. + /// EXISTS` statement decides, so there is no check-then-write race within a + /// connection. The backing table has no unique constraint on + /// `(store_name, key)`, so two *separate* processes writing the same key to + /// a shared persistent store concurrently could both pass `NOT EXISTS` and + /// double-insert; within a single process (including the MCP daemon, which + /// serializes engine access) the guard is exact. /// /// # Errors /// diff --git a/hyperdb-mcp/CHANGELOG.md b/hyperdb-mcp/CHANGELOG.md index 80bfab6..a446fbc 100644 --- a/hyperdb-mcp/CHANGELOG.md +++ b/hyperdb-mcp/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Added -- **`kv_set_many` tool** — atomic batch write accepting an array of `{key, value}` entries. Validates all keys before opening the transaction; an invalid key aborts the whole batch without writing anything. Default behavior (`overwrite` absent or `true`) reports `{stored, created, overwritten, total_bytes}`; with `overwrite: false`, existing keys are skipped (not errors) and the response reports `{stored, created: 0, skipped, total_bytes}`. Each oversized entry (> 1 MiB) adds a keyed `warning` to a `warnings` array. +- **`kv_set_many` tool** — atomic batch write accepting an array of `{key, value}` entries. Validates all keys before opening the transaction; an invalid key aborts the whole batch without writing anything. Default behavior (`overwrite` absent or `true`) reports `{stored, created, overwritten, total_bytes}`; with `overwrite: false`, existing keys are skipped (not errors) and the response reports `{stored, created, skipped, total_bytes}` where `created` is the number of keys newly inserted. `total_bytes` sums all submitted values, so it is an upper bound on bytes actually persisted when keys are skipped or duplicated. Each oversized entry (> 1 MiB) adds a keyed `warning` to a `warnings` array. - **`kv_set` — `value_path` parameter** — absolute path to a file whose contents become the value (read server-side). Provide exactly one of `value` or `value_path`; neither or both is `INVALID_ARGUMENT`. Reads any path the server process can read (same posture as `load_file` — no sandbox), with I/O errors preserved (`PermissionDenied` → `ErrorCode::PermissionDenied`, not collapsed to `FileNotFound`). A hard 64 MiB size cap is enforced against the file's metadata *before* reading, so a stray path to a huge file is rejected with `INVALID_ARGUMENT` instead of being slurped into memory. - **`kv_set` — `overwrite` parameter** (default `true`) — when `false`, skips the write if the key already exists (calls `set_if_absent` instead of `set`), returning `{stored: false, created: false, existed: true}` with the original value unchanged. Eliminates silent data-loss from accidental overwrites. - **`kv_set` — `created` and `value_bytes` in response** — `created: true` means the key was newly inserted, `false` means an existing value was overwritten. `value_bytes` reports the UTF-8 byte length of the written value. diff --git a/hyperdb-mcp/src/readme.rs b/hyperdb-mcp/src/readme.rs index c5fdefd..01873c8 100644 --- a/hyperdb-mcp/src/readme.rs +++ b/hyperdb-mcp/src/readme.rs @@ -191,13 +191,16 @@ watcher targets the alias; call `unwatch_directory` first. (response: `{stored: false, existed: true}`). Pass `value_path: ` to store a file's contents server-side instead of inlining `value` (exactly one of `value` / `value_path`; - reads any path the server process can read — no sandbox). + reads any path the server process can read — no sandbox; files over + 64 MiB are rejected before reading). - `kv_set_many` — atomic batch write. Pass an `entries` array of `{key, value}` objects. All keys validated up front; an invalid key aborts the whole batch without writing anything. `overwrite: false` skips existing keys within the batch. Returns `{stored, created, overwritten, total_bytes}` (or `skipped` instead - of `overwritten` under `overwrite: false`). + of `overwritten` under `overwrite: false`). `total_bytes` counts all + submitted values — an upper bound on bytes actually persisted when + keys are skipped or duplicated. - `kv_get` — read a value by store + key. - `kv_delete` — delete a key. - `kv_list` — list keys in a store. Pass `values: true` to return