Test#418
Merged
Merged
Conversation
fix: track backend label separately in JSONL search handler (#14)
fix: add existence guards to all put_* methods (#12)
…act-guard fix(proposals): refuse to overwrite existing artifact on approve
kb.register_source_from_path read the contents of any file the process could access, letting an agent register /etc/passwd, ~/.ssh/id_rsa, ~/.aws/credentials etc. as a "source" and then retrieve the bytes via kb.cite or kb.list_sources. Resolve the path (following symlinks) and require it to be inside the KB root before reading. Adds KBStore.resolve_under_root() so both the JSONL handler and the MCP tool share one containment check. Fixes #10
The CLI handlers for approve and reject caught only (ArtifactNotFoundError, ValueError) but proposals.approve() / proposals.reject() raise ProposalError -- a RuntimeError subclass. The four propose-* shortcuts caught nothing at all. Result: a double- approve, an empty rejection reason, an empty claim text, or an unknown source id surfaced a raw Python traceback instead of the one-line `Error: ...` the rest of the CLI emits. Add a small `_cli_errors` context manager that translates ArtifactNotFoundError / ValueError / ProposalError / LifecycleError into click.ClickException, and apply it to every command that calls into proposals, lifecycle, sessions, or storage. The MCP and JSONL servers already do the equivalent in their own envelopes; this brings the human-facing surface in line. Adds tests/test_cli.py with regressions for approve, reject, propose- claim, propose-entity, and show.
ruff B904 requires `raise ... from err` inside an except block so the original exception is preserved on the chained __cause__. Without it, the CI lint step rejects the file. The seven sites are pre-existing (put_claim, put_page, put_entity, put_relation, put_evidence, put_session, put_proposal); this PR inherited the failure from main rather than introducing it, but the path traversal fix cannot land until lint is green, so the cleanup happens here. No behavior change: the chained exception carries .__cause__ but the public ValueError message and type are unchanged.
verify_source() caught FileNotFoundError, but store.read_source_content() raises ArtifactNotFoundError (a KeyError subclass) when the content blob is missing -- so the "stored content missing" graceful path never ran and a single broken source crashed the entire verify_all() sweep, breaking `vouch source verify` and `vouch doctor`. The same call can also raise OSError (permission denied, TOCTOU race between exists() and read_bytes(), underlying I/O error) which was likewise unhandled. Catch both: the existing "missing" path keeps its note, and OSError surfaces as "stored content unreadable: <reason>". Adds three regression tests: - missing-content blob -> graceful per-source failure - unreadable stored content (monkeypatched PermissionError) -> graceful per-source failure with the underlying reason in the note - mixed sweep with one good + one broken source -> verify_all returns both results instead of aborting at the first failure Fixes #30
The CI workflow runs `ruff check`, `mypy`, then `pytest`, and the fix/cli-clean-domain-errors branch was failing all three on pre-existing main-branch issues unrelated to the CLI change itself. * ruff: add `from e` to the seven `raise ValueError(...) from FileExistsError` re-raises added by the recent exclusive-create guards in storage.put_* (B904), and let ruff re-sort the cli.py import block (I001). * mypy: narrow the `kind` value flowing into `ContextItem(type=...)` with a `Literal` cast so the strict-typed field accepts what the search backends actually return. * pytest: `session_end()` mutates a session that `session_start()` has already written, but `put_session()` now uses exclusive create and rejects the second write. Add `KBStore.update_session()` mirroring `update_claim` and have `session_end` call it; existing test_sessions coverage now passes again. No behavior change for the CLI surface — these are infrastructure fixes so the existing test_sessions / lint / type assertions pass on this branch.
The B904 lint failures were already addressed in 2a439a5, but the CI matrix still fails on two pre-existing main-branch issues: * mypy: context.py:64 passed `kind: str` into ContextItem.type, which is Literal["claim","page","entity","relation","source"]. Narrow it with a typing.cast so strict mode accepts the search-backend output. * pytest: session_end() mutates a session that session_start() already wrote, but put_session() switched to exclusive create and rejects the second write. Add KBStore.update_session() mirroring update_claim and have session_end call it; test_sessions coverage passes again. No behavior change for the path-traversal fix itself.
CodeRabbit on PR #28 noted that resolve_under_root() only validated a pathname snapshot. Callers then re-opened the same name with .is_file() and .read_bytes(), so an attacker who can swap the resolved path for a symlink between the containment check and the read can still exfiltrate an out-of-root file via kb.register_source_from_path. Collapse the validate-then-read into a single trusted helper `KBStore.read_under_root(path)` that returns `(resolved, bytes)`: 1. Path.resolve() chases pre-existing symlinks and the resulting target is checked for containment (existing behaviour -- legitimate in-root symlinks still work). 2. The read goes through `os.open(resolved, O_RDONLY | O_NOFOLLOW)` so a fresh symlink placed at the resolved name *after* the check fails with ELOOP rather than following the swap. 3. `fstat` + `S_ISREG` rejects directories / device nodes / pipes atomically on the same fd, replacing the racy `is_file()` test the callers used to do. Both register_source_from_path handlers (MCP + JSONL) switch to the new helper and drop their now-redundant follow-up checks. Adds tests: - symlink swapped into the resolved name -> rejected via ELOOP - directory at a valid path -> rejected via S_ISREG Existing "outside the root" and "inside the root" tests still pass.
…versal fix(server): block path traversal in register_source_from_path
ruff B904 requires `raise ... from err` inside an except block so the original exception is preserved on the chained __cause__. Without it, the CI lint step rejects the file. The seven sites are pre-existing (put_claim, put_page, put_entity, put_relation, put_evidence, put_session, put_proposal); this PR inherited the failure from main rather than introducing it, but the path traversal fix cannot land until lint is green, so the cleanup happens here. No behavior change: the chained exception carries .__cause__ but the public ValueError message and type are unchanged.
The B904 lint failures were already addressed in 2a439a5, but the CI matrix still fails on two pre-existing main-branch issues: * mypy: context.py:64 passed `kind: str` into ContextItem.type, which is Literal["claim","page","entity","relation","source"]. Narrow it with a typing.cast so strict mode accepts the search-backend output. * pytest: session_end() mutates a session that session_start() already wrote, but put_session() switched to exclusive create and rejects the second write. Add KBStore.update_session() mirroring update_claim and have session_end call it; test_sessions coverage passes again. No behavior change for the path-traversal fix itself.
Resolve conflicts in context.py, sessions.py, storage.py (keep main). fix: block bad writes in import_apply per review feedback (#13) import_apply now captures schema validation issues and skips the file instead of passing a throwaway list.
fix(cli): translate domain errors into clean ClickException output
fix(bundle): reject path traversal in import (CVE-2007-4559, #9)
fix(verify): catch ArtifactNotFoundError on missing stored content
Approved design for feat/semantic-search. Embedding (sentence- transformers all-mpnet-base-v2) becomes the primary search backend with FTS5 as fallback. Synchronous-at-write indexing across all six artifact types (claim, page, source, entity, relation, evidence). Maximally functional scope (~3000 LOC): pluggable model adapter registry, sqlite-vec ANN with NumPy fallback, cross-encoder rerank, HyDE query expansion, ingest-time duplicate detection, model-identity migration, recall/MRR/nDCG eval harness, and full CLI/MCP/JSONL parity. The writing-plans step will turn the rollout order in section 16 into concrete phased tasks.
32-task TDD plan for the semantic-search feature: foundation, storage, write hooks across all six artifact types, semantic-primary search integration in MCP/JSONL/CLI, RRF fusion + hybrid, cross-encoder rerank, HyDE expansion, ingest-time duplicate detection, model-identity migration, recall/MRR/nDCG scorer, end-to-end integration test, and user docs. Each task: failing test, minimal implementation, passing test, commit. MockEmbedder test double keeps the unit suite fast; the real model is exercised only under @pytest.mark.integration. The evaluation module is named scorer.py rather than eval.py to avoid shadowing the Python builtin and to keep static analysers quiet; the user-facing CLI subcommand remains `vouch eval embedding` (the Click group is registered under the name "eval", with the Python identifier eval_group).
CI ran ruff which flagged SIM105 on the three try/except ImportError guard blocks in src/vouch/embeddings/__init__.py. Replace each with `contextlib.suppress(ImportError)` -- same semantics, satisfies ruff. Also add mypy overrides for numpy / sqlite_vec / sentence_transformers / fastembed so the type check passes in the base [dev] CI install where those optional extras aren't present. The embedding code paths that import these libraries are only reached when the extras are installed at runtime; for the static type check the missing stubs are noise.
CI runs `pip install -e '.[dev]'` which deliberately excludes the
optional `[embeddings]` extras. tests/embeddings/test_*.py modules
import numpy at top level, so pytest collection fails with
ModuleNotFoundError before any test can be deselected.
Add tests/embeddings/conftest.py with `pytest.importorskip("numpy")`
so the entire embeddings test directory skips gracefully when numpy
is absent. Once `pip install vouch[embeddings]` (or numpy itself) is
present, the skip is a no-op and the tests run normally.
`_validate_content` in bundle.py uses the path's first directory component to look up a Pydantic validator. For sources, both `sources/<sha>/meta.yaml` (the Source model) and `sources/<sha>/content` (raw opaque bytes) hit the same "sources" key, so the validator was being run on the raw content bytes and failing with "1 validation error for Source". Add an early return for any non-meta.yaml path under `sources/` so opaque content bytes are not Pydantic-validated. Only the Source metadata file is checked, which matches the original intent of the PR #13 validation. Unblocks three pre-existing test_bundle.py failures inherited from upstream main.
full docstrings for every exposed tool are paid on every turn. under minimal and standard, trim each tool's description to its first line (full keeps the complete docstrings), cutting the per-turn context cost.
note VOUCH_TOOL_PROFILE / mcp.tool_profile (default minimal, values minimal|standard|full) in the transports reference and the claude-code adapter guide; this repo has no mintlify/ tree yet so both live under docs/ and adapters/claude-code/ instead.
_dedupe_near_duplicates sorted by score and returned that score-sorted order, which fought graph-expansion: build_context_pack appends decayed-score neighbours after the ranked hits, and fused primary hits now carry small rrf scores. re-sorting let an irrelevant depth-2 neighbour outrank a real match, and the max_chars tail-pop then evicted the real match instead of the neighbour. keep the keep-decision in descending-score order so the highest-scored member of a near-duplicate cluster still survives, but return survivors in the caller's original order so budget eviction drops the tail (appended neighbours), not the ranked hits.
summarize this branch's user-visible changes under [Unreleased]: the minimal-by-default mcp tool profile, rrf fusion for auto/hybrid retrieval, and the per-prompt auto-recall hook in the claude-code adapter.
merges the friendlier-mcp-surface feature into the test staging branch: minimal mcp tool profile by default, fusion-by-default retrieval with near-duplicate suppression, per-prompt auto-recall hook, compact tool descriptions, and a real mcp<->methods parity test. also pulls in the 8 main commits the test branch was missing (our base is current main). changelog and test_capabilities conflicts resolved by keeping both sides.
…rofile install-mcp now writes a fourth hook (UserPromptSubmit -> vouch context-hook), and recall fires per prompt, not only at session start. also note the kb.* mcp surface defaults to a lean profile that widens via VOUCH_TOOL_PROFILE.
the host-compat drift check (#237) read openclaw.compat.pluginApi from openclaw.plugin.json. that block moved to package.json when the plugin packaging was split, and openclaw.* is now banned from the manifest (enforced by test_manifest_carries_no_dead_dialect_fields). the #237 reader was left pointing at the old, now-empty location, so host_compat silently degraded to {} and both host-compat assertions failed on the test branch — which every open pr targeting test inherited, including #413. repoint _load_host_compat and the test helpers at package.json. the openclaw.compat.pluginApi key path is identical in both files, so this is purely a source-file change with no behaviour difference.
github-actions
Bot
added
docs
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (75)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
these two directories were committed by accident and reach every first-time contributor on clone. neither is project source: - web/ is the netlify-deployed marketing landing page. it is shipped via the netlify cli and is not meant to live in the repo tree. - .claude/ is a personal claude code config: a settings.json with owner-specific hooks, plus command files that are byte-identical duplicates of adapters/claude-code/.claude/commands/. untrack both (files stay on disk) and gitignore them, anchored to the repo root so src/vouch/web and adapters/claude-code/.claude stay tracked. also ignore coverage.xml and the .netlify/ and .playwright-mcp/ tool-scratch dirs.
github-actions
Bot
added
embeddings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Why
What might break
VEP
Tests
make checkpasses locally (lint + mypy + pytest)CHANGELOG.mdupdated under## [Unreleased]