Feat/volunteer context#240
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds HTTP and web review surfaces, scoped retrieval and volunteered context, migrations and sync tooling, metrics and logging, diff and lifecycle commands, adapter installers and host configs, onboarding templates, bundle and storage validation, benchmarks, and extensive product, contributor, and agent documentation. ChangesPlatform expansion
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP as HTTP Server
participant JSONL as JSONL Dispatcher
participant KB as KBStore
Client->>HTTP: POST /rpc or /mcp
HTTP->>HTTP: validate bearer token
HTTP->>JSONL: dispatch kb.* request
JSONL->>KB: read/write KB data
KB-->>JSONL: result
JSONL-->>HTTP: response
HTTP-->>Client: JSON payload
sequenceDiagram
participant Session
participant Volunteer as volunteer_context
participant Retrieval
participant MCP
Session->>Volunteer: start session watch
Volunteer->>Retrieval: evaluate scoped claim relevance
Retrieval-->>Volunteer: ranked approved claims
Volunteer-->>MCP: push volunteered offer
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/vouch/jsonl_server.py (1)
573-625:⚠️ Potential issue | 🔴 CriticalAll 51 methods are missing from
server.pyand 50 of 51 are missing fromcli.py—registration is incomplete.The four-surface registration requirement is not met. Cross-check shows:
jsonl_server.py: 51 methods registered ✓capabilities.py: 51 methods registered ✓server.py: 0 of 51 methods registered ✗cli.py: 50 of 51 methods missing (onlykb.statuspresent) ✗Per coding guidelines, every
kb.*method must be registered in all four files before merge. Thetest_capabilitiestest should catch this drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/jsonl_server.py` around lines 573 - 625, All 51 methods defined in the HANDLERS dictionary in jsonl_server.py must be registered in both server.py and cli.py to meet the four-surface registration requirement. Add all 51 kb.* method handlers to server.py (currently 0 registered) and add the 50 missing method handlers to cli.py (which currently only has kb.status). Use the HANDLERS dictionary in jsonl_server.py as the source of truth for which methods need to be registered, ensuring consistency across all four registration points.Sources: Coding guidelines, Learnings
src/vouch/bundle.py (1)
488-543:⚠️ Potential issue | 🟠 Major | ⚡ Quick winavoid caching every bundle file body during
import_check.Line 529 stores all tar member bytes in
incoming, including potentially hugesources/*/contentfiles. this can spike memory and fail imports on large bundles.proposed fix
- incoming[member.name] = body + # keep full bytes only for artifacts parsed in _check_graph_integrity; + # for other paths, path-derived ids are enough. + if member.name.startswith(("claims/", "pages/", "relations/")): + incoming[member.name] = body + else: + incoming[member.name] = b""🤖 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/bundle.py` around lines 488 - 543, The current implementation stores every tar member body in the incoming dictionary at line 529, which can cause significant memory spikes when processing large bundle files. Instead of caching all file bodies in the incoming dictionary before calling _check_graph_integrity, restructure the code to either pass file bodies directly to _check_graph_integrity as they are validated, or modify _check_graph_integrity to accept a callback or generator that yields file data on demand rather than requiring all file contents to be pre-loaded into memory. This allows large content files to be processed and discarded rather than accumulated in the incoming dictionary throughout the entire bundle import check process.
🟠 Major comments (28)
src/vouch/server.py-540-548 (1)
540-548:⚠️ Potential issue | 🟠 MajorImport
get_contextfromfastmcp.server.dependenciesand call it directly.
mcp.get_context()will fail at runtime withAttributeError. Theget_context()function is a standalone utility infastmcp.server.dependencies, not a method on the FastMCP instance. Add the import and callget_context()directly instead ofmcp.get_context().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/server.py` around lines 540 - 548, The kb_session_start function incorrectly calls get_context() as a method on the mcp instance, which will fail at runtime with AttributeError. Fix this by importing get_context directly from fastmcp.server.dependencies at the top of the file, then replace the mcp.get_context() call with a direct call to get_context() in the kb_session_start function.src/vouch/onboarding.py-123-145 (1)
123-145:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftroute onboarding writes through the review gate
these paths write durable artifacts directly (
put_page/put_entity/put_claim) and mark them approved without going through proposal approval flow. that creates a parallel write path and weakens lifecycle/audit invariants for seeded content. please stage these through the same review-gated approval pipeline used by other kb writes.Based on learnings: all writes to the knowledge base must go through the
proposals.approve()review gate; never create parallel data paths that bypass this invariant.Also applies to: 236-268
🤖 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/onboarding.py` around lines 123 - 145, The _starter_page function is directly writing to the knowledge base by calling store.put_page(page) without routing through the review approval gate. This creates a parallel write path that bypasses the proposals.approve() review pipeline used by all other knowledge base writes. Replace the direct store.put_page(page) call with the proper review-gated approval flow by creating a proposal for the page artifact and passing it through the proposals.approve() method, ensuring the starter page write follows the same lifecycle and audit invariants as all other knowledge base writes.Source: Learnings
src/vouch/migrations/runner.py-194-233 (1)
194-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winblock re-apply when a step may be partially applied
apply()recalculates diffs from current bytes and journals only what still differs. after a mid-step crash, a second apply can succeed with a partial journal; thenrollback()may revert only that subset and leave already-transformed files behind while schema version moves backward. add a preflight guard to detect unfinished migration state (for example, latest journal/header not aligned with current schema) and force explicit recovery before continuing.Also applies to: 259-293
🤖 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/migrations/runner.py` around lines 194 - 233, The apply() function needs a preflight guard at its beginning to detect unfinished migration states where the latest journal and header are not aligned with the current schema version. Add a check before the existing _pending_proposals_guard(store) call that validates the migration state is consistent, and raise an exception (or return an error state) if the journal entries or manifest headers indicate a partial application that was interrupted, forcing the user to perform explicit recovery before allowing the migration to proceed.src/vouch/vault_sync.py-299-303 (1)
299-303:⚠️ Potential issue | 🟠 Major | ⚡ Quick winpreserve edited page id when filing proposals
Line 299 parses
page_id, but the proposal at Line 335-345 does not pass it through.propose_pagewill derive a new id from title, so a title edit can turn into a new page proposal instead of an edit proposal for the existing page.suggested patch
@@ page_id = _page_id_from_frontmatter(text) if not page_id: result.pages_skipped_unknown_id.append(rel) continue + try: + store.get_page(page_id) + except ArtifactNotFoundError: + result.pages_skipped_unknown_id.append(rel) + continue @@ propose_page( store, title=edited.title, body=edited.body, page_type=str(edited.type.value if hasattr(edited.type, "value") else edited.type), claim_ids=list(edited.claims), entity_ids=list(edited.entities), source_ids=list({*edited.sources, source.id}), + slug_hint=page_id, proposed_by=actor, tags=list(edited.tags), )Also applies to: 335-346
🤖 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/vault_sync.py` around lines 299 - 303, The page_id extracted by _page_id_from_frontmatter at line 299 is not being passed to the propose_page function call in the proposal creation block (around lines 335-345). This causes propose_page to derive a new id from the title instead of using the existing page_id, which can incorrectly turn title edits into new page proposals. Pass the extracted page_id variable to the propose_page function call to preserve the edited page identity.src/vouch/vault_sync.py-225-227 (1)
225-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winbackward sync should prune orphaned mirrored markdown files
The sync treats
<vault>/vouch/*as vouch-managed, but this pass only overwrites known ids and never removes old files. Removed/archived artifacts can remain indefinitely and drift the mirror from KB truth.suggested patch
@@ # Pages + expected_pages: set[str] = set() for page in _approved_pages(store): text = _serialize_page(page) dst = mirror / f"{page.id}.md" dst.write_text(text, encoding="utf-8") result.pages_mirrored.append(page.id) + expected_pages.add(f"{page.id}.md") @@ # Claim stubs + expected_claims: set[str] = set() for claim in _approved_claims(store): @@ dst = claims_out / f"{claim.id}.md" dst.write_text(body, encoding="utf-8") result.claims_mirrored.append(claim.id) + expected_claims.add(f"{claim.id}.md") + + for f in mirror.glob("*.md"): + if f.name not in expected_pages: + f.unlink() + for f in claims_out.glob("*.md"): + if f.name not in expected_claims: + f.unlink()Also applies to: 235-253, 257-263
🤖 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/vault_sync.py` around lines 225 - 227, The backward sync operation creates or overwrites files in the mirror and claims_out directories for known artifact IDs, but does not remove orphaned markdown files that correspond to archived or deleted artifacts. After the mkdir calls on mirror and claims_out directories and after all known artifact IDs have been synced/written to their respective directories, implement a cleanup step that iterates through existing files in both the mirror and claims_out directories, compares them against the set of known/current artifact IDs that were just processed, and removes any files that do not have a corresponding known ID. This ensures the mirror stays in sync with the knowledge base by pruning files for removed or archived artifacts.src/vouch/sessions.py-36-37 (1)
36-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winmake volunteer hooks best-effort and always run end teardown on idempotent calls
Line 36 and Line 56 execute optional hooks inline after durable writes. If a hook raises, the api returns an error even though state already changed. Also, Line 43’s early return can skip
on_session_endon retries, which can leave background watch state dangling.suggested patch
@@ - volunteer_context.on_session_start(store, sess) + try: + volunteer_context.on_session_start(store, sess) + except Exception: + logger.exception("session_start: volunteer hook failed for %s", sess.id) @@ - if sess.ended_at is not None: - return sess # idempotent + if sess.ended_at is not None: + try: + volunteer_context.on_session_end(session_id) + except Exception: + logger.exception("session_end: volunteer hook failed for %s", session_id) + return sess # idempotent @@ - volunteer_context.on_session_end(session_id) + try: + volunteer_context.on_session_end(session_id) + except Exception: + logger.exception("session_end: volunteer hook failed for %s", session_id)Also applies to: 42-44, 56-57
🤖 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/sessions.py` around lines 36 - 37, The volunteer hooks on_session_start at line 36 and on_session_end at line 56 are executed inline after durable writes, causing API errors when hooks fail even though state has persisted, and the early return at line 43 can skip on_session_end during retries leaving background state dangling. Refactor these hooks to be best-effort by either wrapping them in try-except blocks that log but don't propagate exceptions, or by moving them to async execution. Additionally, ensure the early return at line 43 does not bypass the on_session_end call by restructuring the control flow so that on_session_end teardown always executes regardless of previous operations.src/vouch/web/server.py-343-346 (1)
343-346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winset auth cookie as secure on https requests
The bootstrap token cookie is
HttpOnlyandSameSite=Strict, but it is not markedSecure. On https deployments this weakens transport guarantees for the auth credential.suggested patch
@@ resp.set_cookie( _TOKEN_COOKIE, exc.token, - httponly=True, samesite="strict", path="/", + httponly=True, + samesite="strict", + path="/", + secure=(request.url.scheme == "https"), )🤖 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/web/server.py` around lines 343 - 346, The set_cookie call for the _TOKEN_COOKIE is missing the secure flag, which weakens transport security on HTTPS deployments since the cookie can then be sent over unencrypted connections. Add the secure=True parameter to the set_cookie method call in the bootstrap token cookie section to ensure the cookie is only transmitted over HTTPS connections.src/vouch/web/server.py-318-327 (1)
318-327:⚠️ Potential issue | 🟠 Major | ⚡ Quick winrestrict query-token auth to bootstrap navigation only
Line 327 currently authenticates non-GET requests using
?token=.... That contradicts the one-time bootstrap model and allows mutation requests with secrets in URLs (logs/history/referrers).suggested patch
@@ q = _query_token(request) if q is not None and auth.matches(q): if request.method in ("GET", "HEAD"): stripped = request.url.remove_query_params("token") location = stripped.path + (f"?{stripped.query}" if stripped.query else "") raise _BootstrapRedirect(location=location or "/", token=q) - return + raise HTTPException( + status_code=401, + detail="query token is only allowed for GET/HEAD bootstrap", + headers={"WWW-Authenticate": "Bearer"}, + )🤖 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/web/server.py` around lines 318 - 327, The unindented return statement at line 327 currently authenticates non-GET/HEAD requests using the query token, which violates the one-time bootstrap model and exposes secrets in URLs for mutation requests. Remove the return statement that follows the if block checking for GET/HEAD methods in the _query_token authentication logic. This ensures that only GET/HEAD requests are authenticated via the query token through the bootstrap redirect mechanism, while other HTTP methods are not authenticated by the query token at all..github/workflows/ci.yml-27-27 (1)
27-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winsync local setup docs with the ci extras set.
Line 27 installs
.[dev,web], but onboarding commands inAGENTS.md(Line 43) andCLAUDE.md(Line 72) still install.[dev]. this creates a ci/local env mismatch for contributors and can cause avoidable red checks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml at line 27, The pip installation extras in the CI workflow (ci.yml line 27) specifies `.[dev,web]` but the onboarding documentation in AGENTS.md (line 43) and CLAUDE.md (line 72) still reference `.[dev]` only. Update both documentation files to include the web extra in their pip install commands, changing `.[dev]` to `.[dev,web]` in both locations to ensure local development environments match the CI configuration and prevent contributors from encountering environment mismatch issues.adapters/zed/install.yaml-9-11 (1)
9-11:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftdeclare full t1-t4 tiers for new install-mcp hosts.
Line 9-11 defines only
T1, but new host manifests are required to declareT1-T4and ship the corresponding host structure/files. this currently under-specifies the adapter contract.as per coding guidelines: “when adding a new install-mcp host, create
adapters/<host>/withinstall.yaml(T1-T4 tier declarations),.mcp.json,CLAUDE.md.snippet,.claude/commands/*, and.claude/settings.json.”🤖 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 `@adapters/zed/install.yaml` around lines 9 - 11, Extend the tiers section in the install.yaml file to include complete T1 through T4 tier declarations. Currently only T1 is defined with the settings.json mapping. Add T2, T3, and T4 tier entries following the same structure pattern as T1, ensuring each tier includes appropriate source and destination file mappings that represent the complete host structure required by the adapter contract specification.Source: Coding guidelines
src/vouch/jsonl_server.py-149-156 (1)
149-156:⚠️ Potential issue | 🟠 Major | ⚡ Quick winavoid breaking
kb.searchresponse shape for existing clients.lines 149-156 now return an object (
backend/viewer/hits) instead of the prior top-level hit list. clients expectingresultas a list will break immediately.backward-compatible pattern
- return { - "backend": used, - "viewer": {"project": viewer.project, "agent": viewer.agent}, - "hits": [ - {"kind": k, "id": i, "snippet": sn, "score": sc, "backend": used} - for k, i, sn, sc in scoped - ], - } + hits_out = [ + {"kind": k, "id": i, "snippet": sn, "score": sc, "backend": used} + for k, i, sn, sc in scoped + ] + if bool(p.get("include_meta", False)): + return { + "backend": used, + "viewer": {"project": viewer.project, "agent": viewer.agent}, + "hits": hits_out, + } + return hits_out🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/jsonl_server.py` around lines 149 - 156, The current return statement in the kb.search response (lines 149-156) changes the response structure from a list of hits to an object containing backend, viewer, and hits fields, which breaks existing clients expecting a list. Modify the return statement to maintain backward compatibility by returning the list of hits as the primary response structure while preserving any necessary metadata in a way that doesn't break existing clients, or wrap the new object structure to include the original list format as the top-level response.schemas/claim.schema.json-145-145 (1)
145-145:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftkeep
Claim.scopebackward-compatible during the transition.line 145 switches
scopeto object-only. older artifacts using scalar scope values will fail validation/import, which is a breaking persisted-contract change.possible compatibility bridge
- "scope": { - "$ref": "`#/`$defs/ArtifactScope" - }, + "scope": { + "oneOf": [ + { "$ref": "`#/`$defs/ArtifactScope" }, + { "$ref": "`#/`$defs/Visibility" } + ] + },🤖 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 `@schemas/claim.schema.json` at line 145, The Claim.scope field at line 145 currently only references the ArtifactScope object definition, which breaks compatibility with older artifacts that use scalar scope values. Modify the scope property definition to accept both the ArtifactScope object reference and the previously supported scalar value types by using a union construct (such as anyOf or oneOf) that includes both the $ref to ArtifactScope and the allowed scalar type(s) that were previously valid.adapters/claude-code/.claude/settings.json-3-19 (1)
3-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winallow the new
kb_volunteer_contextmethod in the adapter allowlist.Lines 4-19 omit
mcp__vouch__kb_volunteer_context, so claude-code sessions can miss the new pull surface for volunteered context even though the feature is introduced in this PR.proposed fix
"mcp__vouch__kb_search", "mcp__vouch__kb_context", + "mcp__vouch__kb_volunteer_context", "mcp__vouch__kb_read_claim",🤖 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 `@adapters/claude-code/.claude/settings.json` around lines 3 - 19, The "allow" array in the settings.json file is missing the new mcp__vouch__kb_volunteer_context method, which prevents Claude from accessing this feature. Add mcp__vouch__kb_volunteer_context to the "allow" array alongside the other existing mcp__vouch__kb_* methods to enable the volunteer context surface for claude-code sessions.src/vouch/__init__.py-3-3 (1)
3-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winavoid regressing package version metadata
setting
__version__to0.0.1appears to roll back versioning for a feature-adding PR, which can break upgrade detection and cross-manifest consistency. keep this value aligned with the repository’s canonical release version.Based on learnings: "Bump
versioninpyproject.tomland sync it with theversionfield inopenclaw.plugin.jsonwhenever you release."🤖 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/__init__.py` at line 3, The `__version__` variable in the vouch package's __init__.py file is hardcoded to "0.0.1" which does not reflect the actual package version. Update the `__version__` value to match the version defined in pyproject.toml (the canonical release version), and ensure this same version is also consistent with the version field in openclaw.plugin.json if that file exists in the repository.Source: Learnings
adapters/claude-desktop/install.yaml-16-20 (1)
16-20:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftadd the required t3/t4 adapter tiers
this manifest currently declares only
T1andT2. the adapter contract for newadapters/*/install.yamlhosts requiresT1–T4; leaving outT3/T4creates install-surface drift.proposed manifest shape update
tiers: T1: - { src: claude_desktop_config.json, dst: claude-desktop/claude_desktop_config.json } T2: - { src: README.snippet.md, dst: claude-desktop/README.md, fenced_append: true } + T3: + - { src: .claude/commands/vouch-status.md, dst: claude-desktop/.claude/commands/vouch-status.md } + T4: + - { src: .claude/settings.json, dst: claude-desktop/.claude/settings.json }As per coding guidelines
adapters/*/install.yaml: "When adding a new install-mcp host ... containing ... T1-T4 tier declarations ..."🤖 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 `@adapters/claude-desktop/install.yaml` around lines 16 - 20, The install.yaml manifest for the claude-desktop adapter is incomplete as it only declares T1 and T2 tiers. According to the adapter contract requirements and coding guidelines, the tiers section must include all four tiers (T1-T4) to prevent install-surface drift. Add T3 and T4 tier entries to the tiers section in the same structure as the existing T1 and T2 entries, ensuring each tier contains the appropriate file mappings with their source and destination paths along with any required configuration options like fenced_append.Source: Coding guidelines
adapters/continue/install.yaml-10-12 (1)
10-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick windeclare the full t1-t4 tiers for this host manifest.
only
T1is present, so this host does not satisfy the required install-mcp tier contract.As per coding guidelines, “when adding a new install-mcp host, create
adapters/<host>/containinginstall.yaml(T1-T4),.mcp.json,CLAUDE.md.snippet,.claude/commands/*, and.claude/settings.json.”🤖 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 `@adapters/continue/install.yaml` around lines 10 - 12, The install.yaml file for the continue adapter only declares the T1 tier and is missing the required T2, T3, and T4 tiers. Add the three missing tier declarations (T2, T3, and T4) to the tiers section in the install.yaml file, each with appropriate configuration entries that follow the same structure as the existing T1 tier definition, to satisfy the install-mcp host manifest contract.Source: Coding guidelines
adapters/cline/install.yaml-9-15 (1)
9-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winadd the required t3/t4 tiers and canonical host scaffold.
this new host manifest only declares
T1/T2, so it does not meet the required install-mcp host contract and will drift from the expected adapter surface.As per coding guidelines, “when adding a new install-mcp host, create
adapters/<host>/containinginstall.yaml(T1-T4),.mcp.json,CLAUDE.md.snippet,.claude/commands/*, and.claude/settings.json.”🤖 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 `@adapters/cline/install.yaml` around lines 9 - 15, The install.yaml file for the cline host adapter is incomplete and only defines T1 and T2 tiers, but the install-mcp host contract requires T1-T4 tiers to be defined. Add T3 and T4 tier sections below the existing T2 tier in the cline install.yaml file, following the same structure pattern with appropriate src and dst file mappings for each tier. This ensures the adapter meets the required host manifest contract and provides the complete canonical scaffold.Source: Coding guidelines
adapters/codex/install.yaml-8-10 (1)
8-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick wincomplete the install tiers to t1-t4 for this new host.
this manifest currently defines only
T1, which breaks the required adapter-host install contract and leaves the host scaffold incomplete.As per coding guidelines, “when adding a new install-mcp host, create
adapters/<host>/containinginstall.yaml(T1-T4),.mcp.json,CLAUDE.md.snippet,.claude/commands/*, and.claude/settings.json.”🤖 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 `@adapters/codex/install.yaml` around lines 8 - 10, The install.yaml manifest in the codex adapter only defines the T1 tier, which violates the adapter-host install contract. Complete the manifest by adding the missing T2, T3, and T4 tiers below the existing T1 tier definition. Each tier should contain appropriate source-to-destination file mappings that follow the same pattern as T1 (using src and dst keys). Ensure all four tiers (T1 through T4) are properly defined with their respective file configurations to satisfy the required scaffold structure for new install-mcp hosts.Source: Coding guidelines
adapters/cursor/install.yaml-13-17 (1)
13-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winfinish this adapter manifest with the required t3/t4 layers.
T1/T2are present, but the new-host install contract requires all four tiers plus the canonical companion files.As per coding guidelines, “when adding a new install-mcp host, create
adapters/<host>/containinginstall.yaml(T1-T4),.mcp.json,CLAUDE.md.snippet,.claude/commands/*, and.claude/settings.json.”🤖 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 `@adapters/cursor/install.yaml` around lines 13 - 17, The install.yaml manifest in the cursor adapter is incomplete with only T1 and T2 tiers defined. Add the missing T3 and T4 tiers to the tiers section in install.yaml with appropriate file mappings for the remaining installation layers. Additionally, ensure all required canonical companion files exist in the adapters/cursor/ directory: .mcp.json, CLAUDE.md.snippet, .claude/commands/*, and .claude/settings.json, following the new-host install contract requirements.Source: Coding guidelines
src/vouch/http_server.py-182-193 (1)
182-193:⚠️ Potential issue | 🟠 Major | ⚡ Quick winenforce max body size while streaming the request body.
the current limit is enforced only from
Content-Length;await request.body()can still buffer an oversized chunked/misreported payload into memory.suggested fix
- raw = await request.body() + size = 0 + chunks: list[bytes] = [] + async for chunk in request.stream(): + size += len(chunk) + if size > MAX_BODY_BYTES: + return _json(413, _error_payload("invalid_request", "request body too large")) + chunks.append(chunk) + raw = b"".join(chunks)🤖 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/http_server.py` around lines 182 - 193, The current implementation only validates the Content-Length header against MAX_BODY_BYTES but does not enforce the limit while actually reading the request body with await request.body(). This allows clients to send chunked or misreported payloads that exceed the maximum size. Modify the code to read the request body in chunks, accumulating the total bytes read and checking against MAX_BODY_BYTES during the streaming process. Return a 413 error if the actual payload exceeds the limit, regardless of what the Content-Length header claims.src/vouch/http_server.py-436-444 (1)
436-444:⚠️ Potential issue | 🟠 Major | ⚡ Quick winnormalize accepted tokens before applying the non-loopback safety gate.
the public-bind check uses raw
tokenstruthiness, but auth middleware drops empty entries. a value liketokens=[""]can satisfy the bind gate while effectively disabling auth.suggested fix
- accepted = list(tokens) + ([token] if token else []) + accepted = [t.strip() for t in tokens if isinstance(t, str) and t.strip()] + if token is not None and token.strip(): + accepted.append(token.strip()) if host not in _LOOPBACK_HOSTS and not (allow_public and accepted): raise RuntimeError( f"refusing to bind non-loopback host {host!r} without both " "--allow-public and at least one bearer token " "(set VOUCH_HTTP_TOKEN / pass --token / use config.yaml serve.bearer_tokens)" ) - app = make_app(token=token, tokens=tokens) + app = make_app(tokens=accepted)🤖 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/http_server.py` around lines 436 - 444, The non-loopback safety gate check uses the raw `accepted` list directly in the condition, but this list may contain empty string tokens that are truthy as a list container but are dropped by the auth middleware, allowing the security check to pass when it shouldn't. Before the safety gate validation that checks `not (allow_public and accepted)`, normalize the `accepted` list by filtering out any empty or falsy token values. This ensures the check correctly validates that at least one meaningful bearer token exists when binding to non-loopback hosts, preventing cases where `tokens=[""]` would incorrectly satisfy the security requirement.adapters/windsurf/install.yaml-8-10 (1)
8-10:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftadd T2-T4 tier declarations for this new host manifest.
this manifest defines only
T1, but the new-host adapter contract requires tier declarations throughT4.based on learnings:
adapters/*/install.yaml: "when adding a new install-mcp host, create the directory structureadapters/<host>/containing:install.yaml(T1-T4 tier declarations),.mcp.json(T1 MCP wire),CLAUDE.md.snippet(T2 fenced append),.claude/commands/*(T3 slash commands), and.claude/settings.json(T4 hooks). use strict YAML manifest validation."🤖 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 `@adapters/windsurf/install.yaml` around lines 8 - 10, The tiers section in the install.yaml manifest is incomplete and only declares T1, but the new-host adapter contract requires tier declarations through T4. Add tier declarations for T2, T3, and T4 to the tiers section following the same structure as the existing T1 entry, ensuring each tier properly maps the appropriate configuration files according to the adapter contract requirements (T2 for fenced append snippets, T3 for slash commands, and T4 for hooks).Source: Coding guidelines
adapters/http-tunnel/fly.toml-4-6 (1)
4-6:⚠️ Potential issue | 🟠 Major | ⚡ Quick winbootstrap token name mismatch can block first fly deploy.
the quickstart sets
VOUCH_TOKEN, but this template’s documented fallback isVOUCH_HTTP_TOKEN. with--allow-publicand a fresh/datavolume, startup can fail before any config-backed token exists.Also applies to: 21-25
🤖 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 `@adapters/http-tunnel/fly.toml` around lines 4 - 6, The bootstrap token environment variable name in the fly.toml file has a mismatch between the quickstart command documentation and the template's expected fallback variable. The fly secrets set command is currently setting VOUCH_TOKEN, but the template's documented fallback expects VOUCH_HTTP_TOKEN instead. Change all occurrences of VOUCH_TOKEN to VOUCH_HTTP_TOKEN in the fly.toml file (including the fly secrets set command and other locations mentioned at lines 21-25) to ensure the bootstrap token is properly recognized on first deploy, preventing startup failures before any config-backed token can be established.src/vouch/health.py-496-553 (1)
496-553:⚠️ Potential issue | 🟠 Major | ⚡ Quick winwire
on_progressinto rebuild phases; it is currently dead code.
_tick()is defined but never called, so the newon_progressAPI never emits updates.proposed patch
def rebuild_index(store: KBStore, *, on_progress: Callable[[str], None] | None = None) -> dict: @@ index_db.reset(store.kb_dir) with index_db.open_db(store.kb_dir) as conn: + _tick("claims") for c in store.list_claims(): index_db.index_claim( @@ + _tick("pages") for p in store.list_pages(): index_db.index_page( @@ + _tick("entities") for e in store.list_entities(): index_db.index_entity( @@ + _tick("embeddings") _rebuild_embeddings(store) return index_db.stats(store.kb_dir)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/health.py` around lines 496 - 553, The _tick() helper function defined in rebuild_index is never invoked, making the on_progress callback dead code. Wire the _tick() function into each phase of the rebuild process by calling _tick() with the appropriate phase label before each processing section: call _tick("claims") before the loop that iterates over store.list_claims(), call _tick("pages") before the loop that iterates over store.list_pages(), call _tick("entities") before the loop that iterates over store.list_entities(), and call _tick("embeddings") before the _rebuild_embeddings() function call. This ensures progress updates are emitted as documented in the function's docstring.schemas/source.schema.json-3-37 (1)
3-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick winpreserve legacy
scopestring compatibility during the transition
Source.scopenow accepts only the object form. that conflicts with the stated back-compat behavior for existing artifacts that still usescope: project, and creates contract drift for schema-driven validators/importers.proposed schema adjustment
"scope": { - "$ref": "`#/`$defs/ArtifactScope" + "oneOf": [ + { "$ref": "`#/`$defs/ArtifactScope" }, + { "$ref": "`#/`$defs/Visibility" } + ], + "default": "project" },Also applies to: 119-121
🤖 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 `@schemas/source.schema.json` around lines 3 - 37, The ArtifactScope schema definition currently only accepts the object form with visibility, project, and agent properties, but this breaks backward compatibility with existing artifacts that use the legacy string form (e.g., scope: "project"). Modify the ArtifactScope schema to support both forms by using anyOf at the root level: one option for the current object schema with the visibility, project, and agent properties, and another option for accepting a string type to maintain legacy string compatibility.src/vouch/install_adapter.py-122-133 (1)
122-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winvalidate manifest paths to prevent escaping adapter/target roots
src/dstare only checked as non-empty strings. absolute paths or..segments can escapesrc_root/target, which allows unintended file reads/writes outside the install boundary.proposed hardening
+ src_rel = Path(src) + dst_rel = Path(dst) + if src_rel.is_absolute() or ".." in src_rel.parts: + raise AdapterError( + f"{host}: install.yaml tier {tier_name}: `src` must be a relative path" + ) + if dst_rel.is_absolute() or ".." in dst_rel.parts: + raise AdapterError( + f"{host}: install.yaml tier {tier_name}: `dst` must stay within target" + ) fenced = bool(raw.get("fenced_append", False)) parsed_entries.append(_FileEntry(src=src, dst=dst, fenced_append=fenced)) @@ - src = src_root / entry.src - dst = target / entry.dst + src = (src_root / entry.src).resolve() + dst = (target / entry.dst).resolve() + if not src.is_relative_to(src_root.resolve()): + raise AdapterError(f"{adapter}: src {entry.src!r} escapes adapter directory") + if not dst.is_relative_to(target): + raise AdapterError(f"{adapter}: dst {entry.dst!r} escapes target directory")Also applies to: 198-220
🤖 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/install_adapter.py` around lines 122 - 133, The src and dst path validation in the function only checks for non-empty strings but does not prevent absolute paths or path traversal sequences like .., which could allow escaping the adapter and target root directories. Add validation after the existing non-empty string checks to ensure both src and dst are relative paths (not absolute) and do not contain path traversal components. This validation should occur before creating the _FileEntry object to prevent files from being read or written outside the intended installation boundaries.skills/pr-precheck/SKILL.md-38-38 (1)
38-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winfix the install command to match the package metadata
Line 38 says
pip install vouch-kb, but the project metadata inpyproject.tomlis nowname = "vouch". this will send users to the wrong install target.proposed doc fix
-- **`vouch` CLI** on `PATH` (`pip install vouch-kb` ≥ the version that ships `pr-cache`). +- **`vouch` CLI** on `PATH` (`pip install vouch` ≥ the version that ships `pr-cache`).🤖 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 `@skills/pr-precheck/SKILL.md` at line 38, The documentation in SKILL.md line 38 instructs users to install the package using `pip install vouch-kb`, but the actual package name defined in pyproject.toml is `vouch`. Update the installation command in SKILL.md from `pip install vouch-kb` to `pip install vouch` to match the current package metadata and ensure users install the correct package.src/vouch/hot_memory.py-54-56 (1)
54-56:⚠️ Potential issue | 🟠 Major | ⚡ Quick windeactivate prior session state when replacing the same
session_id
register()overwrites_registry[session_id]but leaves any previously returnedHotMemoryobject withactive=True. if a watcher still holds that object, it can keep running and emit duplicate pushes.proposed fix
with _lock: + prev = _registry.get(session_id) + if prev is not None: + prev.active = False _registry[session_id] = mem🤖 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/hot_memory.py` around lines 54 - 56, The register() function overwrites the _registry[session_id] entry without deactivating the previously stored HotMemory object, allowing old watchers to continue running and emitting duplicate pushes. Before assigning the new mem object to _registry[session_id], check if a prior HotMemory instance already exists for that session_id and deactivate it by setting its active attribute to False before the overwrite occurs.
🧹 Nitpick comments (3)
src/vouch/sync.py (1)
148-154: 💤 Low valueSame
extractfile()None risk in_load_bundle_source.Line 154 has the same pattern where
extractfile(member)could returnNoneifmanifest.jsonwere somehow a non-regular file (unlikely but defensive coding would catch malformed bundles).🤖 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/sync.py` around lines 148 - 154, The `_load_bundle_source` function has a defensive coding issue where `tar.extractfile(member)` could return `None` if the manifest.json file is not a regular file within the tar archive. Add a null check after calling `tar.extractfile(member)` to verify the result is not `None` before attempting to call `.read()` on it. If `extractfile()` returns `None`, raise a descriptive RuntimeError similar to the existing manifest validation error to handle malformed bundles gracefully.src/vouch/stats.py (1)
16-17: 💤 Low valueImporting private functions creates coupling to internal implementations.
_yaml_loadfromstorageand_load_claims_for_lintfromhealth(line 88) are private functions. If these internals change, this module will break silently. Consider either making these public APIs or duplicating the minimal logic needed.🤖 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/stats.py` around lines 16 - 17, The import of `_yaml_load` from storage module creates coupling to private implementation details (functions prefixed with underscore are internal APIs). The comment also notes `_load_claims_for_lint` from health has the same issue. To fix this, either refactor these private functions to be public by removing the underscore prefix and treating them as public APIs, or alternatively duplicate the minimal logic needed from these private functions directly in the stats module to eliminate the dependency on internal implementations. Choose the approach based on whether the logic is general-purpose enough to be a public API or specific to stats module requirements.src/vouch/health.py (1)
359-372: ⚡ Quick winguard proposal-kind lookup to keep fsck from aborting on unmapped kinds.
presence[pr.kind]assumes only four kinds. if an approved proposal kind falls outside that set, fsck raisesKeyErrorand stops reporting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/health.py` around lines 359 - 372, The code directly accesses presence[pr.kind] without checking if pr.kind exists in the presence dictionary. If an approved proposal has a kind that is not one of the expected kinds, this raises a KeyError and stops the fsck process. Guard the lookup by checking if pr.kind exists in presence before accessing it, or handle the case where pr.kind is not found by adding an appropriate error finding similar to how the missing artifact_id case is handled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b8fa4735-1f87-4638-bd9e-bc46c70cd977
📒 Files selected for processing (144)
.github/workflows/ci.yml.github/workflows/release.ymlAGENTS.mdCHANGELOG.mdCLAUDE.mdCONTRIBUTING.mdREADME.mdROADMAP.mdadapters/claude-code/.claude/commands/vouch-propose-from-pr.mdadapters/claude-code/.claude/commands/vouch-recall.mdadapters/claude-code/.claude/commands/vouch-resolve-issue.mdadapters/claude-code/.claude/commands/vouch-status.mdadapters/claude-code/.claude/settings.jsonadapters/claude-code/install.yamladapters/claude-desktop/README.mdadapters/claude-desktop/README.snippet.mdadapters/claude-desktop/claude_desktop_config.jsonadapters/claude-desktop/install.yamladapters/cline/README.mdadapters/cline/cline_mcp_servers.jsonadapters/cline/install.yamladapters/cline/vscode_settings.jsonadapters/codex/install.yamladapters/continue/install.yamladapters/cursor/AGENTS.md.snippetadapters/cursor/install.yamladapters/http-tunnel/Dockerfileadapters/http-tunnel/README.mdadapters/http-tunnel/cloudflare-tunnel/compose.ymladapters/http-tunnel/fly.tomladapters/windsurf/README.mdadapters/windsurf/install.yamladapters/windsurf/mcp_config.jsonadapters/zed/README.mdadapters/zed/install.yamladapters/zed/settings.jsonbenchmarks/bench_bundle.pybenchmarks/bench_index_rebuild.pybenchmarks/bench_propose.pybenchmarks/bench_search.pybenchmarks/conftest.pydocs/README.mddocs/gittensor.mddocs/metrics.mddocs/migrations.mddocs/multi-agent.mddocs/provenance.mddocs/review-ui.mddocs/superpowers/plans/2026-05-27-claude-code-adapter.mddocs/superpowers/specs/2026-05-25-vouch-diff-design.mdllms.txtmigrations/README.mdopenclaw.plugin.jsonproposals/README.mdproposals/VEP-0004-http-transport.mdproposals/VEP-0005-richer-scopes.mdpyproject.tomlschemas/capabilities.schema.jsonschemas/claim.schema.jsonschemas/source.schema.jsonskills/pr-precheck/SKILL.mdspec/transports.mdsrc/vouch/__init__.pysrc/vouch/bundle.pysrc/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/diff.pysrc/vouch/embeddings/similarity.pysrc/vouch/health.pysrc/vouch/hot_memory.pysrc/vouch/http_server.pysrc/vouch/index_db.pysrc/vouch/install_adapter.pysrc/vouch/jsonl_server.pysrc/vouch/logging_config.pysrc/vouch/metrics.pysrc/vouch/migrations/__init__.pysrc/vouch/migrations/_legacy.pysrc/vouch/migrations/journal.pysrc/vouch/migrations/manifest.pysrc/vouch/migrations/rewriter.pysrc/vouch/migrations/runner.pysrc/vouch/migrations/schema.pysrc/vouch/migrations/semver.pysrc/vouch/models.pysrc/vouch/onboarding.pysrc/vouch/pr_cache.pysrc/vouch/proposals.pysrc/vouch/provenance/__init__.pysrc/vouch/provenance/cache.pysrc/vouch/provenance/graph.pysrc/vouch/provenance/model.pysrc/vouch/provenance/query.pysrc/vouch/scoping.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/stats.pysrc/vouch/storage.pysrc/vouch/sync.pysrc/vouch/vault_sync.pysrc/vouch/volunteer_context.pysrc/vouch/web/__init__.pysrc/vouch/web/server.pysrc/vouch/web/static/app.csssrc/vouch/web/static/app.jssrc/vouch/web/templates/audit.htmlsrc/vouch/web/templates/base.htmlsrc/vouch/web/templates/claim.htmlsrc/vouch/web/templates/queue.htmlsrc/vouch/web/templates/session.htmlsrc/vouch/web/templates/source.htmltemplates/config.template.yamltests/embeddings/test_search.pytests/fixtures/migrations/0099-test-rename.yamltests/test_bundle.pytests/test_cli.pytests/test_context.pytests/test_diff.pytests/test_expire.pytests/test_health.pytests/test_http_server.pytests/test_http_server_mcp.pytests/test_install_adapter.pytests/test_jsonl_server.pytests/test_logging.pytests/test_logging_config.pytests/test_metrics.pytests/test_migrations.pytests/test_pr_cache.pytests/test_propose_similarity.pytests/test_provenance.pytests/test_retrieval_backend.pytests/test_schema_migrations.pytests/test_scoping.pytests/test_sessions.pytests/test_stats.pytests/test_storage.pytests/test_sync.pytests/test_vault_sync.pytests/test_volunteer_context.pytests/test_web.pytests/test_web_e2e.pyweb/index.html
💤 Files with no reviewable changes (1)
- .github/workflows/release.yml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (3)
adapters/claude-code/.claude/commands/vouch-recall.md (1)
13-13:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # verify parameter names for kb.context across surfaces and tests rg -n -C3 'def _h_context|p\["task"\]|kb\.context|kb_context|query=|task=' \ src/vouch/jsonl_server.py src/vouch/server.py adapters/claude-code/.claude/commands/vouch-recall.md testsRepository: vouchdev/vouch
Length of output: 22380
Fix parameter name in kb_context call: use
taskinstead ofquery.Line 13 calls
kb_contextwithquery: "$ARGUMENTS", but both the MCP tool definition (src/vouch/server.py, line 174) and the JSONL handler (src/vouch/jsonl_server.py, line 163) expect the parameter to be namedtask. The command will fail with a missing parameter error.Change line 13 to:
1. Call `kb_context` with `task: "$ARGUMENTS"`.🤖 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 `@adapters/claude-code/.claude/commands/vouch-recall.md` at line 13, The kb_context call is using an incorrect parameter name. The parameter should be named `task` instead of `query` to match the expected interface defined in both the MCP tool definition and the JSONL handler. Update the kb_context call to use `task: "$ARGUMENTS"` instead of `query: "$ARGUMENTS"` to ensure the command passes the correct parameter name.src/vouch/server.py (1)
80-88:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify new kb.* methods are registered in all four places methods=( "kb_stats" "kb_expire" "kb_volunteer_context" "kb_why" "kb_trace" "kb_impact" "kb_graph_export" "kb_provenance_rebuild" ) echo "=== Checking registrations ===" for m in "${methods[@]}"; do echo "" echo "--- $m ---" echo "server.py:" rg -n "def $m" src/vouch/server.py | head -1 echo "jsonl_server.py:" rg -n "$m" src/vouch/jsonl_server.py | head -2 echo "capabilities.py:" rg -n "$m" src/vouch/capabilities.py | head -1 echo "cli.py:" rg -n "$m" src/vouch/cli.py | head -1 doneRepository: vouchdev/vouch
Length of output: 1209
All eight new
kb.*methods require registration injsonl_server.py,capabilities.py, andcli.py.The following methods are defined in
server.pybut missing from the other three required registration points:
kb_stats(line 81)kb_expire(line 460)kb_volunteer_context(line 552)kb_why(line 725)kb_trace(line 735)kb_impact(line 742)kb_graph_export(line 751)kb_provenance_rebuild(line 759)Per the coding guidelines, each new
kb.*method must be registered in exactly four places:server.py(✓),jsonl_server.py(as handler withHANDLERSmap entry),capabilities.py(appended toMETHODS), andcli.py(as CLI mirror). Thetest_capabilitiestest validates this requirement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/server.py` around lines 80 - 88, The eight kb.* methods (kb_stats, kb_expire, kb_volunteer_context, kb_why, kb_trace, kb_impact, kb_graph_export, and kb_provenance_rebuild) are defined in server.py but are missing required registrations in three other files. Add each method to jsonl_server.py by creating handler functions and adding entries to the HANDLERS map, append each method to the METHODS list in capabilities.py, and add CLI mirror functions for each method in cli.py. The test_capabilities test will validate that all eight methods are properly registered in all four locations.Source: Coding guidelines
src/vouch/volunteer_context.py (1)
229-245:
⚠️ Potential issue | 🔴 Critical🧩 Analysis chain
🌐 Web query:
MCP python server send_notification ServerNotification custom method💡 Result:
In the Model Context Protocol (MCP) Python SDK, the
send_notificationmethod is used to emit one-way messages from a server to a client that do not require a response [1][2][3]. The standard way to send notifications is through high-level helper methods provided by theContextobject orServerSession, such asctx.session.send_resource_list_changed()orctx.session.send_log_message()[4][5][2]. These helpers ensure that notifications conform to the protocol's expected types and schemas [2][3]. Regarding the use ofsend_notificationfor custom methods: 1. Protocol Validation: Thesend_notificationmethod in the base session relies on Pydantic models (specificallyServerNotificationorClientNotification) to validate outgoing messages [6][7][1]. These are defined as union types within the SDK [6]. 2. Limitations: The SDK enforces strict validation on notification methods and parameters [7]. Attempting to pass a customServerNotificationthat does not match the predefined types (e.g.,notifications/tools/list_changed,notifications/resources/updated) will typically result in validation errors [7]. 3. Recommended Approach: If you need to send information that does not fit into the standard notification types, it is generally recommended to use established channels such as logging messages (if for monitoring) or to extend the standard types if custom behavior is required [5][2]. Directly usingsend_notificationwith custom, non-standard types is not officially supported by the SDK's current architecture, as the underlying Pydantic models are restrictive [6][7]. If you encounter a need for a custom notification, it is often a sign that you should either use an existing, standardized notification type or that the desired communication pattern might be better suited for a request-response interaction [2].Citations:
- 1: https://github.com/modelcontextprotocol/python-sdk/blob/7ba41dcf/src/mcp/shared/session.py
- 2: https://deepwiki.com/modelcontextprotocol/python-sdk/5.4-progress-and-notifications
- 3: https://deepwiki.com/modelcontextprotocol/python-sdk/3.3-handling-server-notifications
- 4: https://github.com/modelcontextprotocol/python-sdk/blob/7ba41dcf/examples/snippets/servers/notifications.py
- 5: http://github.com/modelcontextprotocol/python-sdk?tab=readme-ov-file
- 6: modelcontextprotocol/python-sdk#1611
- 7: modelcontextprotocol/python-sdk#1399
Custom notification methods are not supported by the MCP SDK.
The MCP Python SDK enforces strict validation on
send_notification()using Pydantic models that restrict notification methods to predefined types (e.g.,notifications/tools/list_changed,notifications/resources/updated). Using a custom method name likekb.volunteer_contextwill fail validation at runtime and is not part of the official MCP API architecture. Consider using a standard notification type or an alternative communication pattern that fits within the protocol's constraints.🤖 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/volunteer_context.py` around lines 229 - 245, The _send() function is attempting to send a notification with a custom method name "kb.volunteer_context", but the MCP Python SDK only validates and supports predefined notification types through its Pydantic models. Replace the custom notification approach in the Notification object with either a standard predefined MCP notification type (such as notifications/tools/list_changed or notifications/resources/updated) that aligns with your use case, or implement an alternative communication pattern that conforms to the MCP protocol constraints rather than using custom method names.
2 participants
What changed
Adds
kb.volunteer_context— a confidence-gated push-context channel for active sessions.kb.session_start(task=…)registers hot memory, runs an immediate relevance pass, and starts a background watch. When an approved claim's normalized relevance exceeds the configured threshold (default0.85), vouch queues{claim_id, relevance, why}and emits an MCP notification. JSONL and CLI clients poll viakb.volunteer_context/vouch session volunteer. New modules:hot_memory.py,volunteer_context.py. Optionalvolunteer:config block inconfig.template.yaml.Why
Today vouch is pull-only: the agent asks, vouch answers. This adds a push channel so vouch can surface "you have an approved claim about this; consider citing it" mid-session when the task is highly relevant — matching the gbrain push-context model described in #236.
Fixes #236.
What might break
No breaking changes for existing
.vouch/directories.kb.session_start/kb.session_endbehaviour is unchanged from the caller's perspective (same return shapes); they now start/stop an in-memory background watch whentaskis set andvolunteer.enabledis true (default).kb.volunteer_contextadded to MCP, JSONL, and capabilities — additive only.kb_session_startis nowasync(FastMCP supports this; return shape unchanged).VEP
Not required — additive
kb.*method; no on-disk or object-model changes. Issue #236 scoped the feature.Tests
make checkpasses locally (lint + mypy + pytest)tests/test_volunteer_context.py)CHANGELOG.mdupdated under## [Unreleased]Summary by CodeRabbit
New Features
fsck)Bug Fixes
Documentation