Fix 420#421
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.
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.
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.
📝 WalkthroughWalkthroughThis PR hardens bundle, storage, retrieval, health, and session flows; expands MCP/JSONL tooling and embeddings support; removes the desktop Electron app; and adds CI, vouch metadata, captures, and docs updates. ChangesCore Vouch Backend Expansion
Desktop Electron App Removal
Repository Metadata, Docs, and CI Housekeeping
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant BundleImport
participant Manifest
participant Store
Client->>BundleImport: import_check(bundle)
BundleImport->>Manifest: verify member sha256
BundleImport->>BundleImport: check graph integrity
BundleImport->>BundleImport: check source content-addressing
Client->>BundleImport: import_apply(bundle)
BundleImport->>Manifest: re-verify expected sha at write time
BundleImport->>Store: write validated members
sequenceDiagram
participant Client
participant Server
participant JsonlServer
participant IndexDb
Client->>Server: kb_search / kb_context
Server->>IndexDb: fetch embeddings / search results
Server->>Server: apply viewer scope and fusion
Client->>JsonlServer: kb.search / kb.context
JsonlServer->>IndexDb: backend lookup / filters
JsonlServer-->>Client: structured result with backend and viewer
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
github-actions
Bot
added
docs
Add back KB_FORMAT_VERSION, SCHEMA_VERSION, SCHEMA_VERSION_FILENAME, _starter_config to storage.py and _RETRACTED_CLAIM_STATUSES to context.py to fix import errors in test suite.
github-actions
Bot
added
retrieval
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/security-audit.yml (2)
36-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider pinning
pip-auditversion.Installing
pip-auditunpinned means CLI behavior/output format could change unexpectedly between runs, affecting reproducibility of this scheduled job.♻️ Proposed fix
- python -m pip install --upgrade pip pip-audit + python -m pip install --upgrade pip "pip-audit==2.7.3"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/security-audit.yml at line 36, The security-audit workflow installs pip-audit without a fixed version, so its behavior can change between runs. Update the pip install step in the security-audit workflow to pin pip-audit to a specific version, and keep the existing upgrade flow otherwise unchanged so the job remains reproducible.
39-40: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo persisted output for a continue-on-error job.
Since the job never fails the build, findings are only visible by manually checking Actions logs. Consider writing results to the job summary (
$GITHUB_STEP_SUMMARY) or uploading as an artifact so newly-disclosed CVEs aren't missed.♻️ Proposed fix
- name: audit - run: pip-audit --desc + run: pip-audit --desc | tee "$GITHUB_STEP_SUMMARY"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/security-audit.yml around lines 39 - 40, The security-audit workflow’s audit job currently runs with continue-on-error but does not persist its findings anywhere, so results can be missed unless the Actions log is checked manually. Update the audit step in the workflow to save pip-audit output to the job summary via $GITHUB_STEP_SUMMARY or upload it as an artifact, and reference the existing audit step/job so the results remain visible even when the build does not fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl:
- Line 71: The captured command in the JSONL entry leaks a local credential
path, so redact the hardcoded token file location from the recorded shell
command before committing. Update the logged command content in the capture data
so it no longer references the workstation-specific path, keeping the rest of
the command and the token-loading logic intact.
In
@.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md:
- Around line 24-39: Redact the workstation-specific absolute paths listed in
the session note before committing it. Update the entries in the modified-files
list that reference /home/a/... and /tmp/... to use repo-relative paths or a
generic redacted placeholder, keeping the rest of the session summary intact.
Locate the affected content in the session note section that enumerates files
modified this session and ensure only portable paths remain.
In `@docs/superpowers/specs/2026-07-06-review-ui-multi-kb-design.md`:
- Around line 4-9: The status line in the spec is too workstation-specific
because it hardcodes a local home directory path. Update the opening status text
in the document to use a repo-relative reference or remove the path entirely,
keeping the branch/reference wording intact and portable for shared docs.
---
Nitpick comments:
In @.github/workflows/security-audit.yml:
- Line 36: The security-audit workflow installs pip-audit without a fixed
version, so its behavior can change between runs. Update the pip install step in
the security-audit workflow to pin pip-audit to a specific version, and keep the
existing upgrade flow otherwise unchanged so the job remains reproducible.
- Around line 39-40: The security-audit workflow’s audit job currently runs with
continue-on-error but does not persist its findings anywhere, so results can be
missed unless the Actions log is checked manually. Update the audit step in the
workflow to save pip-audit output to the job summary via $GITHUB_STEP_SUMMARY or
upload it as an artifact, and reference the existing audit step/job so the
results remain visible even when the build does not fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 86299273-9b16-43e0-ae01-24514a03c6bd
⛔ Files ignored due to path filters (7)
desktop/docs/screenshots/browse.pngis excluded by!**/*.pngdesktop/docs/screenshots/dashboard.pngis excluded by!**/*.pngdesktop/docs/screenshots/dual-solve.pngis excluded by!**/*.pngdesktop/docs/screenshots/review.pngis excluded by!**/*.pngdesktop/package-lock.jsonis excluded by!**/package-lock.jsondocs/banner.svgis excluded by!**/*.svgdocs/img/examples/decision-log-search.svgis excluded by!**/*.svg
📒 Files selected for processing (119)
.github/workflows/security-audit.yml.pre-commit-config.yaml.vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl.vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl.vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl.vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl.vouch/claims/vouch-starter-reviewed-knowledge.yaml.vouch/decided/20260707-092531-154cb163.yaml.vouch/decided/20260707-093005-05345f47.yaml.vouch/decided/20260707-093125-4d6972f9.yaml.vouch/pages/edit-in-obsidian.md.vouch/pages/reviewed-knowledge-store.md.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md.vouch/schema_version.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yamlREADME.mddesktop/.gitignoredesktop/CHANGELOG.mddesktop/LICENSEdesktop/README.mddesktop/docs/architecture.mddesktop/docs/reports/2026-06-26.mddesktop/electron-builder.ymldesktop/electron.vite.config.tsdesktop/package.jsondesktop/resources/vouch/.gitkeepdesktop/scripts/dev-with-vouch.tsdesktop/scripts/gen-methods.tsdesktop/src/catalog/backend.jsondesktop/src/catalog/methods.jsondesktop/src/main/http-client.tsdesktop/src/main/index.tsdesktop/src/main/ipc.tsdesktop/src/main/jsonl-client.tsdesktop/src/main/kb-store.tsdesktop/src/main/supervisor.tsdesktop/src/main/tray.tsdesktop/src/main/types.tsdesktop/src/main/vouch-locator.tsdesktop/src/preload/index.tsdesktop/src/renderer/index.htmldesktop/src/renderer/src/App.tsxdesktop/src/renderer/src/app.cssdesktop/src/renderer/src/components/Diff.tsxdesktop/src/renderer/src/components/Drawer.tsxdesktop/src/renderer/src/components/EmptyState.tsxdesktop/src/renderer/src/components/MethodCard.tsxdesktop/src/renderer/src/components/MethodForm.tsxdesktop/src/renderer/src/components/Placeholder.tsxdesktop/src/renderer/src/components/Rail.tsxdesktop/src/renderer/src/components/StatusBar.tsxdesktop/src/renderer/src/components/Topbar.tsxdesktop/src/renderer/src/components/controls/index.tsxdesktop/src/renderer/src/components/results/Cards.tsxdesktop/src/renderer/src/components/results/JsonTree.tsxdesktop/src/renderer/src/components/results/Renderers.tsxdesktop/src/renderer/src/components/results/ResultView.tsxdesktop/src/renderer/src/components/results/atoms.tsxdesktop/src/renderer/src/global.d.tsdesktop/src/renderer/src/lib/VouchContext.tsxdesktop/src/renderer/src/lib/client.tsdesktop/src/renderer/src/lib/format.tsdesktop/src/renderer/src/lib/useOnOpen.tsxdesktop/src/renderer/src/lib/useVouchEvents.tsdesktop/src/renderer/src/main.tsxdesktop/src/renderer/src/views/Dashboard.tsxdesktop/src/renderer/src/views/DualSolve.tsxdesktop/src/renderer/src/views/GenericView.tsxdesktop/src/renderer/src/views/Review.tsxdesktop/src/renderer/src/views/blurbs.tsdesktop/src/renderer/src/views/registry.tsxdesktop/src/shared/ipc.tsdesktop/src/shared/methods.gen.tsdesktop/src/shared/methods.types.tsdesktop/test/catalog.test.tsdesktop/test/controls.test.tsxdesktop/test/gen-methods.test.tsdesktop/test/http-client.test.tsdesktop/test/jsonl-client.test.tsdesktop/test/method-card.test.tsxdesktop/test/method-form.test.tsxdesktop/test/method-gate.test.tsdesktop/test/smoke-jsonl.tsdesktop/test/vouch-locator.test.tsdesktop/tsconfig.jsondesktop/tsconfig.node.jsondesktop/tsconfig.scripts.jsondesktop/tsconfig.web.jsondesktop/vitest.config.tsdocs/superpowers/specs/2026-07-06-review-ui-multi-kb-design.mdmigrations/README.mdspec/2026-05-21/README.mdspec/2026-05-21/SPEC.mdspec/2026-05-21/audit-vocabulary.mdspec/2026-05-21/methods.mdspec/2026-05-21/retrieval.mdspec/2026-05-21/review-gate.mdspec/2026-05-21/transports.mdspec/README.mdspec/audit-vocabulary.mdspec/methods.mdspec/retrieval.mdspec/review-gate.mdspec/transports.mdsrc/vouch/bundle.pysrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/embeddings/base.pysrc/vouch/health.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/storage.pytests/embeddings/conftest.pytests/embeddings/test_integration.pytests/embeddings/test_search.pytests/test_cli.py
💤 Files with no reviewable changes (87)
- spec/2026-05-21/README.md
- migrations/README.md
- desktop/test/method-gate.test.ts
- spec/audit-vocabulary.md
- desktop/LICENSE
- desktop/src/renderer/src/components/Rail.tsx
- spec/2026-05-21/SPEC.md
- desktop/src/renderer/src/app.css
- desktop/electron.vite.config.ts
- desktop/README.md
- desktop/src/main/index.ts
- desktop/src/main/kb-store.ts
- desktop/src/renderer/src/views/Review.tsx
- spec/methods.md
- spec/2026-05-21/audit-vocabulary.md
- desktop/src/renderer/src/components/Placeholder.tsx
- desktop/src/renderer/src/components/Topbar.tsx
- desktop/vitest.config.ts
- spec/review-gate.md
- spec/transports.md
- desktop/src/renderer/index.html
- desktop/src/renderer/src/views/GenericView.tsx
- desktop/src/renderer/src/components/StatusBar.tsx
- desktop/src/renderer/src/components/EmptyState.tsx
- desktop/src/shared/methods.types.ts
- desktop/test/gen-methods.test.ts
- desktop/src/renderer/src/App.tsx
- desktop/src/renderer/src/components/results/Renderers.tsx
- desktop/src/main/jsonl-client.ts
- desktop/tsconfig.node.json
- desktop/tsconfig.json
- desktop/src/renderer/src/main.tsx
- desktop/src/main/tray.ts
- desktop/src/shared/ipc.ts
- desktop/.gitignore
- desktop/package.json
- desktop/src/renderer/src/components/results/ResultView.tsx
- desktop/docs/reports/2026-06-26.md
- desktop/test/http-client.test.ts
- desktop/src/main/types.ts
- .pre-commit-config.yaml
- spec/2026-05-21/methods.md
- desktop/tsconfig.scripts.json
- desktop/src/preload/index.ts
- spec/retrieval.md
- desktop/src/renderer/src/components/Diff.tsx
- desktop/src/renderer/src/views/DualSolve.tsx
- desktop/src/renderer/src/components/results/JsonTree.tsx
- desktop/src/renderer/src/lib/format.ts
- desktop/scripts/gen-methods.ts
- desktop/src/renderer/src/views/registry.tsx
- desktop/test/catalog.test.ts
- desktop/src/shared/methods.gen.ts
- desktop/src/renderer/src/views/blurbs.ts
- desktop/tsconfig.web.json
- desktop/docs/architecture.md
- desktop/src/catalog/backend.json
- desktop/test/vouch-locator.test.ts
- desktop/src/renderer/src/components/controls/index.tsx
- desktop/src/main/ipc.ts
- desktop/test/method-card.test.tsx
- spec/2026-05-21/retrieval.md
- desktop/src/renderer/src/lib/useVouchEvents.ts
- desktop/test/method-form.test.tsx
- desktop/scripts/dev-with-vouch.ts
- desktop/test/smoke-jsonl.ts
- desktop/electron-builder.yml
- desktop/src/renderer/src/lib/VouchContext.tsx
- desktop/src/renderer/src/global.d.ts
- spec/README.md
- desktop/CHANGELOG.md
- desktop/src/renderer/src/components/Drawer.tsx
- desktop/src/renderer/src/components/MethodCard.tsx
- desktop/src/renderer/src/components/MethodForm.tsx
- spec/2026-05-21/transports.md
- desktop/src/renderer/src/components/results/atoms.tsx
- desktop/src/main/supervisor.ts
- desktop/src/renderer/src/components/results/Cards.tsx
- desktop/src/renderer/src/lib/client.ts
- desktop/src/renderer/src/lib/useOnOpen.tsx
- desktop/test/jsonl-client.test.ts
- desktop/src/renderer/src/views/Dashboard.tsx
- desktop/test/controls.test.tsx
- desktop/src/main/http-client.ts
- desktop/src/catalog/methods.json
- desktop/src/main/vouch-locator.ts
- spec/2026-05-21/review-gate.md
| {"cmd": "echo \"=== active gh account ===\" && gh api user --jq .login; echo \"=== is plind-junior now a PUBLIC member of vouchdev? ===\" && gh api orgs/vouchdev/public_members --jq '.[].login' 2>&1 | head -20; ec", "summary": "Ran: echo \"=== active gh account ===\" && gh api user --jq .login;", "tool": "Bash", "ts": 1783420221.0744834} | ||
| {"cmd": "echo \"=== publish ===\" && mcp-publisher publish 2>&1 | tail -15", "summary": "Command failed: echo \"=== publish ===\" && mcp-publisher publish 2>&1 | tail ", "tool": "Bash", "ts": 1783420292.7722068} | ||
| {"cmd": "echo \"=== look for mcp-publisher token files ===\" && find / -maxdepth 6 -name \"*mcp-publisher*\" -o -name \".mcpregistry*\" 2>/dev/null | head -20; echo \"=== any recently-modified dotfiles in likely spot", "summary": "Ran: echo \"=== look for mcp-publisher token files ===\" && find / ", "tool": "Bash", "ts": 1783420318.553565} | ||
| {"cmd": "python3 -c \"\nimport json, base64, time\nd = json.load(open('/home/a/.config/mcp-publisher/token.json'))\nprint('keys in token.json:', list(d.keys()))\ntok = d.get('token') or d.get('access_token') or ''\n", "summary": "Ran: python3 -c \"", "tool": "Bash", "ts": 1783420336.7329283} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the local credential path from the captured command.
/home/a/.config/mcp-publisher/token.json exposes the workstation layout and auth setup even though the token contents are not shown. Redact the path before committing this log.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl at line 71, The
captured command in the JSONL entry leaks a local credential path, so redact the
hardcoded token file location from the recorded shell command before committing.
Update the logged command content in the capture data so it no longer references
the workstation-specific path, keeping the rest of the command and the
token-loading logic intact.
Comment on lines
+24
to
+39
| ## files modified this session | ||
|
|
||
| - .gitignore | ||
| - .vouch/.gitignore | ||
| - .vouch/audit.log.jsonl | ||
| - .vouch/claims/vouch-uses-a-review-gated-proposal-workflow-agents-propose-c.yaml | ||
| - .vouch/config.yaml | ||
| - .vouch/decided/20260521-055206-7d6d92d6.yaml | ||
| - .vouch/sources/06d8519f8dcf4149d23c8a48984541b2e9365ec364e7e58192e28ed149a2c47c/content | ||
| - .vouch/sources/06d8519f8dcf4149d23c8a48984541b2e9365ec364e7e58192e28ed149a2c47c/meta.yaml | ||
| - .vouch/sources/67478e72acfb8fac3a059143e95c95f5cc6f7e8d4dccc05fbcea8dbccb8a4eba/content | ||
| - .vouch/sources/67478e72acfb8fac3a059143e95c95f5cc6f7e8d4dccc05fbcea8dbccb8a4eba/meta.yaml | ||
| - /home/a/.claude/projects/-home-a-Dev-plind-junior-vouch/memory/MEMORY.md | ||
| - /home/a/.claude/projects/-home-a-Dev-plind-junior-vouch/memory/commit-scope-src-adapters.md | ||
| - /tmp/claude-1000/-home-a-Dev-plind-junior-vouch/e5ead202-24c2-4916-b59c-ddb4c123d63f/scratchpad/merge_hook.py | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact local filesystem paths before committing this session note.
The file captures /home/a/... and /tmp/... paths, which leak workstation-specific details and make the artifact non-portable. Replace them with repo-relative placeholders or redacted values.
🛡️ Suggested fix
- /home/a/.claude/projects/-home-a-Dev-plind-junior-vouch/memory/MEMORY.md
+ <redacted>/memory/MEMORY.mdAlso applies to: 63-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md
around lines 24 - 39, Redact the workstation-specific absolute paths listed in
the session note before committing it. Update the entries in the modified-files
list that reference /home/a/... and /tmp/... to use repo-relative paths or a
generic redacted placeholder, keeping the rest of the session summary intact.
Locate the affected content in the session note section that enumerates files
modified this session and ensure only portable paths remain.
Comment on lines
+4
to
+9
| status: SHIPPED on `feat/multi-project-aggregation` in ~/Dev/plind-junior/vouch-ui | ||
| (repo vouchdev/webApp). an earlier draft of this spec targeted vouch's built-in | ||
| `review-ui` (`src/vouch/web/`); the user redirected — the vouch repo's web | ||
| folder stays untouched, aggregation lives in the separate vouch-ui console. | ||
| that server-side branch survives unpushed as `feat/review-ui-multi-kb` in the | ||
| vouch repo, reference only. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the status line repo-portable.
~/Dev/plind-junior/vouch-ui is workstation-specific and will age poorly in shared docs. Prefer a repo-relative note or drop the path entirely.
Suggested fix
-status: SHIPPED on `feat/multi-project-aggregation` in ~/Dev/plind-junior/vouch-ui
+status: SHIPPED on `feat/multi-project-aggregation` in vouch-ui📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| status: SHIPPED on `feat/multi-project-aggregation` in ~/Dev/plind-junior/vouch-ui | |
| (repo vouchdev/webApp). an earlier draft of this spec targeted vouch's built-in | |
| `review-ui` (`src/vouch/web/`); the user redirected — the vouch repo's web | |
| folder stays untouched, aggregation lives in the separate vouch-ui console. | |
| that server-side branch survives unpushed as `feat/review-ui-multi-kb` in the | |
| vouch repo, reference only. | |
| status: SHIPPED on `feat/multi-project-aggregation` in vouch-ui | |
| (repo vouchdev/webApp). an earlier draft of this spec targeted vouch's built-in | |
| `review-ui` (`src/vouch/web/`); the user redirected — the vouch repo's web | |
| folder stays untouched, aggregation lives in the separate vouch-ui console. | |
| that server-side branch survives unpushed as `feat/review-ui-multi-kb` in the | |
| vouch repo, reference only. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-06-review-ui-multi-kb-design.md` around lines
4 - 9, The status line in the spec is too workstation-specific because it
hardcodes a local home directory path. Update the opening status text in the
document to use a repo-relative reference or remove the path entirely, keeping
the branch/reference wording intact and portable for shared docs.
the test-branch merges (2ddd1f9 onward) took main's older, smaller storage.py / context.py / index_db.py / cli.py / server.py / health.py / jsonl_server.py / bundle.py / sessions.py while keeping the newer caller modules (lifecycle, proposals, graph, notify, sync, provenance, codex_rollout, triage). that desync left 31 mypy errors — callers referencing methods the truncated backing no longer defined (put_relation_idempotent, _validate_claim_refs, update_page, update_proposal, index_prov_edge, get_meta) — so the test job failed at mypy before pytest ever ran. restore src/ and tests/ to bb06565, the last coherent snapshot, where the fuller backing matches the callers. this also brings back the reindex cli command the recall-eval job invokes (the truncated cli only had index), fixing the second red job. keep __version__ at 1.2.2 so the four version sites stay in lockstep.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/health.py`:
- Around line 568-626: `rebuild_index` defines `_tick` and documents phase
labels, but never actually emits them. Call `_tick("claims")`, `_tick("pages")`,
`_tick("entities")`, and `_tick("embeddings")` at the start of each
corresponding stage in `rebuild_index`, including before
`_rebuild_embeddings(store)`, so the `on_progress` callback is exercised and the
CLI progress display receives updates.
- Around line 629-651: _rebuild_embeddings currently writes vectors into the
wrong table/index, so semantic search stays empty after reset. Update
_rebuild_embeddings in health.py to use the same storage path as
search_semantic/search_embedding and reset, specifically by indexing into
embedding_index via the existing index_db helper instead of embeddings, so
rebuilt vectors are immediately searchable.
In `@src/vouch/index_db.py`:
- Around line 170-177: The _snippet_for helper is using the raw kind segment to
build paths, but the on-disk layout uses the artifact-specific resolver and
pluralized directories, so it misses files like sources/<id>/meta.yaml and falls
back to eid. Update _snippet_for in src/vouch/index_db.py to resolve the correct
artifact-specific base path before checking for YAML/MD snippets, and keep the
fallback behavior only when that resolved path truly does not exist.
In `@src/vouch/jsonl_server.py`:
- Around line 713-726: The actor attribution in _h_propose_theme is bypassing
the request-scoped _actor ContextVar by reading p.get("agent") and VOUCH_AGENT
directly. Update _h_propose_theme to use _agent() for the actor value so
propose_theme(store, cluster, proposed_by=...) records the same HTTP-request
actor as the other handlers in jsonl_server.py; keep the rest of the
ThemeCluster construction unchanged.
In `@src/vouch/server.py`:
- Line 1017: The actor attribution in the server flow is using a direct
environment lookup instead of the shared helper, which makes it inconsistent
with the other tool paths. Update the assignment in the relevant request
handling code to resolve `actor` via `_agent()` first, using `agent` as the
override if present, so `proposed_by` follows the same attribution logic as
`kb_propose_claim`, `kb_compile`, and `kb_reject_extracted`.
- Around line 853-864: The kb_reindex_embeddings tool currently ignores its
backfill and model arguments, so the migration always uses the default embedder.
Update kb_reindex_embeddings to either pass backfill/model through to
backfill_embeddings and the embedder selection path, or remove those parameters
from the tool signature if they are not meant to affect behavior. Make the same
change in jsonl_server’s equivalent reindex tool so both entry points behave
consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 185f1fdb-9cdb-40d7-bd7c-fef9e616721d
📒 Files selected for processing (14)
src/vouch/bundle.pysrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/embeddings/base.pysrc/vouch/health.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/storage.pytests/embeddings/conftest.pytests/embeddings/test_integration.pytests/embeddings/test_search.pytests/test_cli.py
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/embeddings/conftest.py
- src/vouch/embeddings/base.py
- tests/embeddings/test_search.py
- src/vouch/sessions.py
- tests/embeddings/test_integration.py
- src/vouch/context.py
- src/vouch/storage.py
Comment on lines
+568
to
626
| def rebuild_index(store: KBStore, *, on_progress: Callable[[str], None] | None = None) -> dict: | ||
| """Drop and rebuild state.db from the durable files. Idempotent. | ||
|
|
||
| `on_progress`, if given, is called with a short phase label ("claims", | ||
| "pages", "entities", "embeddings") as each stage starts — for CLI | ||
| progress display. It never affects the result. | ||
| """ | ||
|
|
||
| def _tick(phase: str) -> None: | ||
| if on_progress is not None: | ||
| on_progress(phase) | ||
|
|
||
| # Detect a stale embedding-model identity before reset() wipes the meta. | ||
| try: | ||
| from . import audit | ||
| from .embeddings.migration import detect_mismatch | ||
|
|
||
| m = detect_mismatch(store.kb_dir) | ||
| if m is not None: | ||
| audit.log_event( | ||
| store.kb_dir, | ||
| event="embedding.model_mismatch", | ||
| actor="vouch-health", | ||
| object_ids=[], | ||
| data=m, | ||
| ) | ||
| except ImportError: | ||
| pass | ||
| index_db.reset(store.kb_dir) | ||
| with index_db.open_db(store.kb_dir) as conn: | ||
| for c in store.list_claims(): | ||
| index_db.index_claim( | ||
| conn, id=c.id, text=c.text, | ||
| type=c.type.value, status=c.status.value, tags=c.tags, | ||
| conn, | ||
| id=c.id, | ||
| text=c.text, | ||
| type=c.type.value, | ||
| status=c.status.value, | ||
| tags=c.tags, | ||
| ) | ||
| for p in store.list_pages(): | ||
| index_db.index_page( | ||
| conn, id=p.id, title=p.title, body=p.body, | ||
| type=p.type.value, tags=p.tags, | ||
| conn, | ||
| id=p.id, | ||
| title=p.title, | ||
| body=p.body, | ||
| type=p.type, | ||
| tags=p.tags, | ||
| ) | ||
| for e in store.list_entities(): | ||
| index_db.index_entity( | ||
| conn, id=e.id, name=e.name, description=e.description, | ||
| type=e.type.value, aliases=e.aliases, | ||
| conn, | ||
| id=e.id, | ||
| name=e.name, | ||
| description=e.description, | ||
| type=e.type.value, | ||
| aliases=e.aliases, | ||
| ) | ||
| _rebuild_embeddings(store) | ||
| return index_db.stats(store.kb_dir) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
on_progress is never invoked — _tick is dead code.
_tick is defined but never called, so the documented phase labels ("claims", "pages", "entities", "embeddings") are never emitted and any CLI progress display stays silent.
🐛 Proposed fix to actually emit phase labels
index_db.reset(store.kb_dir)
with index_db.open_db(store.kb_dir) as conn:
+ _tick("claims")
for c in store.list_claims():
index_db.index_claim(
conn,
id=c.id,
text=c.text,
type=c.type.value,
status=c.status.value,
tags=c.tags,
)
+ _tick("pages")
for p in store.list_pages():
index_db.index_page(
conn,
id=p.id,
title=p.title,
body=p.body,
type=p.type,
tags=p.tags,
)
+ _tick("entities")
for e in store.list_entities():
index_db.index_entity(
conn,
id=e.id,
name=e.name,
description=e.description,
type=e.type.value,
aliases=e.aliases,
)
+ _tick("embeddings")
_rebuild_embeddings(store)
return index_db.stats(store.kb_dir)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def rebuild_index(store: KBStore, *, on_progress: Callable[[str], None] | None = None) -> dict: | |
| """Drop and rebuild state.db from the durable files. Idempotent. | |
| `on_progress`, if given, is called with a short phase label ("claims", | |
| "pages", "entities", "embeddings") as each stage starts — for CLI | |
| progress display. It never affects the result. | |
| """ | |
| def _tick(phase: str) -> None: | |
| if on_progress is not None: | |
| on_progress(phase) | |
| # Detect a stale embedding-model identity before reset() wipes the meta. | |
| try: | |
| from . import audit | |
| from .embeddings.migration import detect_mismatch | |
| m = detect_mismatch(store.kb_dir) | |
| if m is not None: | |
| audit.log_event( | |
| store.kb_dir, | |
| event="embedding.model_mismatch", | |
| actor="vouch-health", | |
| object_ids=[], | |
| data=m, | |
| ) | |
| except ImportError: | |
| pass | |
| index_db.reset(store.kb_dir) | |
| with index_db.open_db(store.kb_dir) as conn: | |
| for c in store.list_claims(): | |
| index_db.index_claim( | |
| conn, id=c.id, text=c.text, | |
| type=c.type.value, status=c.status.value, tags=c.tags, | |
| conn, | |
| id=c.id, | |
| text=c.text, | |
| type=c.type.value, | |
| status=c.status.value, | |
| tags=c.tags, | |
| ) | |
| for p in store.list_pages(): | |
| index_db.index_page( | |
| conn, id=p.id, title=p.title, body=p.body, | |
| type=p.type.value, tags=p.tags, | |
| conn, | |
| id=p.id, | |
| title=p.title, | |
| body=p.body, | |
| type=p.type, | |
| tags=p.tags, | |
| ) | |
| for e in store.list_entities(): | |
| index_db.index_entity( | |
| conn, id=e.id, name=e.name, description=e.description, | |
| type=e.type.value, aliases=e.aliases, | |
| conn, | |
| id=e.id, | |
| name=e.name, | |
| description=e.description, | |
| type=e.type.value, | |
| aliases=e.aliases, | |
| ) | |
| _rebuild_embeddings(store) | |
| return index_db.stats(store.kb_dir) | |
| def rebuild_index(store: KBStore, *, on_progress: Callable[[str], None] | None = None) -> dict: | |
| """Drop and rebuild state.db from the durable files. Idempotent. | |
| `on_progress`, if given, is called with a short phase label ("claims", | |
| "pages", "entities", "embeddings") as each stage starts — for CLI | |
| progress display. It never affects the result. | |
| """ | |
| def _tick(phase: str) -> None: | |
| if on_progress is not None: | |
| on_progress(phase) | |
| # Detect a stale embedding-model identity before reset() wipes the meta. | |
| try: | |
| from . import audit | |
| from .embeddings.migration import detect_mismatch | |
| m = detect_mismatch(store.kb_dir) | |
| if m is not None: | |
| audit.log_event( | |
| store.kb_dir, | |
| event="embedding.model_mismatch", | |
| actor="vouch-health", | |
| object_ids=[], | |
| data=m, | |
| ) | |
| except ImportError: | |
| pass | |
| index_db.reset(store.kb_dir) | |
| with index_db.open_db(store.kb_dir) as conn: | |
| _tick("claims") | |
| for c in store.list_claims(): | |
| index_db.index_claim( | |
| conn, | |
| id=c.id, | |
| text=c.text, | |
| type=c.type.value, | |
| status=c.status.value, | |
| tags=c.tags, | |
| ) | |
| _tick("pages") | |
| for p in store.list_pages(): | |
| index_db.index_page( | |
| conn, | |
| id=p.id, | |
| title=p.title, | |
| body=p.body, | |
| type=p.type, | |
| tags=p.tags, | |
| ) | |
| _tick("entities") | |
| for e in store.list_entities(): | |
| index_db.index_entity( | |
| conn, | |
| id=e.id, | |
| name=e.name, | |
| description=e.description, | |
| type=e.type.value, | |
| aliases=e.aliases, | |
| ) | |
| _tick("embeddings") | |
| _rebuild_embeddings(store) | |
| return index_db.stats(store.kb_dir) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/health.py` around lines 568 - 626, `rebuild_index` defines `_tick`
and documents phase labels, but never actually emits them. Call
`_tick("claims")`, `_tick("pages")`, `_tick("entities")`, and
`_tick("embeddings")` at the start of each corresponding stage in
`rebuild_index`, including before `_rebuild_embeddings(store)`, so the
`on_progress` callback is exercised and the CLI progress display receives
updates.
Comment on lines
+629
to
+651
| def _rebuild_embeddings(store: KBStore) -> None: | ||
| try: | ||
| from .embeddings import get_embedder | ||
|
|
||
| embedder = get_embedder() | ||
| except Exception: | ||
| return | ||
| with index_db.open_db(store.kb_dir) as conn: | ||
| texts: list[tuple[str, str, str]] = [] | ||
| for c in store.list_claims(): | ||
| texts.append(("claim", c.id, c.text)) | ||
| for p in store.list_pages(): | ||
| texts.append(("page", p.id, f"{p.title} {p.body}")) | ||
| for e in store.list_entities(): | ||
| texts.append(("entity", e.id, f"{e.name} {e.description or ''}")) | ||
| if not texts: | ||
| return | ||
| batch_size = 64 | ||
| for i in range(0, len(texts), batch_size): | ||
| batch = texts[i : i + batch_size] | ||
| vecs = embedder.encode_batch([t[2] for t in batch]) | ||
| for (kind, eid, _), row in zip(batch, vecs, strict=True): | ||
| index_db.index_embedding(conn, kind=kind, id=eid, vec=row.tolist()) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Which table does the live semantic search path read, and is the rebuild's target the same?
ast-grep outline src/vouch/index_db.py --items all
echo "--- search_embedding (singular) body ---"
rg -nP 'def search_embedding\b' -A40 src/vouch/index_db.py
echo "--- table reads ---"
rg -nP 'FROM embedding_index|FROM embeddings\b' src/vouch/index_db.py
echo "--- does _embed_and_store target embedding_index? ---"
rg -nP 'def _embed_and_store\b' -A40 src/vouch/storage.py
echo "--- is search_embeddings (plural) referenced anywhere? ---"
rg -nP '\bsearch_embeddings\b' src testsRepository: vouchdev/vouch
Length of output: 6253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- src/vouch/index_db.py: schema, reset, index_embedding, search_embeddings ---"
sed -n '1,180p' src/vouch/index_db.py
echo "--- src/vouch/index_db.py: get_embedding and search_embedding/search_semantic ---"
sed -n '321,500p' src/vouch/index_db.py
echo "--- src/vouch/health.py: _rebuild_embeddings ---"
sed -n '620,665p' src/vouch/health.pyRepository: vouchdev/vouch
Length of output: 13459
rebuild_embeddings needs to populate embedding_index, not embeddings.
search_semantic() goes through search_embedding(), and both read from embedding_index. reset() also clears embedding_index, so this rebuild currently leaves semantic search empty until the normal write hook repopulates it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/health.py` around lines 629 - 651, _rebuild_embeddings currently
writes vectors into the wrong table/index, so semantic search stays empty after
reset. Update _rebuild_embeddings in health.py to use the same storage path as
search_semantic/search_embedding and reset, specifically by indexing into
embedding_index via the existing index_db helper instead of embeddings, so
rebuilt vectors are immediately searchable.
Source: Linters/SAST tools
Comment on lines
+170
to
+177
| def _snippet_for(kb_dir: Path, kind: str, eid: str) -> str: | ||
| path = kb_dir / kind / f"{eid}.yaml" | ||
| if not path.exists(): | ||
| path = kb_dir / kind / f"{eid}.md" | ||
| if not path.exists(): | ||
| return eid | ||
| text = path.read_text(encoding="utf-8") | ||
| return text[:200].replace("\n", " ") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm _snippet_for is reachable and the on-disk dir names are plural
rg -nP '\bsearch_embeddings\b|\b_snippet_for\b' src tests
rg -nP 'SUBDIRS\s*=|kb_dir / "claims"|kb_dir / "pages"|kb_dir / "entities"' src/vouch/storage.pyRepository: vouchdev/vouch
Length of output: 601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/vouch/index_db.py (relevant section) ---'
sed -n '130,220p' src/vouch/index_db.py
echo
echo '--- src/vouch/storage.py (subdir constants and writers) ---'
sed -n '1,120p' src/vouch/storage.py
echo
sed -n '420,620p' src/vouch/storage.py
echo
echo '--- occurrences of kind assignments/usages ---'
rg -n '\bkind\b' src/vouch/index_db.py src/vouch/storage.py src -g '!**/__pycache__/**'Repository: vouchdev/vouch
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/vouch/index_db.py around search_embeddings and _snippet_for ---'
sed -n '142,190p' src/vouch/index_db.py
echo
echo '--- all call sites / producers of (kind, eid) in src/vouch ---'
rg -n '\b(search_embeddings|_snippet_for|kind\s*[:=])\b' src/vouch -g '!**/__pycache__/**'
echo
echo '--- storage subdir-related lines ---'
rg -n '"claims"|"pages"|"entities"|"sources"|"relations"|"evidence"|SUBDIRS' src/vouch/storage.pyRepository: vouchdev/vouch
Length of output: 5031
Use the artifact-specific path resolver here. kind is singular, while the on-disk layout uses plural dirs and sources/<id>/meta.yaml, so kb_dir / kind / ... misses the snippet files and falls back to the bare id. src/vouch/index_db.py:170-177
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/index_db.py` around lines 170 - 177, The _snippet_for helper is
using the raw kind segment to build paths, but the on-disk layout uses the
artifact-specific resolver and pluralized directories, so it misses files like
sources/<id>/meta.yaml and falls back to eid. Update _snippet_for in
src/vouch/index_db.py to resolve the correct artifact-specific base path before
checking for YAML/MD snippets, and keep the fallback behavior only when that
resolved path truly does not exist.
Comment on lines
+713
to
+726
| def _h_propose_theme(p: dict) -> dict: | ||
| from . import themes | ||
|
|
||
| store = _store() | ||
| actor = p.get("agent") or os.environ.get("VOUCH_AGENT", "unknown-agent") | ||
| cluster = themes.ThemeCluster( | ||
| entities=p["entities"], | ||
| claim_ids=p["claim_ids"], | ||
| session_ids=p.get("session_ids", []), | ||
| score=float(p.get("score", 0.0)), | ||
| session_count=len(p.get("session_ids", [])), | ||
| claim_count=len(p["claim_ids"]), | ||
| ) | ||
| return themes.propose_theme(store, cluster, proposed_by=actor) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use _agent() for actor attribution instead of reading the env directly.
Line 717 resolves the actor as p.get("agent") or os.environ.get("VOUCH_AGENT", ...), bypassing the request-scoped _actor ContextVar that _agent() consults. Over the HTTP transport the actor is set from X-Vouch-Agent into _actor; this handler ignores it, so themes.propose_theme(..., proposed_by=actor) records the wrong actor in the audit log. Every other handler in this file uses _agent().
🛡️ Proposed fix
- actor = p.get("agent") or os.environ.get("VOUCH_AGENT", "unknown-agent")
+ actor = p.get("agent") or _agent()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _h_propose_theme(p: dict) -> dict: | |
| from . import themes | |
| store = _store() | |
| actor = p.get("agent") or os.environ.get("VOUCH_AGENT", "unknown-agent") | |
| cluster = themes.ThemeCluster( | |
| entities=p["entities"], | |
| claim_ids=p["claim_ids"], | |
| session_ids=p.get("session_ids", []), | |
| score=float(p.get("score", 0.0)), | |
| session_count=len(p.get("session_ids", [])), | |
| claim_count=len(p["claim_ids"]), | |
| ) | |
| return themes.propose_theme(store, cluster, proposed_by=actor) | |
| def _h_propose_theme(p: dict) -> dict: | |
| from . import themes | |
| store = _store() | |
| actor = p.get("agent") or _agent() | |
| cluster = themes.ThemeCluster( | |
| entities=p["entities"], | |
| claim_ids=p["claim_ids"], | |
| session_ids=p.get("session_ids", []), | |
| score=float(p.get("score", 0.0)), | |
| session_count=len(p.get("session_ids", [])), | |
| claim_count=len(p["claim_ids"]), | |
| ) | |
| return themes.propose_theme(store, cluster, proposed_by=actor) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/jsonl_server.py` around lines 713 - 726, The actor attribution in
_h_propose_theme is bypassing the request-scoped _actor ContextVar by reading
p.get("agent") and VOUCH_AGENT directly. Update _h_propose_theme to use _agent()
for the actor value so propose_theme(store, cluster, proposed_by=...) records
the same HTTP-request actor as the other handlers in jsonl_server.py; keep the
rest of the ThemeCluster construction unchanged.
Comment on lines
+853
to
+864
| @mcp.tool() | ||
| def kb_reindex_embeddings( | ||
| *, backfill: bool = False, force: bool = False, model: str | None = None, | ||
| ) -> dict[str, Any]: | ||
| """Re-encode every artifact under the current embedding adapter.""" | ||
| from .embeddings.migration import backfill_embeddings | ||
| store = _store() | ||
| if model: | ||
| from .embeddings import get_embedder | ||
| get_embedder(model) | ||
| n = backfill_embeddings(store, force=force) | ||
| return {"touched": n, "model": _current_model_name()} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep --pattern 'def backfill_embeddings($$$)' --lang python src/vouch/embeddings/migration.py
rg -nP 'def backfill_embeddings' -A6 src/vouch/embeddings/migration.pyRepository: vouchdev/vouch
Length of output: 3142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/vouch/server.py around kb_reindex_embeddings ---'
sed -n '840,875p' src/vouch/server.py
echo
echo '--- src/vouch/jsonl_server.py around reindex handler ---'
rg -n "reindex_embeddings|backfill_embeddings|kb_reindex_embeddings|_h_reindex_embeddings" -A8 -B4 src/vouch/jsonl_server.py
echo
echo '--- src/vouch/cli.py around reindex command ---'
rg -n "reindex_embeddings|backfill_embeddings|kb_reindex_embeddings|reindex" -A8 -B4 src/vouch/cli.py
echo
echo '--- src/vouch/capabilities.py around reindex capability ---'
rg -n "reindex_embeddings|backfill_embeddings|kb_reindex_embeddings|reindex" -A8 -B4 src/vouch/capabilities.pyRepository: vouchdev/vouch
Length of output: 4319
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- src/vouch/embeddings/__init__.py (or provider) get_embedder definition ---'
rg -n "def get_embedder|_current_model_name|current_model|embedder" -A30 -B8 src/vouch/embeddings
echo
echo '--- locate get_embedder callers in migration/server ---'
rg -n "get_embedder\(" -A3 -B3 src/vouchRepository: vouchdev/vouch
Length of output: 20566
backfill and model are no-ops in kb_reindex_embeddings (src/vouch/server.py:853-864). backfill_embeddings() always runs with the default embedder, so these arguments don’t affect the result; src/vouch/jsonl_server.py has the same gap. Either pass them through to the migration path or remove them from the tool signature.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/server.py` around lines 853 - 864, The kb_reindex_embeddings tool
currently ignores its backfill and model arguments, so the migration always uses
the default embedder. Update kb_reindex_embeddings to either pass backfill/model
through to backfill_embeddings and the embedder selection path, or remove those
parameters from the tool signature if they are not meant to affect behavior.
Make the same change in jsonl_server’s equivalent reindex tool so both entry
points behave consistently.
| """ | ||
| from . import themes | ||
| store = _store() | ||
| actor = agent or os.environ.get("VOUCH_AGENT", "unknown-agent") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Prefer _agent() over a direct env read for actor attribution.
actor = agent or os.environ.get("VOUCH_AGENT", "unknown-agent") diverges from the other tools here (kb_propose_claim, kb_compile, kb_reject_extracted) which resolve the actor via _agent(). Falling back to _agent() keeps proposed_by attribution consistent across tools.
🛠️ Proposed fix
- actor = agent or os.environ.get("VOUCH_AGENT", "unknown-agent")
+ actor = agent or _agent()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| actor = agent or os.environ.get("VOUCH_AGENT", "unknown-agent") | |
| actor = agent or _agent() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/server.py` at line 1017, The actor attribution in the server flow
is using a direct environment lookup instead of the shared helper, which makes
it inconsistent with the other tool paths. Update the assignment in the relevant
request handling code to resolve `actor` via `_agent()` first, using `agent` as
the override if present, so `proposed_by` follows the same attribution logic as
`kb_propose_claim`, `kb_compile`, and `kb_reject_extracted`.
This was referenced Jul 7, 2026
Open
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]Summary by CodeRabbit