Test#420
Open
plind-junior wants to merge 157 commits into
Open
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 webapp's review console fans out to kb.list_sessions on load; the method never existed, so the jsonl dispatcher returned method_not_found and the whole view failed to render. add it across all four surfaces (mcp, jsonl, capabilities, cli), returning the session-review pipeline: open capture buffers awaiting a summary (stage "buffer") and filed summary proposals awaiting review (stage "pending"), newest first. also extend kb.summarize_session's return additively with the webapp-facing keys session_id / summarized / proposal_id / skipped, so the review console's summarize action reads a stable shape. existing callers and tests are unaffected — the pre-existing keys stay.
the review console lists a mechanically-rolled-up session summary as "needs summary" (summarized=false) and its Summarize action calls kb.summarize_session. by then the capture buffer is gone, so narrate from the filed proposal's body instead: file the narrated topical page(s) as pending and reject the mechanical proposal (reason: superseded by llm narrative summary), keeping one summary per session. an llm failure leaves the mechanical proposal intact and returns skipped: llm-failed. build_session_rows now marks only session-split proposals summarized; a vouch-capture rollup stays summarized=false until it is narrated, so it surfaces in the review pipeline. review gate intact throughout — the narrated page is a pending proposal, never auto-approved.
design for a hard-delete path that routes through proposals.approve(): new ProposalKind.DELETE, one generic kb.propose_delete across all four durable kinds, block-if-referenced integrity per an inbound-ref matrix, idempotent approve, and per-kind storage/index removal. covers claim, page, entity, relation; source/evidence and cascade are out of scope.
relations are embedded on write (_embed_and_store), so deindex must drop their embedding_index row; only the fts presence differs by kind.
six tdd tasks: storage delete_*, index deindex, ProposalKind.DELETE + referenced_by, propose_delete, approve() delete branch, and the four surface registrations. bottom-up so each task's tests pass with real deps.
delete_claim/page/entity/relation unlink the artifact file and raise ArtifactNotFoundError if absent. no ref checks here — those live in the proposals review gate.
drops the fts row (claim/page/entity), the embedding row (any kind), and prov edges touching the id, so state.db stays consistent when an artifact is hard-deleted.
referenced_by returns inbound referrers per kind (claim: pages, relations, supersede/contradict; page: relations; entity: claims, pages, relations; relation: none). shared by the propose and approve delete gates.
files a PENDING delete proposal for a claim/page/entity/relation, snapshots the artifact into the payload, and refuses up front if the target is still referenced (claims get a supersede hint).
approve() now removes the artifact + index rows for a DELETE proposal, re-checks references at the gate, logs a per-kind <kind>.delete audit event with the snapshot, and is idempotent if the file is already gone. batch precheck (check_approvable) reports a referenced target as unapprovable.
register the method on the MCP tool surface, the JSONL handler map, the METHODS list, and the CLI (`vouch propose-delete <kind> <id>`). approval stays on the existing kb.approve. capabilities parity test passes.
add session_id/dry_run params and translate ProposalError to ValueError via the shared try/except + _proposal_response, matching the other propose tools so the mcp surface no longer drifts from jsonl for the same method.
the already-gone approve path returned before deindex, so a crash between the file unlink and deindex could leave stale fts/embedding/prov rows and keep a deleted artifact searchable. run the idempotent no-op deindex there too. mark the per-kind delete audit event reversible=false, note the untyped relation-endpoint limitation, and add tests for the idempotent index cleanup, per-kind delete audit events, and embedding deindex.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-07-09-session-split-summaries-design.md (1)
76-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSpecify a language for fenced code blocks.
markdownlint flags MD040 on lines 76 and 130 — the ASCII art diagrams use bare
```fences without a language tag. Addingtextorplaintextsilences the linter and improves rendering in some Markdown viewers.♻️ Proposed fix
-``` +```text HOST EDGE (per-host, thin) NEUTRAL CORE (host-blind)And similarly for the control-flow diagram at line 130.
Also applies to: 130-130
🤖 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-09-session-split-summaries-design.md` at line 76, The fenced ASCII art blocks in this document are missing a language tag and trigger markdownlint MD040. Update the bare fences used for the diagrams near the session-split summaries section to use a text-compatible language such as text or plaintext, including the matching control-flow diagram later in the same spec, so the Markdown renderer and linter both accept them.
🤖 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 `@docs/superpowers/plans/2026-07-09-artifact-delete.md`:
- Around line 857-883: The new `kb.propose_delete` test in
`tests/test_delete.py` only covers the success payload and needs JSONL envelope
coverage. Update the `handle_request`-based tests to assert the full success
envelope shape (`id`, `ok`, `result`) and add a failure-case assertion for the
error envelope (`id`, `ok: false`, `error`) using the existing `handle_request`
and `capabilities.METHODS` entry as anchors.
In `@docs/superpowers/plans/2026-07-09-session-split-summaries.md`:
- Around line 1014-1027: The current summarize-session test only calls HANDLERS
directly, so it misses the JSONL response envelope returned by handle_request.
Update test_jsonl_handler_summarizes to assert both success and failure through
vouch.jsonl_server.handle_request for kb.summarize_session, checking the {id,
ok, result} shape on success and {id, ok: false, error} on failure, while
keeping the direct handler assertions if needed for behavior coverage.
In `@src/vouch/capabilities.py`:
- Line 57: Add end-to-end JSONL envelope coverage for kb.list_sessions and
kb.summarize_session in the same test area that already exercises handle_request
for kb.propose_delete. Update or add tests to send requests through
handle_request(...) and assert both the success shape ({id, ok, result}) and
failure shape ({id, ok: false, error}) for these methods, rather than relying
only on direct handler calls.
In `@src/vouch/llm_draft.py`:
- Around line 43-55: Stop passing the configured LLM command through shell
evaluation in llm_draft.py’s helper around subprocess.run; parse llm_cmd into an
argv list and invoke subprocess.run with shell=False (or route through a fixed
runner wrapper) so the llm_draft execution used by compile/session-split cannot
execute arbitrary shell syntax. Update the command-handling logic in the
TemporaryDirectory block and keep the existing timeout/error handling and
LLMDraftError flow intact.
In `@webapp/app.py`:
- Around line 47-49: The in-memory session cache stored in _sessions is
unbounded, so chat history and new session_id entries can accumulate forever
even though MAX_HISTORY_MESSAGES only limits model input. Add eviction/pruning
in the chat session flow that appends to _sessions, such as TTL or LRU-based
cleanup, and ensure stale sessions are removed when handling new turns. Keep the
existing MAX_HISTORY_MESSAGES behavior, but bound the lifetime and size of
_sessions itself so the long-lived UI process cannot grow memory indefinitely.
- Around line 117-136: The record lookup logic in the app.py RPC resolution path
is swallowing all RpcError cases and turning backend failures into “not found.”
Update the try/except around the record fetch loop so only a confirmed not-found
condition from the RPC layer continues the search, while any other RpcError from
methods like rpc, kb.read_*, or kb.list_sources is converted into an
HTTPException with 502 (similar to the existing kb.list_sources handling). Keep
the final 404 only for the case where all lookups succeed but no matching record
is found.
In `@webapp/e2e/global-setup.ts`:
- Around line 67-75: The e2e bootstrap can leave a detached vouch server running
and persist stale state if health checks fail. In `global-setup.ts`, update the
`spawn`/`writeFileSync`/`waitForHealth` flow so failures from `waitForHealth()`
trigger cleanup of the started process and removal of the `.kb-state.json`
state, using the existing `server` handle and `STATE_FILE` symbols. Ensure the
bootstrap only leaves behind a live server and state when startup succeeds.
In `@webapp/plugins/claude-bridge.ts`:
- Around line 55-60: The request body handling in claude-bridge.ts currently
calls req.destroy() when raw exceeds MAX_PROMPT_CHARS * 2, which aborts the
connection without returning a client-visible validation error. Update the
request stream handling in the data/end flow to detect the oversized payload and
send a handled error response instead of destroying the socket, using the
existing request handler logic around req.on('data') and req.on('end') so the
client receives a clear 4xx-style error message.
In `@webapp/plugins/vouch-proxy.test.ts`:
- Line 132: The temporary upstream shutdown in the vouch proxy test is not being
awaited, so the next request can race the socket teardown. Update the cleanup in
vouch-proxy.test.ts to await the close of the temporary upstream object
associated with dropUpstream, using the existing test teardown flow around
dropUpstream.close() so the socket is fully released before the next request.
- Around line 43-46: The afterAll cleanup in vouch-proxy.test.ts is not waiting
for the servers to finish closing, so the test hook can return before sockets
are released. Update the afterAll callback to await the asynchronous shutdown of
both upstream and proxy by handling the Promise returned from close(), using the
existing upstream and proxy variables. Keep the cleanup logic in the same
afterAll block so the suite fully releases handles before exiting.
In `@webapp/src/styles.css`:
- Around line 85-92: The `.markdown-body code` rule is using the deprecated
`word-break: break-word` value, which Stylelint flags. Update that styling in
`styles.css` by replacing the `word-break` declaration with `overflow-wrap:
anywhere` while keeping the same `.markdown-body code` selector and existing
monospace/code formatting intact.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-07-09-session-split-summaries-design.md`:
- Line 76: The fenced ASCII art blocks in this document are missing a language
tag and trigger markdownlint MD040. Update the bare fences used for the diagrams
near the session-split summaries section to use a text-compatible language such
as text or plaintext, including the matching control-flow diagram later in the
same spec, so the Markdown renderer and linter both accept them.
🪄 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: beb4d6f4-7f16-46da-af3a-3d13c577adac
⛔ Files ignored due to path filters (1)
webapp/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (86)
docs/superpowers/plans/2026-07-09-artifact-delete.mddocs/superpowers/plans/2026-07-09-session-split-summaries.mddocs/superpowers/specs/2026-07-09-artifact-delete-design.mddocs/superpowers/specs/2026-07-09-session-split-summaries-design.mdsrc/vouch/capabilities.pysrc/vouch/capture.pysrc/vouch/cli.pysrc/vouch/compile.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/llm_draft.pysrc/vouch/models.pysrc/vouch/proposals.pysrc/vouch/server.pysrc/vouch/session_split.pysrc/vouch/storage.pytests/test_delete.pytests/test_llm_draft.pytests/test_session_split.pywebapp/.gitignorewebapp/README.mdwebapp/app.pywebapp/docs/superpowers/plans/2026-07-04-vouch-ui.mdwebapp/docs/superpowers/specs/2026-07-04-vouch-ui-design.mdwebapp/e2e/global-setup.tswebapp/e2e/global-teardown.tswebapp/e2e/smoke.spec.tswebapp/index.htmlwebapp/package.jsonwebapp/playwright.config.tswebapp/plugins/claude-bridge.tswebapp/plugins/vouch-proxy.test.tswebapp/plugins/vouch-proxy.tswebapp/src/App.test.tsxwebapp/src/App.tsxwebapp/src/components/ArtifactDrawer.test.tsxwebapp/src/components/ArtifactDrawer.tsxwebapp/src/components/ClaimLifecycleActions.test.tsxwebapp/src/components/ClaimLifecycleActions.tsxwebapp/src/components/DeleteArtifactButton.test.tsxwebapp/src/components/DeleteArtifactButton.tsxwebapp/src/components/EmptyState.tsxwebapp/src/components/ErrorBoundary.test.tsxwebapp/src/components/ErrorBoundary.tsxwebapp/src/components/ErrorCard.tsxwebapp/src/components/Markdown.tsxwebapp/src/components/Shell.multi.test.tsxwebapp/src/components/Shell.test.tsxwebapp/src/components/Shell.tsxwebapp/src/components/Toast.test.tsxwebapp/src/components/Toast.tsxwebapp/src/connection/ConnectDialog.test.tsxwebapp/src/connection/ConnectDialog.tsxwebapp/src/connection/ConnectionContext.multi.test.tsxwebapp/src/connection/ConnectionContext.test.tsxwebapp/src/connection/ConnectionContext.tsxwebapp/src/lib/citations.test.tswebapp/src/lib/citations.tswebapp/src/lib/claude.tswebapp/src/lib/fanout.tswebapp/src/lib/rpc.test.tswebapp/src/lib/rpc.tswebapp/src/lib/types.tswebapp/src/main.tsxwebapp/src/styles.csswebapp/src/test/setup.tswebapp/src/test/utils.tsxwebapp/src/views/BrowseView.test.tsxwebapp/src/views/BrowseView.tsxwebapp/src/views/ChatView.claude.test.tsxwebapp/src/views/ChatView.test.tsxwebapp/src/views/ChatView.tsxwebapp/src/views/ClaimsView.test.tsxwebapp/src/views/ClaimsView.tsxwebapp/src/views/PendingView.multi.test.tsxwebapp/src/views/PendingView.test.tsxwebapp/src/views/PendingView.tsxwebapp/src/views/ReviewView.test.tsxwebapp/src/views/ReviewView.tsxwebapp/src/views/StatsView.test.tsxwebapp/src/views/StatsView.tsxwebapp/static/app.csswebapp/static/index.htmlwebapp/tsconfig.jsonwebapp/vite.config.tswebapp/vitest.config.ts
✅ Files skipped from review due to trivial changes (1)
- docs/superpowers/specs/2026-07-09-artifact-delete-design.md
Comment on lines
+857
to
+883
| - [ ] **Step 1: Write the failing test** | ||
|
|
||
| Append to `tests/test_delete.py`: | ||
|
|
||
| ```python | ||
| from vouch import capabilities | ||
| from vouch.jsonl_server import handle_request | ||
|
|
||
|
|
||
| def test_method_registered_in_capabilities() -> None: | ||
| assert "kb.propose_delete" in capabilities.METHODS | ||
|
|
||
|
|
||
| def test_jsonl_propose_delete_end_to_end(store: KBStore, monkeypatch) -> None: | ||
| monkeypatch.chdir(store.root) | ||
| _claim(store, "c1", "kill via jsonl") | ||
| resp = handle_request({ | ||
| "id": "r1", | ||
| "method": "kb.propose_delete", | ||
| "params": {"target_kind": "claim", "target_id": "c1"}, | ||
| }) | ||
| assert resp["ok"] is True, resp | ||
| result = resp["result"] | ||
| assert result["kind"] == "delete" | ||
| assert result["status"] == "pending" | ||
| assert result["proposal_id"] | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Add JSONL envelope assertions for the new method.
The proposed test only checks the raw success payload. It still needs handle_request(...) coverage for both the success envelope ({id, ok, result}) and a failure envelope ({id, ok: false, error}), otherwise the new kb.propose_delete surface can regress silently. As per coding guidelines, for each new kb.* method, add a test that asserts the JSONL envelope shape: {id, ok, result} on success and {id, ok: false, error} on failure.
🤖 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/plans/2026-07-09-artifact-delete.md` around lines 857 - 883,
The new `kb.propose_delete` test in `tests/test_delete.py` only covers the
success payload and needs JSONL envelope coverage. Update the
`handle_request`-based tests to assert the full success envelope shape (`id`,
`ok`, `result`) and add a failure-case assertion for the error envelope (`id`,
`ok: false`, `error`) using the existing `handle_request` and
`capabilities.METHODS` entry as anchors.
Source: Coding guidelines
Comment on lines
+1014
to
+1027
| def test_kb_summarize_session_in_capabilities_and_handlers() -> None: | ||
| from vouch import capabilities | ||
| from vouch.jsonl_server import HANDLERS | ||
| assert "kb.summarize_session" in capabilities.METHODS | ||
| assert "kb.summarize_session" in HANDLERS | ||
|
|
||
|
|
||
| def test_jsonl_handler_summarizes(store: KBStore, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| import vouch.jsonl_server as js | ||
| _observe(store, "s1", 5) | ||
| monkeypatch.setattr(js, "_store", lambda: store) | ||
| res = js.HANDLERS["kb.summarize_session"]({"session_id": "s1"}) | ||
| assert res["mode"] == "mechanical" | ||
| assert res["summary_proposal_id"] is not None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Cover the JSONL envelope, not just the handler map.
test_jsonl_handler_summarizes calls HANDLERS[...] directly, so it never exercises the envelope that handle_request(...) returns. Add success and failure handle_request assertions here as well; otherwise the new kb.summarize_session contract can regress without a test catching {id, ok, result} / {id, ok: false, error}. As per coding guidelines, for each new kb.* method, add a test that asserts the JSONL envelope shape: {id, ok, result} on success and {id, ok: false, error} on failure.
🤖 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/plans/2026-07-09-session-split-summaries.md` around lines
1014 - 1027, The current summarize-session test only calls HANDLERS directly, so
it misses the JSONL response envelope returned by handle_request. Update
test_jsonl_handler_summarizes to assert both success and failure through
vouch.jsonl_server.handle_request for kb.summarize_session, checking the {id,
ok, result} shape on success and {id, ok: false, error} on failure, while
keeping the direct handler assertions if needed for behavior coverage.
Source: Coding guidelines
| "kb.propose_page", | ||
| "kb.propose_entity", | ||
| "kb.propose_relation", | ||
| "kb.propose_delete", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify all three new kb.* methods are registered in all four files
# and have tests asserting the JSONL envelope shape.
for method in "kb.propose_delete" "kb.list_sessions" "kb.summarize_session"; do
echo "=== $method ==="
echo "--- server.py ---"
rg -n "$method" src/vouch/server.py || echo "NOT FOUND"
echo "--- jsonl_server.py ---"
rg -n "$method" src/vouch/jsonl_server.py || echo "NOT FOUND"
echo "--- capabilities.py ---"
rg -n "$method" src/vouch/capabilities.py || echo "NOT FOUND"
echo "--- cli.py ---"
rg -n "$method" src/vouch/cli.py || echo "NOT FOUND"
echo "--- tests ---"
rg -l "$method" tests/ || echo "NO TESTS FOUND"
echo
doneRepository: vouchdev/vouch
Length of output: 964
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Registration context =="
for f in src/vouch/server.py src/vouch/jsonl_server.py src/vouch/capabilities.py src/vouch/cli.py; do
echo "--- $f ---"
rg -n "kb\.(propose_delete|list_sessions|summarize_session)" "$f" || true
done
echo
echo "== Test envelope assertions =="
for f in tests/test_delete.py tests/test_session_split.py; do
echo "--- $f ---"
rg -n "ok|error|result|id|jsonl|envelope|kb\.(propose_delete|list_sessions|summarize_session)" "$f" || true
doneRepository: vouchdev/vouch
Length of output: 9546
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/vouch/server.py relevant sections =="
sed -n '500,680p' src/vouch/server.py
echo
echo "== src/vouch/cli.py relevant sections =="
sed -n '1,260p' src/vouch/cli.py
echo
echo "== src/vouch/jsonl_server.py handler table =="
sed -n '760,815p' src/vouch/jsonl_server.pyRepository: vouchdev/vouch
Length of output: 17479
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tests/test_delete.py JSONL-related section =="
sed -n '320,390p' tests/test_delete.py
echo
echo "== tests/test_session_split.py JSONL-related sections =="
sed -n '205,370p' tests/test_session_split.py
echo
echo "== Search for failure-envelope assertions for the three methods =="
rg -n '("ok"\s*:\s*False|ok.*False|error|ValueError|jsonl|HANDLERS\["kb\.(propose_delete|list_sessions|summarize_session)"\])' tests/test_delete.py tests/test_session_split.pyRepository: vouchdev/vouch
Length of output: 9529
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== capabilities.py method list / schema =="
sed -n '1,140p' src/vouch/capabilities.py
echo
echo "== cli.py references to capabilities or JSONL/MCP =="
rg -n "build_caps|capabilities|METHODS|jsonl|mcp|HANDLERS|kb\." src/vouch/cli.py src/vouch/capabilities.pyRepository: vouchdev/vouch
Length of output: 11154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== JSONL request envelope tests in repository =="
rg -n 'handle_request\(|"ok"\s*:\s*False|ok.*False|"\berror\b"|Invalid JSON|method not found|JSONL' tests/ src/vouch/jsonl_server.py
echo
echo "== jsonl_server error wrapping helpers =="
sed -n '1,220p' src/vouch/jsonl_server.pyRepository: vouchdev/vouch
Length of output: 18042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tests/test_jsonl_server.py excerpts =="
sed -n '1,280p' tests/test_jsonl_server.py
echo
echo "== search for the three methods in JSONL tests =="
rg -n 'kb\.(propose_delete|list_sessions|summarize_session)|"ok"|"\berror\b"|"\bresult\b"' tests/test_jsonl_server.py tests/test_delete.py tests/test_session_split.pyRepository: vouchdev/vouch
Length of output: 13190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' tests/test_capabilities.pyRepository: vouchdev/vouch
Length of output: 4243
Add JSONL envelope tests for kb.list_sessions and kb.summarize_session. kb.propose_delete already has an end-to-end handle_request(...) test, but the other two methods are only covered via direct handler calls. Add success/failure envelope assertions for them so the {id, ok, result} / {id, ok: false, error} contract is exercised too.
🤖 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/capabilities.py` at line 57, Add end-to-end JSONL envelope coverage
for kb.list_sessions and kb.summarize_session in the same test area that already
exercises handle_request for kb.propose_delete. Update or add tests to send
requests through handle_request(...) and assert both the success shape ({id, ok,
result}) and failure shape ({id, ok: false, error}) for these methods, rather
than relying only on direct handler calls.
Source: Coding guidelines
Comment on lines
+43
to
+55
| with tempfile.TemporaryDirectory(prefix="vouch-llm-") as tmp: | ||
| try: | ||
| proc = subprocess.run( | ||
| llm_cmd, shell=True, cwd=tmp, | ||
| input=prompt, capture_output=True, text=True, | ||
| encoding="utf-8", errors="replace", | ||
| timeout=timeout_seconds, | ||
| ) | ||
| except subprocess.TimeoutExpired as e: | ||
| raise LLMDraftError(f"{label} timed out after {timeout_seconds:.0f}s") from e | ||
| if proc.returncode != 0: | ||
| detail = (proc.stderr or proc.stdout or "").strip()[:400] | ||
| raise LLMDraftError(f"{label} failed ({proc.returncode}): {detail}") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Stop shell-evaluating the configured LLM command.
llm_cmd is passed straight to subprocess.run(..., shell=True), so a config value can execute arbitrary shell syntax. Parse it into argv and run with shell=False (or wrap it in a fixed runner script) before this helper is used by compile/session-split.
Suggested hardening
+import shlex
import json
import re
import subprocess
import tempfile
@@
- proc = subprocess.run(
- llm_cmd, shell=True, cwd=tmp,
+ proc = subprocess.run(
+ shlex.split(llm_cmd), shell=False, cwd=tmp,
input=prompt, capture_output=True, text=True,
encoding="utf-8", errors="replace",
timeout=timeout_seconds,
)📝 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
| with tempfile.TemporaryDirectory(prefix="vouch-llm-") as tmp: | |
| try: | |
| proc = subprocess.run( | |
| llm_cmd, shell=True, cwd=tmp, | |
| input=prompt, capture_output=True, text=True, | |
| encoding="utf-8", errors="replace", | |
| timeout=timeout_seconds, | |
| ) | |
| except subprocess.TimeoutExpired as e: | |
| raise LLMDraftError(f"{label} timed out after {timeout_seconds:.0f}s") from e | |
| if proc.returncode != 0: | |
| detail = (proc.stderr or proc.stdout or "").strip()[:400] | |
| raise LLMDraftError(f"{label} failed ({proc.returncode}): {detail}") | |
| with tempfile.TemporaryDirectory(prefix="vouch-llm-") as tmp: | |
| try: | |
| proc = subprocess.run( | |
| shlex.split(llm_cmd), shell=False, cwd=tmp, | |
| input=prompt, capture_output=True, text=True, | |
| encoding="utf-8", errors="replace", | |
| timeout=timeout_seconds, | |
| ) | |
| except subprocess.TimeoutExpired as e: | |
| raise LLMDraftError(f"{label} timed out after {timeout_seconds:.0f}s") from e | |
| if proc.returncode != 0: | |
| detail = (proc.stderr or proc.stdout or "").strip()[:400] | |
| raise LLMDraftError(f"{label} failed ({proc.returncode}): {detail}") |
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 44-49: Command coming from incoming request
Context: subprocess.run(
llm_cmd, shell=True, cwd=tmp,
input=prompt, capture_output=True, text=True,
encoding="utf-8", errors="replace",
timeout=timeout_seconds,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[error] 44-49: Use of unsanitized data to create processes
Context: subprocess.run(
llm_cmd, shell=True, cwd=tmp,
input=prompt, capture_output=True, text=True,
encoding="utf-8", errors="replace",
timeout=timeout_seconds,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
🪛 OpenGrep (1.23.0)
[ERROR] 45-50: Dynamic command passed to subprocess with shell=True. Use a command list without shell=True, or use shlex.quote() to sanitize input.
(coderabbit.command-injection.python-shell-true)
🤖 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/llm_draft.py` around lines 43 - 55, Stop passing the configured LLM
command through shell evaluation in llm_draft.py’s helper around subprocess.run;
parse llm_cmd into an argv list and invoke subprocess.run with shell=False (or
route through a fixed runner wrapper) so the llm_draft execution used by
compile/session-split cannot execute arbitrary shell syntax. Update the
command-handling logic in the TemporaryDirectory block and keep the existing
timeout/error handling and LLMDraftError flow intact.
Comment on lines
+47
to
+49
| # in-memory chat sessions; the durable record lives in vouch, not here | ||
| _sessions: dict[str, list[dict[str, Any]]] = {} | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
Bound the in-memory session cache.
MAX_HISTORY_MESSAGES only caps what gets sent to the model; each chat turn still appends to _sessions forever, and new session_ids are never evicted. A long-lived UI process can leak memory indefinitely unless you add pruning/TTL/LRU eviction.
Also applies to: 265-304
🤖 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 `@webapp/app.py` around lines 47 - 49, The in-memory session cache stored in
_sessions is unbounded, so chat history and new session_id entries can
accumulate forever even though MAX_HISTORY_MESSAGES only limits model input. Add
eviction/pruning in the chat session flow that appends to _sessions, such as TTL
or LRU-based cleanup, and ensure stale sessions are removed when handling new
turns. Keep the existing MAX_HISTORY_MESSAGES behavior, but bound the lifetime
and size of _sessions itself so the long-lived UI process cannot grow memory
indefinitely.
Comment on lines
+67
to
+75
| const server = spawn('vouch', ['serve', '--transport', 'http', '--port', String(VOUCH_PORT)], { | ||
| cwd: dir, | ||
| detached: true, | ||
| stdio: 'ignore', | ||
| }) | ||
| server.unref() | ||
|
|
||
| writeFileSync(STATE_FILE, JSON.stringify({ dir, pid: server.pid })) | ||
| await waitForHealth(`http://127.0.0.1:${VOUCH_PORT}/health`) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the e2e bootstrap fail-safe.
If waitForHealth() throws, the detached vouch serve process is still alive and .kb-state.json has already been written, which can leak a live server and stale state into later runs.
🔧 Proposed fix
const server = spawn('vouch', ['serve', '--transport', 'http', '--port', String(VOUCH_PORT)], {
cwd: dir,
detached: true,
stdio: 'ignore',
})
server.unref()
-
- writeFileSync(STATE_FILE, JSON.stringify({ dir, pid: server.pid }))
- await waitForHealth(`http://127.0.0.1:${VOUCH_PORT}/health`)
+ try {
+ await waitForHealth(`http://127.0.0.1:${VOUCH_PORT}/health`)
+ writeFileSync(STATE_FILE, JSON.stringify({ dir, pid: server.pid }))
+ } catch (err) {
+ server.kill('SIGTERM')
+ throw err
+ }📝 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
| const server = spawn('vouch', ['serve', '--transport', 'http', '--port', String(VOUCH_PORT)], { | |
| cwd: dir, | |
| detached: true, | |
| stdio: 'ignore', | |
| }) | |
| server.unref() | |
| writeFileSync(STATE_FILE, JSON.stringify({ dir, pid: server.pid })) | |
| await waitForHealth(`http://127.0.0.1:${VOUCH_PORT}/health`) | |
| const server = spawn('vouch', ['serve', '--transport', 'http', '--port', String(VOUCH_PORT)], { | |
| cwd: dir, | |
| detached: true, | |
| stdio: 'ignore', | |
| }) | |
| server.unref() | |
| try { | |
| await waitForHealth(`http://127.0.0.1:${VOUCH_PORT}/health`) | |
| writeFileSync(STATE_FILE, JSON.stringify({ dir, pid: server.pid })) | |
| } catch (err) { | |
| server.kill('SIGTERM') | |
| throw err | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync, spawn } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@webapp/e2e/global-setup.ts` around lines 67 - 75, The e2e bootstrap can leave
a detached vouch server running and persist stale state if health checks fail.
In `global-setup.ts`, update the `spawn`/`writeFileSync`/`waitForHealth` flow so
failures from `waitForHealth()` trigger cleanup of the started process and
removal of the `.kb-state.json` state, using the existing `server` handle and
`STATE_FILE` symbols. Ensure the bootstrap only leaves behind a live server and
state when startup succeeds.
Comment on lines
+55
to
+60
| let raw = '' | ||
| req.on('data', (c) => { | ||
| raw += String(c) | ||
| if (raw.length > MAX_PROMPT_CHARS * 2) req.destroy() | ||
| }) | ||
| req.on('end', () => { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Return a handled error for oversized bodies.
req.destroy() aborts the connection without a response, so the client sees an opaque reset instead of a validation error.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process'
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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 `@webapp/plugins/claude-bridge.ts` around lines 55 - 60, The request body
handling in claude-bridge.ts currently calls req.destroy() when raw exceeds
MAX_PROMPT_CHARS * 2, which aborts the connection without returning a
client-visible validation error. Update the request stream handling in the
data/end flow to detect the oversized payload and send a handled error response
instead of destroying the socket, using the existing request handler logic
around req.on('data') and req.on('end') so the client receives a clear 4xx-style
error message.
Comment on lines
+43
to
+46
| afterAll(() => { | ||
| upstream.close() | ||
| proxy.close() | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Await server shutdown in afterAll.
close() is async, so this hook returns before the sockets are actually released. That can leave open handles and make the suite flaky.
🧹 Proposed fix
-afterAll(() => {
- upstream.close()
- proxy.close()
-})
+afterAll(async () => {
+ await Promise.all([
+ new Promise<void>((resolve) => upstream.close(() => resolve())),
+ new Promise<void>((resolve) => proxy.close(() => resolve())),
+ ])
+})📝 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
| afterAll(() => { | |
| upstream.close() | |
| proxy.close() | |
| }) | |
| afterAll(async () => { | |
| await Promise.all([ | |
| new Promise<void>((resolve) => upstream.close(() => resolve())), | |
| new Promise<void>((resolve) => proxy.close(() => resolve())), | |
| ]) | |
| }) |
🤖 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 `@webapp/plugins/vouch-proxy.test.ts` around lines 43 - 46, The afterAll
cleanup in vouch-proxy.test.ts is not waiting for the servers to finish closing,
so the test hook can return before sockets are released. Update the afterAll
callback to await the asynchronous shutdown of both upstream and proxy by
handling the Promise returned from close(), using the existing upstream and
proxy variables. Keep the cleanup logic in the same afterAll block so the suite
fully releases handles before exiting.
| // equally acceptable way for the request to terminate. | ||
| } | ||
|
|
||
| dropUpstream.close() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Await the temporary upstream teardown too.
dropUpstream.close() is fire-and-forget here, so the test can race the socket teardown before the next request.
🧹 Proposed fix
- dropUpstream.close()
+ await new Promise<void>((resolve) => dropUpstream.close(() => resolve()))📝 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
| dropUpstream.close() | |
| await new Promise<void>((resolve) => dropUpstream.close(() => resolve())) |
🤖 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 `@webapp/plugins/vouch-proxy.test.ts` at line 132, The temporary upstream
shutdown in the vouch proxy test is not being awaited, so the next request can
race the socket teardown. Update the cleanup in vouch-proxy.test.ts to await the
close of the temporary upstream object associated with dropUpstream, using the
existing test teardown flow around dropUpstream.close() so the socket is fully
released before the next request.
Comment on lines
+85
to
+92
| .markdown-body code { | ||
| font-family: var(--font-mono); | ||
| font-size: 0.8em; | ||
| background: var(--paper-3); | ||
| border-radius: 4px; | ||
| padding: 0.1em 0.35em; | ||
| word-break: break-word; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the deprecated word-break value.
break-word is already flagged by Stylelint and can be swapped for overflow-wrap: anywhere without changing the intent.
♻️ Proposed fix
.markdown-body code {
font-family: var(--font-mono);
font-size: 0.8em;
background: var(--paper-3);
border-radius: 4px;
padding: 0.1em 0.35em;
- word-break: break-word;
+ overflow-wrap: anywhere;
}📝 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
| .markdown-body code { | |
| font-family: var(--font-mono); | |
| font-size: 0.8em; | |
| background: var(--paper-3); | |
| border-radius: 4px; | |
| padding: 0.1em 0.35em; | |
| word-break: break-word; | |
| } | |
| .markdown-body code { | |
| font-family: var(--font-mono); | |
| font-size: 0.8em; | |
| background: var(--paper-3); | |
| border-radius: 4px; | |
| padding: 0.1em 0.35em; | |
| overflow-wrap: anywhere; | |
| } |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 91-91: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🤖 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 `@webapp/src/styles.css` around lines 85 - 92, The `.markdown-body code` rule
is using the deprecated `word-break: break-word` value, which Stylelint flags.
Update that styling in `styles.css` by replacing the `word-break` declaration
with `overflow-wrap: anywhere` while keeping the same `.markdown-body code`
selector and existing monospace/code formatting intact.
Source: Linters/SAST tools
… API support both workflows in the demo: - if ~/.claude exists, use 'claude -p' for session capture in Claude - if ANTHROPIC_API_KEY is set, use direct API shim (vouch-llm) - entrypoint detects available approach and configures accordingly update README to clarify how users can access vouch without cloning: - docker demo as entry point for exploring full UI - pipx install + vouch install-mcp for Claude Code integration - CLI tools for review/approval workflows - option to point Docker UI at locally-running KB users no longer need to choose between easy setup (API key) and session capture (claude -p)—the demo now supports both.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
demo/Dockerfile (1)
22-59: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a non-root
USERdirective to the runtime stage.The image runs as root with no
USERinstruction, flagged by Trivy DS-0002. For a demo image this is common, but adding a non-root user is a simple security posture improvement.🔒️ Suggested fix
FROM node:22-slim +RUN groupadd --system vouch && useradd --system --gid vouch --home-dir /data vouch ENV PYTHONDONTWRITEBYTECODE=1 \ PYTHONUNBUFFERED=1 \ LANG=C.UTF-8 \ NODE_ENV=production \ VOUCH_UI_ALLOW_REMOTE=1 \ VOUCH_TARGET=http://127.0.0.1:8731 \ VOUCH_HTTP_TOKEN=vouch-demo \ VOUCH_DATA_DIR=/data \ ANTHROPIC_MODEL=claude-sonnet-4-5 +# … existing RUN/COPY steps … +RUN chown -R vouch:vouch /data /app/webapp +USER vouch VOLUME ["/data"] EXPOSE 5173 ENTRYPOINT ["/entrypoint.sh"]🤖 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 `@demo/Dockerfile` around lines 22 - 59, The runtime image in the Dockerfile currently defaults to root because there is no USER directive in the final stage. Add a non-root user for the runtime stage and switch to it after the app files and entrypoint are in place, making sure the user can still read the venv, /app/webapp, and write to /data as needed. Keep the change local to the demo Dockerfile and preserve the existing ENTRYPOINT setup.Source: Linters/SAST tools
🤖 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 `@demo/entrypoint.sh`:
- Around line 66-74: The startup flow in entrypoint.sh should fail fast if vouch
never becomes healthy. After the backgrounded vouch serve launch and the
health-check loop, explicitly verify success and exit non-zero when the probe
never passes so the UI does not start against a dead backend. Use the existing
vouch background process setup, VOUCH_PID, trap, and the curl health check loop
to locate the fix.
- Around line 28-29: The `entrypoint.sh` LLM command selection is too permissive
because `~/.claude` exists does not mean the `claude` binary is installed.
Update the `LLM_CMD` setup in the entrypoint so it only uses `claude -p --model
sonnet-4-5` when `command -v claude` succeeds, and otherwise leave the existing
fallback behavior intact. This should be done in the conditional that currently
checks `~/.claude`, using the same `LLM_CMD` assignment path so
compile/summarize only targets a real CLI.
- Line 29: Update the Claude CLI command in the demo entrypoint so it uses the
canonical Sonnet 4.5 model alias instead of the short identifier. In the LLM_CMD
assignment, replace the model value used by the claude invocation with
claude-sonnet-4-5 to match the Dockerfile’s ANTHROPIC_MODEL setting and keep the
demo path aligned.
In `@README.md`:
- Around line 110-113: The README docker run example uses host.docker.internal
without the Linux Docker Engine host mapping, so update the documentation near
the docker command to either include the
--add-host=host.docker.internal:host-gateway flag or add a clear Linux-specific
note. Keep the guidance tied to the existing docker run snippet so users
understand the VOUCH_TARGET setup and can locate the exact command easily.
---
Nitpick comments:
In `@demo/Dockerfile`:
- Around line 22-59: The runtime image in the Dockerfile currently defaults to
root because there is no USER directive in the final stage. Add a non-root user
for the runtime stage and switch to it after the app files and entrypoint are in
place, making sure the user can still read the venv, /app/webapp, and write to
/data as needed. Keep the change local to the demo Dockerfile and preserve the
existing ENTRYPOINT setup.
🪄 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: a0697553-406a-4e91-b1b4-b582a37ab1d2
📒 Files selected for processing (3)
README.mddemo/Dockerfiledemo/entrypoint.sh
Comment on lines
+28
to
+29
| if [ -d "$HOME/.claude" ]; then | ||
| LLM_CMD="claude -p --model sonnet-4-5" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
~/.claude directory check doesn't verify the claude CLI is installed.
The Dockerfile does not install the Claude CLI (claude), yet the entrypoint configures claude -p --model sonnet-4-5 as compile.llm_cmd whenever ~/.claude exists. If a user mounts their ~/.claude directory, the check passes but claude is not in $PATH, so vouch's compile/summarize will fail with "command not found." Use command -v claude to verify the binary is actually available.
🐛 Proposed fix
-if [ -d "$HOME/.claude" ]; then
+if command -v claude >/dev/null 2>&1 && [ -d "$HOME/.claude" ]; then
LLM_CMD="claude -p --model sonnet-4-5"📝 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
| if [ -d "$HOME/.claude" ]; then | |
| LLM_CMD="claude -p --model sonnet-4-5" | |
| if command -v claude >/dev/null 2>&1 && [ -d "$HOME/.claude" ]; then | |
| LLM_CMD="claude -p --model sonnet-4-5" |
🤖 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 `@demo/entrypoint.sh` around lines 28 - 29, The `entrypoint.sh` LLM command
selection is too permissive because `~/.claude` exists does not mean the
`claude` binary is installed. Update the `LLM_CMD` setup in the entrypoint so it
only uses `claude -p --model sonnet-4-5` when `command -v claude` succeeds, and
otherwise leave the existing fallback behavior intact. This should be done in
the conditional that currently checks `~/.claude`, using the same `LLM_CMD`
assignment path so compile/summarize only targets a real CLI.
| CONFIG="$DATA/.vouch/config.yaml" | ||
| LLM_CMD="" | ||
| if [ -d "$HOME/.claude" ]; then | ||
| LLM_CMD="claude -p --model sonnet-4-5" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if pyproject.toml or any config references the expected model names
rg -n 'sonnet-4-5\|claude-sonnet-4-5' --type yaml --type python || true
rg -n 'sonnet-4-5\|claude-sonnet-4-5' demo/ pyproject.toml || trueRepository: vouchdev/vouch
Length of output: 183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first
git ls-files demo/ Dockerfile* . | sed -n '1,120p'
echo "== demo/entrypoint.sh =="
cat -n demo/entrypoint.sh | sed -n '1,120p'
echo "== Dockerfile =="
cat -n Dockerfile | sed -n '1,220p'
echo "== model name references =="
rg -n 'claude-sonnet-4-5|sonnet-4-5|ANTHROPIC_MODEL|claude -p --model' .Repository: vouchdev/vouch
Length of output: 11626
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== demo/Dockerfile =="
cat -n demo/Dockerfile | sed -n '1,120p'
echo "== src/vouch/pr_cache.py (model env handling) =="
sed -n '320,360p' src/vouch/pr_cache.py
echo "== compile docs references =="
sed -n '1,120p' src/vouch/compile.py
echo "--- README excerpt ---"
sed -n '70,95p' README.md
echo "--- docs/compile.md excerpt ---"
sed -n '1,80p' docs/compile.mdRepository: vouchdev/vouch
Length of output: 13023
🌐 Web query:
Anthropic Claude CLI --model sonnet-4-5 claude-sonnet-4-5 model name format docs
💡 Result:
For the Anthropic Claude API, the model identifier for Claude Sonnet 4.5 is officially claude-sonnet-4-5 [1][2][3]. Anthropic model naming conventions have evolved over time [4]. Current generation models (starting with the 4.6 release series) have transitioned to a standardized, dateless format, while earlier models often included a specific date suffix (e.g., claude-sonnet-4-5-20250929) to denote a pinned snapshot [4][2]. When using Anthropic's models through various platforms, the naming format may vary: - Anthropic API: claude-sonnet-4-5 [1][2] - Amazon Bedrock: anthropic.claude-sonnet-4-5-20250929-v1:0 [2][5] - Google Cloud: claude-sonnet-4-5 (or claude-sonnet-4-5@20250929) [2][3] - Microsoft Foundry: claude-sonnet-4-5 [6] For development and CLI tools, it is recommended to use the canonical API model ID (claude-sonnet-4-5) where supported [4][1]. If you are working with an older environment or specific infrastructure provider (like AWS Bedrock), you may need to use the platform-specific identifier, which often includes a snapshot date and versioning suffix [4][2]. Always check the specific documentation for your integration environment, as aliases can sometimes be used to resolve to these model IDs [7][8].
Citations:
- 1: https://www.anthropic.com/news/claude-sonnet-4-5
- 2: https://platform.claude.com/docs/en/about-claude/models/overview?fcdaa149_sort_date=desc&us=
- 3: https://docs.cloud.google.com/gemini-enterprise-agent-platform/models/partner-models/claude/sonnet-4-5
- 4: https://platform.claude.com/docs/en/about-claude/models/model-ids-and-versions
- 5: https://aws.amazon.com/blogs/aws/introducing-claude-sonnet-4-5-in-amazon-bedrock-anthropics-most-intelligent-model-best-for-coding-and-complex-agents/
- 6: https://ai.azure.com/catalog/models/claude-sonnet-4-5
- 7: https://platform.claude.com/docs/en/api/models
- 8: https://code.claude.com/docs/en/model-config
🌐 Web query:
Claude CLI sonnet-4-5 model alias claude -p --model sonnet-4-5
💡 Result:
The correct alias to use for Claude Sonnet 4.5 is claude-sonnet-4-5 [1][2]. If you are using the Claude Code CLI (claude), you can specify the model using the --model flag [3]. While claude-sonnet-4-5 is the standard alias that points to the most recent version of that model [4], some environments or configurations may require the full, dated model ID, which is claude-sonnet-4-5-20250929 [3][1]. For the Anthropic official ant CLI, you would use the model ID within your command, such as: ant messages create --model claude-sonnet-4-5 ... [5][6]. In summary, use claude-sonnet-4-5 as your primary alias [1][2]. If you encounter errors, verify the specific model availability for your provider (e.g., Anthropic API, AWS Bedrock, or Google Cloud), as some platforms require the full dated ID [7][1].
Citations:
- 1: https://platform.claude.com/docs/en/about-claude/models/overview?cc61befa_page=2
- 2: https://github.com/anthropics/skills/blob/main/skills/claude-api/shared/models.md
- 3: https://support.claude.com/en/articles/11940350-claude-code-model-configuration
- 4: https://platform.claude.com/docs/en/about-claude/models/model-ids-and-versions
- 5: https://platform.claude.com/docs/en/cli-sdks-libraries/cli/using
- 6: https://platform.claude.com/docs/en/cli-sdks-libraries/cli/quickstart
- 7: https://code.claude.com/docs/en/model-config.md
Use claude-sonnet-4-5 in the Claude CLI path
claude -p --model should use the canonical Sonnet 4.5 alias, claude-sonnet-4-5, to stay aligned with ANTHROPIC_MODEL=claude-sonnet-4-5 in the Dockerfile. sonnet-4-5 is the wrong identifier here and can break the demo when Claude CLI is present.
🤖 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 `@demo/entrypoint.sh` at line 29, Update the Claude CLI command in the demo
entrypoint so it uses the canonical Sonnet 4.5 model alias instead of the short
identifier. In the LLM_CMD assignment, replace the model value used by the
claude invocation with claude-sonnet-4-5 to match the Dockerfile’s
ANTHROPIC_MODEL setting and keep the demo path aligned.
Comment on lines
+66
to
+74
| ( cd "$DATA" && exec vouch serve --transport http --host 127.0.0.1 --port 8731 --token "$TOKEN" ) & | ||
| VOUCH_PID=$! | ||
| trap 'kill "$VOUCH_PID" 2>/dev/null || true' EXIT INT TERM | ||
|
|
||
| # Wait for vouch to answer its public liveness probe before starting the UI. | ||
| for _ in $(seq 1 30); do | ||
| if curl -fsS -m 2 "http://127.0.0.1:8731/health" >/dev/null 2>&1; then break; fi | ||
| sleep 1 | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
UI starts even if the vouch server fails to come up.
vouch serve runs in a background subshell where set -e doesn't apply, and the health-check loop exits silently after 30 iterations without verifying success. If vouch crashes or fails to bind, the console still starts and the user sees proxy errors with no indication the backend is down.
🛡️ Proposed fix: fail fast after the loop
for _ in $(seq 1 30); do
if curl -fsS -m 2 "http://127.0.0.1:8731/health" >/dev/null 2>&1; then break; fi
sleep 1
done
+if ! curl -fsS -m 2 "http://127.0.0.1:8731/health" >/dev/null 2>&1; then
+ echo "[demo] ERROR: vouch server did not become healthy within 30 s" >&2
+ exit 1
+fi📝 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
| ( cd "$DATA" && exec vouch serve --transport http --host 127.0.0.1 --port 8731 --token "$TOKEN" ) & | |
| VOUCH_PID=$! | |
| trap 'kill "$VOUCH_PID" 2>/dev/null || true' EXIT INT TERM | |
| # Wait for vouch to answer its public liveness probe before starting the UI. | |
| for _ in $(seq 1 30); do | |
| if curl -fsS -m 2 "http://127.0.0.1:8731/health" >/dev/null 2>&1; then break; fi | |
| sleep 1 | |
| done | |
| ( cd "$DATA" && exec vouch serve --transport http --host 127.0.0.1 --port 8731 --token "$TOKEN" ) & | |
| VOUCH_PID=$! | |
| trap 'kill "$VOUCH_PID" 2>/dev/null || true' EXIT INT TERM | |
| # Wait for vouch to answer its public liveness probe before starting the UI. | |
| for _ in $(seq 1 30); do | |
| if curl -fsS -m 2 "http://127.0.0.1:8731/health" >/dev/null 2>&1; then break; fi | |
| sleep 1 | |
| done | |
| if ! curl -fsS -m 2 "http://127.0.0.1:8731/health" >/dev/null 2>&1; then | |
| echo "[demo] ERROR: vouch server did not become healthy within 30 s" >&2 | |
| exit 1 | |
| fi |
🤖 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 `@demo/entrypoint.sh` around lines 66 - 74, The startup flow in entrypoint.sh
should fail fast if vouch never becomes healthy. After the backgrounded vouch
serve launch and the health-check loop, explicitly verify success and exit
non-zero when the probe never passes so the UI does not start against a dead
backend. Use the existing vouch background process setup, VOUCH_PID, trap, and
the curl health check loop to locate the fix.
Comment on lines
+110
to
+113
| docker run --rm -p 127.0.0.1:5173:5173 \ | ||
| -e VOUCH_TARGET=http://host.docker.internal:8731 \ | ||
| ghcr.io/plind-junior/vouch-demo | ||
| # then open http://localhost:5173 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
host.docker.internal needs --add-host on Linux Docker Engine.
On Docker Desktop (macOS/Windows) host.docker.internal resolves automatically, but on Linux Docker Engine it requires --add-host=host.docker.internal:host-gateway. Without it, Linux users following this two-terminal setup get a connection refused error with no hint why.
📝 Suggested fix: add a Linux note or include the flag
docker run --rm -p 127.0.0.1:5173:5173 \
+ --add-host=host.docker.internal:host-gateway \
-e VOUCH_TARGET=http://host.docker.internal:8731 \
ghcr.io/plind-junior/vouch-demoOr add a note after the block:
+> **Linux users:** add `--add-host=host.docker.internal:host-gateway` to the
+> `docker run` command above (Docker Desktop handles this automatically).📝 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
| docker run --rm -p 127.0.0.1:5173:5173 \ | |
| -e VOUCH_TARGET=http://host.docker.internal:8731 \ | |
| ghcr.io/plind-junior/vouch-demo | |
| # then open http://localhost:5173 | |
| docker run --rm -p 127.0.0.1:5173:5173 \ | |
| --add-host=host.docker.internal:host-gateway \ | |
| -e VOUCH_TARGET=http://host.docker.internal:8731 \ | |
| ghcr.io/plind-junior/vouch-demo | |
| # then open http://localhost:5173 |
🤖 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 `@README.md` around lines 110 - 113, The README docker run example uses
host.docker.internal without the Linux Docker Engine host mapping, so update the
documentation near the docker command to either include the
--add-host=host.docker.internal:host-gateway flag or add a clear Linux-specific
note. Keep the guidance tied to the existing docker run snippet so users
understand the VOUCH_TARGET setup and can locate the exact command easily.
* feat(claims): add bulk clear for auto-approved claims enables users to bulk clear auto-saved claims while preserving manually reviewed knowledge. when auto-approval is enabled (review.approver_role: trusted-agent), users can reset by clearing auto-approved claims in bulk. implemented: - new lifecycle function: clear_claims() filters by auto-approval status and optional date range, archives matching claims - added Claim.auto_approved and Claim.proposed_by fields - updated approval logic to set auto_approved flag when approver == proposer - cli command: vouch claims-clear [--auto-only] [--before DATE] [--confirm] - mcp tool: kb_clear_claims() with dry-run support - jsonl handler for kb.clear_claims in jsonl_server - registered in capabilities.METHODS for parity - comprehensive test suite covering filters, dry-run, audit trail design: - archives rather than deletes to preserve audit history - single bulk audit event logs all affected claim ids - no review gate (consistent with supersede/archive) - dry-run support for preview before confirmation fix: #433 * feat(web): integrate bulk clear into the review console adds a "clear" view to the browser review-ui that mirrors `vouch claims-clear`: a GET renders a dry-run preview of every auto-approved durable claim (optionally filtered by a --before date), and a POST archives them under a single `claim.bulk_clear` audit event. routes through `lifecycle.clear_claims`, so the audit trail is identical to the cli and mcp surfaces — no parallel data path. auto_only is fixed on, so the console can only ever clear auto-approved calibration cruft, never human-reviewed knowledge. progressive-enhancement friendly (plain form posts) with a js confirm() guard and a hub refresh broadcast. * fix(claims): satisfy ruff and regenerate claim/proposal schemas the bulk-clear feature landed with three ruff violations in the `claims-clear` cli handler (B904 missing `raise ... from`, E501 long line, SIM102 nested `if`) and an unused `claim2` binding in the test (F841), plus stale committed schemas — the additive `auto_approved` / `proposed_by` claim fields and the `delete` proposal kind weren't reflected in `schemas/`. regenerate via `scripts/gen_schemas.py` and clear the lint errors so the ci lint/type/drift gates pass.
github-actions
Bot
added
review-ui
* feat(transcript): locate raw Claude session files by id * feat(transcript): parse Claude JSONL into normalized blocks * feat(transcript): orchestrate load with observation fallback * feat(transcript): expose kb.session_transcript across all surfaces * feat(webapp): transcript client types and fetch * feat(webapp): thinking, diff, and code block renderers * feat(webapp): tool block with per-tool rendering and diffs * feat(webapp): sessions tab with full transcript viewer * feat(transcript): parse Codex rollouts into normalized blocks * test(webapp): cover degraded render and subagent drill-down * test(webapp): e2e smoke for the sessions transcript tab * docs(transcript): session transcript viewer design and plan * refactor(webapp): merge session transcript into Review, drop Sessions tab * docs(transcript): note the merge of the viewer into Review * feat(activity): expose kb.activity audit buckets across all surfaces one pass over the audit log returning per-day counts (with proposal/decision breakdowns), an hour-of-week matrix, and actor/event histograms — the data a dashboard needs that kb.audit's tail-limited raw events and kb.stats' review aggregates don't cover. windowed in viewer-local calendar days so the oldest heatmap cell is never partially counted (days=0 for all-time, negatives rejected). local bucketing prefers an iana tz name (dst-correct across the year) and falls back to a clamped fixed utc offset. scope-filtered through the same viewer context as kb.audit so multi-project kbs don't leak actor names or activity timing across project boundaries. registered at all four sites (mcp tool, jsonl handler, capabilities methods, vouch activity cli) plus trust read methods. * feat(webapp): dashboard view of kb activity, landing on / new /dashboard view driven by kb.activity: a 12-month activity calendar (ember heat ramp themed for dark and light via --heat-* css vars), last-30-days bars, an hour-of-week heatmap, and top-actor / event-mix bar lists, with stat tiles fed by kb.status and kb.stats. the calendar fetches 371 days so its oldest drawn column always sits inside the server window, sends the browser's iana timezone for dst-correct bucketing, and degrades cleanly: endpoints that don't advertise kb.activity get an upgrade hint, empty audit logs get an empty state, and a malformed stats payload no longer crashes the view. / now redirects to the dashboard instead of chat and the tab leads the sidebar; the e2e smoke flow clicks through to chat accordingly.
…#456) * feat(transcript): locate raw Claude session files by id * feat(transcript): parse Claude JSONL into normalized blocks * feat(transcript): orchestrate load with observation fallback * feat(transcript): expose kb.session_transcript across all surfaces * feat(webapp): transcript client types and fetch * feat(webapp): thinking, diff, and code block renderers * feat(webapp): tool block with per-tool rendering and diffs * feat(webapp): sessions tab with full transcript viewer * feat(transcript): parse Codex rollouts into normalized blocks * test(webapp): cover degraded render and subagent drill-down * test(webapp): e2e smoke for the sessions transcript tab * docs(transcript): session transcript viewer design and plan * refactor(webapp): merge session transcript into Review, drop Sessions tab * docs(transcript): note the merge of the viewer into Review * feat(console): serve the react web console from the installed package add `vouch console`: a starlette app that serves the vendored webapp SPA plus a same-origin /proxy bridge to `vouch serve --transport http` backends — a python port of the vite dev-proxy (loopback-guarded, reads X-Vouch-Target, passes backend statuses through unmasked). the built SPA is bundled into the wheel as vouch/web/console via a conditional hatch build hook, so `pip install 'vouch-kb[web]'` then `vouch console` needs no node and no repo clone. wire the release workflow to build the webapp before packaging and build the wheel from the working tree so the console rides inside it. bump to 1.3.0 across the four version sites, document the pip path in the readme, and add a changelog entry.
github-actions
Bot
added
openclaw
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vouch/capabilities.py (1)
30-97: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd the missing
kb.session_transcriptCLI mirror
kb.activityalready has the four-place registration and JSONL coverage.kb.session_transcriptis present insrc/vouch/server.py,src/vouch/jsonl_server.py, andsrc/vouch/capabilities.py, but notsrc/vouch/cli.py; add the CLI command to keep the tool surface consistent.tests/test_session_transcript.pyalready covers the JSONL envelope.🤖 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/capabilities.py` around lines 30 - 97, The CLI is missing the kb.session_transcript command despite server and capability support. Add a corresponding CLI registration and handler in src/vouch/cli.py, mirroring the existing kb.activity command’s four-place registration, argument handling, and output behavior; use the existing session_transcript implementation and preserve the JSONL envelope covered by tests.Source: Coding guidelines
🤖 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 `@docs/superpowers/specs/2026-07-10-session-transcript-viewer-design.md`:
- Around line 304-308: Update the e2e smoke-test description to reference the
Review tab instead of the removed standalone Sessions tab: instruct opening
Review, selecting a session, and verifying the rendered transcript, while
preserving the existing mocked proxy endpoints and frontend-only test setup.
In `@tests/test_stats.py`:
- Around line 269-275: Extend test_jsonl_kb_activity to assert the successful
response id and exact success envelope fields, then add a failure-case request
for kb.activity with invalid parameters and assert the response has the expected
id, ok=False, and an error field while excluding result.
In `@webapp/src/components/transcript/ThinkingBlock.tsx`:
- Around line 8-14: Add aria-expanded={open} to the collapsible toggle button in
the ThinkingBlock component, alongside its existing onClick handler and
className, so assistive technologies receive the current expanded state.
In `@webapp/src/views/TranscriptView.tsx`:
- Around line 47-52: Reset TranscriptView state when the selected session
changes by adding a key derived from sessionId to the TranscriptView usage in
ReviewView, forcing remount and reinitializing stack. Locate the TranscriptView
render in ReviewView and ensure the key changes whenever sessionId changes.
---
Outside diff comments:
In `@src/vouch/capabilities.py`:
- Around line 30-97: The CLI is missing the kb.session_transcript command
despite server and capability support. Add a corresponding CLI registration and
handler in src/vouch/cli.py, mirroring the existing kb.activity command’s
four-place registration, argument handling, and output behavior; use the
existing session_transcript implementation and preserve the JSONL envelope
covered by tests.
🪄 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: 93ab1e1d-8591-4b55-a6b5-ba67f4144394
📒 Files selected for processing (35)
CHANGELOG.mddocs/superpowers/plans/2026-07-10-session-transcript-viewer.mddocs/superpowers/specs/2026-07-10-session-transcript-viewer-design.mdsrc/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/stats.pysrc/vouch/transcript.pysrc/vouch/trust.pytests/test_session_transcript.pytests/test_stats.pytests/test_trust.pywebapp/e2e/review-transcript.spec.tswebapp/e2e/smoke.spec.tswebapp/src/App.test.tsxwebapp/src/App.tsxwebapp/src/components/Shell.tsxwebapp/src/components/transcript/CodeBlock.tsxwebapp/src/components/transcript/DiffView.tsxwebapp/src/components/transcript/MessageBlock.tsxwebapp/src/components/transcript/ThinkingBlock.tsxwebapp/src/components/transcript/ToolBlock.test.tsxwebapp/src/components/transcript/ToolBlock.tsxwebapp/src/components/transcript/blocks.test.tsxwebapp/src/lib/transcript.test.tswebapp/src/lib/transcript.tswebapp/src/lib/types.tswebapp/src/styles.csswebapp/src/views/DashboardView.test.tsxwebapp/src/views/DashboardView.tsxwebapp/src/views/ReviewView.test.tsxwebapp/src/views/ReviewView.tsxwebapp/src/views/TranscriptView.test.tsxwebapp/src/views/TranscriptView.tsx
✅ Files skipped from review due to trivial changes (4)
- webapp/src/lib/transcript.test.ts
- webapp/src/components/transcript/blocks.test.tsx
- CHANGELOG.md
- docs/superpowers/plans/2026-07-10-session-transcript-viewer.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/vouch/jsonl_server.py
Comment on lines
+304
to
+308
| - `webapp/e2e/` smoke: open Sessions, pick a row, see the rendered transcript. | ||
| It stubs `/proxy/*` via Playwright `page.route` (health, capabilities, | ||
| `kb.list_pending`, `kb.list_sessions`, `kb.session_transcript`) so it drives | ||
| the real frontend independent of the backend build — the local `vouch` on | ||
| PATH is an editable install of a different checkout without the new RPC. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the e2e test plan to use Review, not Sessions.
The standalone Sessions tab was removed; this test should say to open Review, select a session, and verify the transcript there, matching Lines [221-230] and [332-338].
Suggested wording
-- webapp/e2e/ smoke: open Sessions, pick a row, see the rendered transcript.
+- webapp/e2e/ smoke: open Review, pick a session row, and see the rendered transcript.📝 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
| - `webapp/e2e/` smoke: open Sessions, pick a row, see the rendered transcript. | |
| It stubs `/proxy/*` via Playwright `page.route` (health, capabilities, | |
| `kb.list_pending`, `kb.list_sessions`, `kb.session_transcript`) so it drives | |
| the real frontend independent of the backend build — the local `vouch` on | |
| PATH is an editable install of a different checkout without the new RPC. | |
| - `webapp/e2e/` smoke: open Sessions, pick a row, see the rendered transcript. | |
| It stubs `/proxy/*` via Playwright `page.route` (health, capabilities, | |
| `kb.list_pending`, `kb.list_sessions`, `kb.session_transcript`) so it drives | |
| the real frontend independent of the backend build — the local `vouch` on | |
| PATH is an editable install of a different checkout without the new RPC. |
🤖 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-10-session-transcript-viewer-design.md` around
lines 304 - 308, Update the e2e smoke-test description to reference the Review
tab instead of the removed standalone Sessions tab: instruct opening Review,
selecting a session, and verifying the rendered transcript, while preserving the
existing mocked proxy endpoints and frontend-only test setup.
Comment on lines
+269
to
+275
| def test_jsonl_kb_activity(store: KBStore) -> None: | ||
| src = store.put_source(b"x") | ||
| propose_claim(store, text="a", evidence=[src.id], proposed_by="agent-a") | ||
| resp = handle_request({"id": "1", "method": "kb.activity", "params": {"days": 0}}) | ||
| assert resp["ok"] is True | ||
| assert resp["result"]["total_events"] >= 1 | ||
| assert len(resp["result"]["by_hour"]) == 7 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a failure-case test for the kb.activity JSONL envelope.
test_jsonl_kb_activity covers success but omits the failure envelope ({id, ok: false, error}) and doesn't assert resp["id"]. The coding guidelines require both success and failure envelope shape assertions for each new kb.* method.
As per coding guidelines: "For each new kb.* method, add a test that asserts the JSONL envelope shape: {id, ok, result} on success and {id, ok: false, error} on failure."
🧪 Suggested failure test
def test_jsonl_kb_activity(store: KBStore) -> None:
src = store.put_source(b"x")
propose_claim(store, text="a", evidence=[src.id], proposed_by="agent-a")
resp = handle_request({"id": "1", "method": "kb.activity", "params": {"days": 0}})
assert resp["ok"] is True
+ assert resp["id"] == "1"
assert resp["result"]["total_events"] >= 1
assert len(resp["result"]["by_hour"]) == 7
+
+
+def test_jsonl_kb_activity_error_envelope(store: KBStore) -> None:
+ resp = handle_request({"id": "2", "method": "kb.activity", "params": {"days": -1}})
+ assert resp["id"] == "2"
+ assert resp["ok"] is False
+ assert "error" in resp📝 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 test_jsonl_kb_activity(store: KBStore) -> None: | |
| src = store.put_source(b"x") | |
| propose_claim(store, text="a", evidence=[src.id], proposed_by="agent-a") | |
| resp = handle_request({"id": "1", "method": "kb.activity", "params": {"days": 0}}) | |
| assert resp["ok"] is True | |
| assert resp["result"]["total_events"] >= 1 | |
| assert len(resp["result"]["by_hour"]) == 7 | |
| def test_jsonl_kb_activity(store: KBStore) -> None: | |
| src = store.put_source(b"x") | |
| propose_claim(store, text="a", evidence=[src.id], proposed_by="agent-a") | |
| resp = handle_request({"id": "1", "method": "kb.activity", "params": {"days": 0}}) | |
| assert resp["ok"] is True | |
| assert resp["id"] == "1" | |
| assert resp["result"]["total_events"] >= 1 | |
| assert len(resp["result"]["by_hour"]) == 7 | |
| def test_jsonl_kb_activity_error_envelope(store: KBStore) -> None: | |
| resp = handle_request({"id": "2", "method": "kb.activity", "params": {"days": -1}}) | |
| assert resp["id"] == "2" | |
| assert resp["ok"] is False | |
| assert "error" in resp |
🤖 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 `@tests/test_stats.py` around lines 269 - 275, Extend test_jsonl_kb_activity to
assert the successful response id and exact success envelope fields, then add a
failure-case request for kb.activity with invalid parameters and assert the
response has the expected id, ok=False, and an error field while excluding
result.
Source: Coding guidelines
Comment on lines
+8
to
+14
| <button | ||
| onClick={() => setOpen((o) => !o)} | ||
| className="flex w-full items-center gap-2 px-3 py-1.5 text-left font-mono text-[10px] uppercase tracking-widest text-sepia" | ||
| > | ||
| <ChevronRight size={12} className={`transition ${open ? 'rotate-90' : ''}`} /> | ||
| <Brain size={12} /> Thinking | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add aria-expanded to the toggle button.
The button controls a collapsible section but doesn't communicate its expanded/collapsed state to assistive technology. Add aria-expanded={open} so screen readers announce the toggle state.
♿ Proposed fix
<button
onClick={() => setOpen((o) => !o)}
+ aria-expanded={open}
className="flex w-full items-center gap-2 px-3 py-1.5 text-left font-mono text-[10px] uppercase tracking-widest text-sepia"
>📝 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
| <button | |
| onClick={() => setOpen((o) => !o)} | |
| className="flex w-full items-center gap-2 px-3 py-1.5 text-left font-mono text-[10px] uppercase tracking-widest text-sepia" | |
| > | |
| <ChevronRight size={12} className={`transition ${open ? 'rotate-90' : ''}`} /> | |
| <Brain size={12} /> Thinking | |
| </button> | |
| <button | |
| onClick={() => setOpen((o) => !o)} | |
| aria-expanded={open} | |
| className="flex w-full items-center gap-2 px-3 py-1.5 text-left font-mono text-[10px] uppercase tracking-widest text-sepia" | |
| > | |
| <ChevronRight size={12} className={`transition ${open ? 'rotate-90' : ''}`} /> | |
| <Brain size={12} /> Thinking | |
| </button> |
🤖 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 `@webapp/src/components/transcript/ThinkingBlock.tsx` around lines 8 - 14, Add
aria-expanded={open} to the collapsible toggle button in the ThinkingBlock
component, alongside its existing onClick handler and className, so assistive
technologies receive the current expanded state.
Comment on lines
+47
to
+52
| const [stack, setStack] = useState<{ id: string; agent?: string }[]>([{ id: sessionId, agent }]) | ||
| const top = stack[stack.length - 1] | ||
| const q = useQuery({ | ||
| queryKey: ['transcript', conn.endpoint, top.id], | ||
| queryFn: () => fetchTranscript(conn, top.id, top.agent), | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Stack state doesn't reset when sessionId prop changes — user sees stale transcript.
useState initializes the stack from sessionId on first mount only. When the parent (ReviewView) passes a new sessionId (user selects a different session), the stack retains the old session ID, so top.id and the useQuery key stay unchanged. The user sees the previous session's transcript instead of the newly selected one.
Fix by adding a key prop in ReviewView.tsx to force remount:
- <TranscriptView
+ <TranscriptView
+ key={selected.s.session_id ?? undefined}
conn={selected.project.conn}
sessionId={selected.s.session_id as string}
/>Alternatively, reset the stack when sessionId changes via useEffect here:
+ useEffect(() => {
+ setStack([{ id: sessionId, agent }])
+ }, [sessionId, agent])The key prop approach is preferred — it's simpler and avoids an extra render cycle.
🤖 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 `@webapp/src/views/TranscriptView.tsx` around lines 47 - 52, Reset
TranscriptView state when the selected session changes by adding a key derived
from sessionId to the TranscriptView usage in ReviewView, forcing remount and
reinitializing stack. Locate the TranscriptView render in ReviewView and ensure
the key changes whenever sessionId changes.
* feat(transcript): locate raw Claude session files by id * feat(transcript): parse Claude JSONL into normalized blocks * feat(transcript): orchestrate load with observation fallback * feat(transcript): expose kb.session_transcript across all surfaces * feat(webapp): transcript client types and fetch * feat(webapp): thinking, diff, and code block renderers * feat(webapp): tool block with per-tool rendering and diffs * feat(webapp): sessions tab with full transcript viewer * feat(transcript): parse Codex rollouts into normalized blocks * test(webapp): cover degraded render and subagent drill-down * test(webapp): e2e smoke for the sessions transcript tab * docs(transcript): session transcript viewer design and plan * refactor(webapp): merge session transcript into Review, drop Sessions tab * docs(transcript): note the merge of the viewer into Review * feat(activity): expose kb.activity audit buckets across all surfaces one pass over the audit log returning per-day counts (with proposal/decision breakdowns), an hour-of-week matrix, and actor/event histograms — the data a dashboard needs that kb.audit's tail-limited raw events and kb.stats' review aggregates don't cover. windowed in viewer-local calendar days so the oldest heatmap cell is never partially counted (days=0 for all-time, negatives rejected). local bucketing prefers an iana tz name (dst-correct across the year) and falls back to a clamped fixed utc offset. scope-filtered through the same viewer context as kb.audit so multi-project kbs don't leak actor names or activity timing across project boundaries. registered at all four sites (mcp tool, jsonl handler, capabilities methods, vouch activity cli) plus trust read methods. * feat(webapp): dashboard view of kb activity, landing on / new /dashboard view driven by kb.activity: a 12-month activity calendar (ember heat ramp themed for dark and light via --heat-* css vars), last-30-days bars, an hour-of-week heatmap, and top-actor / event-mix bar lists, with stat tiles fed by kb.status and kb.stats. the calendar fetches 371 days so its oldest drawn column always sits inside the server window, sends the browser's iana timezone for dst-correct bucketing, and degrades cleanly: endpoints that don't advertise kb.activity get an upgrade hint, empty audit logs get an empty state, and a malformed stats payload no longer crashes the view. / now redirects to the dashboard instead of chat and the tab leads the sidebar; the e2e smoke flow clicks through to chat accordingly. * feat(console): add vouch-ui console, demo setup, and web integration add web-based console with dashboard view of kb activity and session management. integrate vouch-ui webapp, docker-compose demo environment, and llm-backed actions (compile + summarize). includes hatch build script and full test coverage. * fix(review): remove remaining conflict markers cleaned up merge conflict markers that were preventing the component from loading.
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