Test#525
Merged
Merged
Conversation
the starter config now ships review.auto_approve_on_receipt: true (and
require_human_approval: false, an advisory key no code path reads): a
fresh kb auto-approves captured claims whose byte-offset receipts
verify, so recall works out of the box with no review pass. pages,
entities, relations, and claims that cannot quote their source still
wait for a human — the gate is structurally unchanged.
making the default real exposed two holes, both fixed here:
- a claim re-deriving text that is already durable crashed the drain
("cannot approve: claim … already exists") and, on the capture-answer
path, was swallowed and left pending forever — every session that
restated a known fact grew the queue. resolve_pending_receipt_claim
now closes such duplicates mechanically (rejected, "duplicate:
identical claim already durable"); an id held by different text is a
real conflict and stays pending for a human. capture answer and the
drain share this resolution.
- nothing ever drained a backlog of verifiable claims filed while the
gate was off. finalize-all (the sessionstart hook) now runs the
drain, so flipping the flag on an existing kb heals it on the next
session start.
observed in the field: a fresh 1.4.0 install whose rollup and repeated
answers piled up pending despite the flag being on.
feat(review): receipt auto-approve on by default, duplicate-safe drain
…tension connects following the readme in a fresh project did not work in the claude code vs code extension: `install-mcp` wrote only a project-scope `.mcp.json`, which claude code loads only after a per-user approval — and the extension never surfaces that prompt. so the server sat at "pending approval", the kb_* tools never appeared, and the hooks (which need no approval) kept running, which reads as connected but isn't. reproduced end-to-end on a fresh project with released 1.4.0. install-mcp now also registers vouch at local scope in ~/.claude.json (`projects[<abs>].mcpServers`), exactly what `claude mcp add` does — a local-scope server is trusted on sight, no approval gate. declared by a new manifest `user_mcp:` block so the quirk lives in the claude-code adapter, not the core (host-neutral: only claude-code opts in). the merge is idempotent, preserves every other key in ~/.claude.json, and never clobbers a `vouch` server the user added themselves; a malformed config is reported as failed, never crashes the install. `--no-approve` opts out. the cli prints the registration and a reload-your-window nudge (mcp servers load once, at window start). verified end-to-end through the shipped cli: fresh project -> `vouch install-mcp claude-code` -> `claude mcp list` reports `vouch ... ✔ Connected` (was `⏸ Pending approval`), and an mcp handshake exposes all eight kb_* tools. tests sandbox ~/.claude.json via an autouse home fixture so the suite (and the many existing install() calls) never touch the developer's real config. readme + adapter readme now document the one-command flow and the reload; changelog updated.
feat(install-mcp): register a local-scope mcp server so the vscode extension connects
WalkthroughThis change adds Claude Code MCP registration through ChangesClaude MCP registration
Receipt-gated approval
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeCode
participant capture_answer
participant resolve_pending_receipt_claim
participant KBStore
ClaudeCode->>capture_answer: capture answer
capture_answer->>resolve_pending_receipt_claim: resolve receipt-backed claim
resolve_pending_receipt_claim->>KBStore: check durable claim
KBStore-->>resolve_pending_receipt_claim: claim identity result
resolve_pending_receipt_claim-->>capture_answer: approved, rejected duplicate, or pending
sequenceDiagram
participant User
participant install_mcp
participant install
participant claude_json
User->>install_mcp: run install-mcp
install_mcp->>install: pass approve option
install->>claude_json: register vouch MCP server
claude_json-->>install: registration result
install-->>install_mcp: report registered or failed
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/vouch/proposals.py (3)
504-510: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAvoid evaluating
Noneto"None".If
proposal.payload.get("id")explicitly returnsNone,str(None)will result in the string"None", which could theoretically match a claim actually named"None". It is safer to check for truthiness before casting to string.🛠️ Proposed refactor
- claim_id = str(proposal.payload.get("id", "")) + claim_id = proposal.payload.get("id") existing: Claim | None = None if claim_id: try: - existing = store.get_claim(claim_id) + existing = store.get_claim(str(claim_id)) except ArtifactNotFoundError: existing = None🤖 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/proposals.py` around lines 504 - 510, Update the claim ID initialization in the proposal lookup flow to check the payload’s raw id value for truthiness before converting it to a string, ensuring an explicit None produces an empty claim_id rather than "None". Preserve the existing get_claim and ArtifactNotFoundError handling for valid IDs.
481-484: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache
_review_configto avoid repeated disk I/O.
resolve_pending_receipt_claimis called inside a loop over all pending proposals inauto_approve_receipts, and inside a loop over extracted claims incapture_answer. Since_review_config(store)reads and parsesconfig.yamlfrom disk on every call, this introduces unnecessary I/O overhead. Consider acceptingreview_cfgas an optional parameter to reuse the configuration across loop iterations.⚡ Proposed refactor
def resolve_pending_receipt_claim( - store: KBStore, proposal: Proposal, *, actor: str, reason: str + store: KBStore, proposal: Proposal, *, actor: str, reason: str, + review_cfg: dict[str, Any] | None = None ) -> Claim | None: """Mechanically decide one pending CLAIM proposal, honouring the gate.Inside the function:
- review_cfg = _review_config(store) + review_cfg = review_cfg if review_cfg is not None else _review_config(store)And update the caller in
auto_approve_receipts:- if not _review_config(store).get("auto_approve_on_receipt"): + review_cfg = _review_config(store) + if not review_cfg.get("auto_approve_on_receipt"): return [] approved: list[Claim] = [] for proposal in store.list_proposals(ProposalStatus.PENDING): claim = resolve_pending_receipt_claim( store, proposal, actor=actor or proposal.proposed_by, reason="receipt verified — auto-approved", + review_cfg=review_cfg )🤖 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/proposals.py` around lines 481 - 484, Update resolve_pending_receipt_claim to accept an optional review_cfg parameter and reuse it when provided, only calling _review_config(store) when absent. In auto_approve_receipts and capture_answer, load the review configuration once per loop operation and pass it into each resolve_pending_receipt_claim call.
497-503: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winOptimize evaluation for trusted agents.
Currently,
_claim_receipts_verifyis evaluated even whentrustedis true. Since this verification reads from the disk and performs string matching, it should be skipped iftrustedis already true.⚡ Proposed refactor
trusted = review_cfg.get("approver_role") == "trusted-agent" - receipted = bool( - review_cfg.get("auto_approve_on_receipt") - ) and _claim_receipts_verify(store, proposal) - if not (trusted or receipted): - return None + if not trusted: + receipted = bool(review_cfg.get("auto_approve_on_receipt")) and _claim_receipts_verify(store, proposal) + if not receipted: + return None🤖 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/proposals.py` around lines 497 - 503, Update the approval condition in the proposal flow to short-circuit receipt verification when trusted is true: evaluate _claim_receipts_verify only if the approver is not trusted, while preserving the existing auto_approve_on_receipt requirement for non-trusted agents.
🤖 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 `@README.md`:
- Around line 151-152: Qualify the approval behavior in README.md lines 151-152
by stating that setting review.auto_approve_on_receipt to false disables
receipt-based auto-approval, while trusted-agent policy may still approve
eligible proposals. Update CHANGELOG.md lines 26-35 to replace any “fully human
gate” wording with “disable receipt-based auto-approval” or explicitly include
the trusted-agent exception.
- Around line 126-128: Update the README approval instructions to apply only to
manual wiring or installations using --no-approve. Clarify that the default
install-mcp flow already registers the trusted local-scope server, and remove
any wording implying install-mcp cannot perform that registration; retain
approval steps for users who skip automatic registration.
In `@src/vouch/install_adapter.py`:
- Around line 249-260: Update the `_UserMcp` manifest-loading validation to
require `scope` to be a string before checking it against the allowed values,
preserving `AdapterError` for invalid YAML types. Validate `spec` for JSON
serializability during loading and convert serialization failures into
`AdapterError`, so invalid values cannot reach registration; apply the same
validation to the additional user MCP parsing path.
- Around line 390-427: Protect the complete configuration read-modify-write flow
in the installer method containing the reg.scope and reg.name checks with an
interprocess lock held from reading cfg_path through tmp.replace(cfg_path). Use
a unique temporary file created in cfg_path.parent for each write, preserving
same-directory atomic replacement and cleaning it up appropriately, so
concurrent installers or Claude Code updates cannot be overwritten.
- Around line 415-419: Update the registration logic around the servers check in
the install-mcp flow to compare the existing mcpServers.vouch specification with
the expected server configuration. Return normally only for an identical
specification; record a differing existing specification as a conflict while
preserving the user’s entry and avoiding result.skipped, so the CLI does not
report it as successfully registered.
---
Nitpick comments:
In `@src/vouch/proposals.py`:
- Around line 504-510: Update the claim ID initialization in the proposal lookup
flow to check the payload’s raw id value for truthiness before converting it to
a string, ensuring an explicit None produces an empty claim_id rather than
"None". Preserve the existing get_claim and ArtifactNotFoundError handling for
valid IDs.
- Around line 481-484: Update resolve_pending_receipt_claim to accept an
optional review_cfg parameter and reuse it when provided, only calling
_review_config(store) when absent. In auto_approve_receipts and capture_answer,
load the review configuration once per loop operation and pass it into each
resolve_pending_receipt_claim call.
- Around line 497-503: Update the approval condition in the proposal flow to
short-circuit receipt verification when trusted is true: evaluate
_claim_receipts_verify only if the approver is not trusted, while preserving the
existing auto_approve_on_receipt requirement for non-trusted agents.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b402f358-a941-4dc4-9258-08ebd55384fb
📒 Files selected for processing (16)
CHANGELOG.mdREADME.mdadapters/claude-code/.claude/settings.jsonadapters/claude-code/README.mdadapters/claude-code/install.yamldocs/getting-started.mdsrc/vouch/capture.pysrc/vouch/cli.pysrc/vouch/install_adapter.pysrc/vouch/proposals.pysrc/vouch/storage.pytests/test_capture.pytests/test_capture_answer.pytests/test_extract.pytests/test_install_adapter.pytests/test_receipt_auto_approve.py
Comment on lines
+126
to
+128
| `install-mcp` initialises the KB when no `.vouch/` is discoverable (pass `--no-init` to skip; `vouch init` still exists for KB-only setup), then writes `.mcp.json` (the `kb.*` MCP tools), the `/vouch-*` slash commands, and five hooks — `SessionStart` recall, `UserPromptSubmit` per-prompt recall, `PostToolUse` capture, `Stop` answer capture, `SessionEnd` rollup. It also registers vouch as a local-scope MCP server in `~/.claude.json` (the `⚑` line in the output). **Reload your editor window** (VS Code: *Developer: Reload Window*) so it loads. | ||
|
|
||
| > **Why the extra registration?** A committed `.mcp.json` is a *project*-scope server, and Claude Code only loads one after a per-user approval — which the **VS Code extension never prompts for**, so `.mcp.json` alone leaves the `kb_*` tools invisible in the extension (they sit at "pending approval", while the hooks quietly work — easy to misread as "connected"). The local-scope entry `install-mcp` writes is trusted on sight, so a fresh install just connects. Verify with `claude mcp list` (`vouch … ✔ Connected`). Pass `--no-approve` to skip it and approve `.mcp.json` yourself. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
scope the following approval instructions to manual installs.
this paragraph says the default command registers a trusted local server, but the next “one-time approval” block still instructs every reader to approve .mcp.json and says install-mcp cannot perform the step. Mark that block as applying only to --no-approve or manual wiring.
🤖 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 126 - 128, Update the README approval instructions to
apply only to manual wiring or installations using --no-approve. Clarify that
the default install-mcp flow already registers the trusted local-scope server,
and remove any wording implying install-mcp cannot perform that registration;
retain approval steps for users who skip automatic registration.
Comment on lines
+151
to
+152
| Receipt-verified claims skip the queue by default (`review.auto_approve_on_receipt: true` in the starter config): each session's captured answers become recallable memory with no review pass. What lands in `vouch review` is everything the mechanical check can't vouch for — session-summary pages, entities, relations, and claims that can't quote their source. Set the flag to `false` in `.vouch/config.yaml` to put every write behind the gate. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
qualify the “fully human gate” claim.
disabling review.auto_approve_on_receipt does not guarantee that every write requires human approval when the opt-in trusted-agent shortcut remains enabled.
README.md#L151-L152: say the flag disables receipt-based approval, while trusted-agent policy may still approve eligible proposals.CHANGELOG.md#L26-L35: replace “fully human gate” with “disable receipt-based auto-approval,” or include the trusted-agent exception.
📍 Affects 2 files
README.md#L151-L152(this comment)CHANGELOG.md#L26-L35
🤖 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 151 - 152, Qualify the approval behavior in README.md
lines 151-152 by stating that setting review.auto_approve_on_receipt to false
disables receipt-based auto-approval, while trusted-agent policy may still
approve eligible proposals. Update CHANGELOG.md lines 26-35 to replace any
“fully human gate” wording with “disable receipt-based auto-approval” or
explicitly include the trusted-agent exception.
Comment on lines
+249
to
+260
| scope = raw.get("scope", "project") | ||
| if scope not in {"project", "user"}: | ||
| raise AdapterError( | ||
| f"{host}: install.yaml user_mcp `scope` must be 'project' or 'user', got {scope!r}" | ||
| ) | ||
| name = raw.get("name") | ||
| if not isinstance(name, str) or not name.strip(): | ||
| raise AdapterError(f"{host}: install.yaml user_mcp needs a non-empty `name`") | ||
| spec = raw.get("spec") | ||
| if not isinstance(spec, dict): | ||
| raise AdapterError(f"{host}: install.yaml user_mcp `spec` must be a mapping") | ||
| return _UserMcp(config=config, scope=scope, name=name, spec=spec) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
validate every yaml value before constructing _UserMcp.
a list or mapping supplied as scope raises TypeError at set membership instead of AdapterError. similarly, yaml permits values in spec that json.dumps() cannot serialize, and that TypeError is not caught during registration. validate scope as a string and verify that spec is json-serializable while loading the manifest.
as per coding guidelines, “install.yaml files for MCP host adapters must use strict YAML manifest validation.”
proposed fix
scope = raw.get("scope", "project")
- if scope not in {"project", "user"}:
+ if not isinstance(scope, str) or scope not in {"project", "user"}:
raise AdapterError(
f"{host}: install.yaml user_mcp `scope` must be 'project' or 'user', got {scope!r}"
)
...
if not isinstance(spec, dict):
raise AdapterError(f"{host}: install.yaml user_mcp `spec` must be a mapping")
+ try:
+ json.dumps(spec)
+ except (TypeError, ValueError) as e:
+ raise AdapterError(
+ f"{host}: install.yaml user_mcp `spec` must contain JSON-compatible values"
+ ) from eAlso applies to: 420-432
🤖 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 249 - 260, Update the `_UserMcp`
manifest-loading validation to require `scope` to be a string before checking it
against the allowed values, preserving `AdapterError` for invalid YAML types.
Validate `spec` for JSON serializability during loading and convert
serialization failures into `AdapterError`, so invalid values cannot reach
registration; apply the same validation to the additional user MCP parsing path.
Source: Coding guidelines
Comment on lines
+390
to
+427
| if cfg_path.exists(): | ||
| raw = cfg_path.read_text(encoding="utf-8") | ||
| data = json.loads(raw) if raw.strip() else {} | ||
| if not isinstance(data, dict): | ||
| result.failed.append(label) | ||
| return | ||
| else: | ||
| data = {} | ||
|
|
||
| if reg.scope == "project": | ||
| projects = data.setdefault("projects", {}) | ||
| if not isinstance(projects, dict): | ||
| result.failed.append(label) | ||
| return | ||
| container = projects.setdefault(str(target), {}) | ||
| else: | ||
| container = data | ||
| if not isinstance(container, dict): | ||
| result.failed.append(label) | ||
| return | ||
| servers = container.setdefault("mcpServers", {}) | ||
| if not isinstance(servers, dict): | ||
| result.failed.append(label) | ||
| return | ||
|
|
||
| if reg.name in servers: | ||
| # already registered — never clobber the user's own edits, and stay | ||
| # out of `result.skipped` (a file-path channel; this is a config | ||
| # entry, not a file). The CLI reminds the user to reload regardless. | ||
| return | ||
| servers[reg.name] = dict(reg.spec) | ||
|
|
||
| cfg_path.parent.mkdir(parents=True, exist_ok=True) | ||
| tmp = cfg_path.with_name(cfg_path.name + ".vouch-tmp") | ||
| tmp.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") | ||
| if cfg_path.exists(): | ||
| shutil.copymode(cfg_path, tmp) | ||
| tmp.replace(cfg_path) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
protect the read-modify-write from concurrent updates.
the atomic replacement prevents partial files, but it does not prevent lost updates: claude code or another installer can modify .claude.json after line 391, and line 427 then replaces those changes with the stale snapshot. lock the configuration across read, merge, and replace, and use a unique temporary file in the same directory.
🧰 Tools
🪛 ast-grep (0.44.1)
[info] 423-423: use jsonify instead of json.dumps for JSON output
Context: json.dumps(data, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🤖 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 390 - 427, Protect the complete
configuration read-modify-write flow in the installer method containing the
reg.scope and reg.name checks with an interprocess lock held from reading
cfg_path through tmp.replace(cfg_path). Use a unique temporary file created in
cfg_path.parent for each write, preserving same-directory atomic replacement and
cleaning it up appropriately, so concurrent installers or Claude Code updates
cannot be overwritten.
Comment on lines
+415
to
+419
| if reg.name in servers: | ||
| # already registered — never clobber the user's own edits, and stay | ||
| # out of `result.skipped` (a file-path channel; this is a config | ||
| # entry, not a file). The CLI reminds the user to reload regardless. | ||
| return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
report a conflicting existing server instead of treating it as registered.
any existing mcpServers.vouch entry is silently accepted, even when it points to another command. consequently, install-mcp prints “vouch is registered” although the expected server was not installed. treat an identical specification as idempotent, but record a differing specification as a conflict without overwriting it.
proposed fix
if reg.name in servers:
- # already registered — never clobber the user's own edits, and stay
- # out of `result.skipped` (a file-path channel; this is a config
- # entry, not a file). The CLI reminds the user to reload regardless.
+ if servers[reg.name] != reg.spec:
+ result.failed.append(label)
return📝 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 reg.name in servers: | |
| # already registered — never clobber the user's own edits, and stay | |
| # out of `result.skipped` (a file-path channel; this is a config | |
| # entry, not a file). The CLI reminds the user to reload regardless. | |
| return | |
| if reg.name in servers: | |
| if servers[reg.name] != reg.spec: | |
| result.failed.append(label) | |
| return |
🤖 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 415 - 419, Update the registration
logic around the servers check in the install-mcp flow to compare the existing
mcpServers.vouch specification with the expected server configuration. Return
normally only for an identical specification; record a differing existing
specification as a conflict while preserving the user’s entry and avoiding
result.skipped, so the CLI does not report it as successfully registered.
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
1 participant
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
vouch install-mcp claude-codenow registers the MCP server locally, with options to approve or skip registration.