Address review feedback on the vault MCP tools

Two non-blocking items from the review pass:

Document why get_statement_coverage reads through accessible_by rather than
writable_by. It reports which documents exist and writes nothing, so read
access is the right bar — and tightening it would hide coverage gaps from
people who can already see the figures those gaps sit behind. The comment
exists so a future refactor doesn't "fix" it.

Close the acknowledged verification gap with tests rather than a one-off
manual check. The review noted that nothing proved a real vault payload
serializes cleanly out through tools/call — vault responses are richer than
the other tools' output, with nested account hashes, decimal balances, dates
and a compacted hash. Two integration tests now drive the real /mcp endpoint
end to end against a real AccountStatement: one listing it, one uploading
bytes and reading back the SHA-256. Permanent regression coverage instead of
a smoke test someone has to remember to repeat.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn
This commit is contained in:
Claude
2026-07-30 01:20:06 +00:00
committed by Juan José Mata
parent ec38a89d0c
commit 5c2c38e37e
2 changed files with 60 additions and 0 deletions

View File

@@ -62,6 +62,11 @@ class Assistant::Function::GetStatementCoverage < Assistant::Function
account_id = params["account_id"].to_s
return error("invalid_account_id", "account_id must be a UUID.") unless valid_uuid?(account_id)
# accessible_by, not writable_by, is deliberate: this reports which documents
# exist for an account and writes nothing. Someone with read access to an
# account is entitled to know which of its statements are on file. Do not
# "tighten" this to writable_by — that would hide gaps from the people who
# can see the figures those gaps sit behind.
account = family.accounts.accessible_by(user).find_by(id: account_id)
return error("account_not_found", "No accessible account found with that ID.") unless account