From 2e1b1e74cd6da1d07c6133df53c7f6c0ba6eee03 Mon Sep 17 00:00:00 2001 From: Vaibhavtripathi7 Date: Fri, 11 Sep 2026 18:19:07 +0530 Subject: [PATCH] fix(ingest): reject unknown --kind instead of storing text as a memory Signed-off-by: Vaibhavtripathi7 --- CHANGELOG.md | 8 +++++++ src/hydradb_cli/commands/canonical.py | 7 ++++++ tests/test_cli_commands.py | 33 +++++++++++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0126325..2c85182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,14 @@ - **`HYDRADB_GRAPH_COLLECTION`** sets the default graph collection (default `default`). It deliberately does not fall back to `HYDRADB_COLLECTION`: a context collection names a memory/knowledge partition and means nothing to a graph, so inheriting it would silently point Cypher at a collection you never chose. +### Fixed + +- **`ingest` rejects an unknown `--kind`** instead of storing the text as a memory. + Only `knowledge` was matched, so `--kind knowlegde`, `--kind Knowledge` or an empty + `--kind ""` fell through to the memory path, sent `type=memory`, and printed + `✓ Memory added`. It now fails with the same `--kind must be one of: knowledge, memory` + error that `query`, `list` and `delete` already give. + ### Internal - `HydraDB.graph` is a hand-rolled `httpx` path rather than an SDK call: the pinned `hydradb-sdk==2.1.2` exposes `context`, `databases`, `connectors` and `webhooks` and has no `byog` resource, so those endpoints are unreachable through it. It reuses the wrapper's existing envelope unwrapping and error translation and raises the same `HydraDBClientError`, so `handle_api_error` treats a BYOG failure exactly like an SDK one. When the SDK grows a `byog` resource, that one class is reimplemented over it and no caller changes. The exact SDK pin (CONTRACT S2 rule 1) is unaffected — there is no generated name to be insulated from yet. diff --git a/src/hydradb_cli/commands/canonical.py b/src/hydradb_cli/commands/canonical.py index 28b6c38..cc97f1e 100644 --- a/src/hydradb_cli/commands/canonical.py +++ b/src/hydradb_cli/commands/canonical.py @@ -102,6 +102,13 @@ def ingest( ) -> None: """Ingest a memory, knowledge text, or knowledge file(s).""" tid, stid = resolve_scope_flags(database, collection, tenant_id, sub_tenant_id) + # Only `knowledge` is matched below; anything else used to fall through to + # the memory path, so a typo like `--kind knowlegde` stored the text as a + # memory and reported success. Reject it up front, the way delete does. + # `is not None`, not truthiness: `--kind ""` (an unset shell variable) must + # be rejected too, and only omitting the flag keeps the memory default. + if kind is not None and kind not in _impl.VALID_KINDS: + print_error(f"--kind must be one of: {', '.join(sorted(_impl.VALID_KINDS))}. Got '{kind}'.") if files: # Files are always knowledge sources. Reject every option that would be # silently ignored rather than storing the file the wrong way. Only diff --git a/tests/test_cli_commands.py b/tests/test_cli_commands.py index 9553f10..aded024 100644 --- a/tests/test_cli_commands.py +++ b/tests/test_cli_commands.py @@ -202,6 +202,39 @@ def test_ingest_knowledge_text(self): assert "uploaded" in result.output.lower() assert w.context.ingest.call_args.kwargs["kind"] == "knowledge" + def test_ingest_unknown_kind_fails(self): + # Anything but "knowledge" used to fall through to the memory path, so + # a typo stored the text in the wrong corpus and reported success. + _auth() + w = _wrapper() + with _patch_wrapper(w): + result = runner.invoke(app, ["ingest", "--kind", "knowlegde", "--text", "notes"]) + assert result.exit_code != 0 + assert "--kind must be one of" in result.output + w.context.ingest.assert_not_called() + + def test_ingest_empty_kind_fails(self): + # An explicit empty value (an unset shell variable) is not the same as + # omitting the flag; it must not default to memory. + _auth() + w = _wrapper() + with _patch_wrapper(w): + result = runner.invoke(app, ["ingest", "--kind", "", "--text", "notes"]) + assert result.exit_code != 0 + assert "--kind must be one of" in result.output + w.context.ingest.assert_not_called() + + def test_ingest_files_with_unknown_kind_fails(self, tmp_path): + _auth() + f = tmp_path / "a.txt" + f.write_text("aaa") + w = _wrapper() + with _patch_wrapper(w): + result = runner.invoke(app, ["ingest", str(f), "--kind", "knowlegde"]) + assert result.exit_code != 0 + assert "--kind must be one of" in result.output + w.context.ingest_many.assert_not_called() + def test_ingest_files_loops(self, tmp_path): _auth() f1 = tmp_path / "a.txt"