Repository navigation
Confidence 8/10 · No issues found
🟢 Confidence 8/10 · likely safe to merge. Both refusals are pinned by new tests; only the observer case of the tools/list hiding in server.ts is untested.
1 note · tests partial −1 · risk high −1
Applies the observer role to the two surfaces that skip the HTTP middlewares: MCP refuses a mutating tool by role and hides it from tools/list, and a chat turn drops its approval-gated tools. Correct on both layers and safe to merge; the McpQueryError issues in registry.ts come from the query path this diff does not touch.
isReadOnlyTenantrefuses an observer every MCP tool that changes data- Observers are no longer shown write tools in
tools/list profileForRoleswithholds an observer chat turn's approval-gated tools
Findings
🔵 Note · F1 · The observer's tools/list hiding has no test
tests · apps/ai/src/mcp/server.ts:78
app.test.ts:738 pins per-credential visibility for a read-only key, and this line extends the same gate to a role, but nothing asserts an observer's tools/list omits create_*. A revert to tenant?.readOnly !== true would pass every test in the change.
Add an observer case beside "hides write tools from a read-only key" in `apps/ai/src/mcp/app.test.ts`.
Production impact
Open errors in the changed files (2)
| Issue | Service | Occurrences | File |
|---|---|---|---|
| @maple/mcp/errors/McpQueryError (3 issues) | maple-chat | 15 | apps/ai/src/mcp/tools/registry.ts |
| @maple/mcp/errors/McpQueryError (7 issues) | maple-ai | 30 | apps/ai/src/mcp/tools/registry.ts |
After this merges, Maple checks whether they stop.
Production traffic of the changed files (last 7 days)
| File | Calls/day | Busiest operations |
|---|---|---|
apps/ai/src/mcp/server.ts |
2.0k | tools/list 2.0k/day, 0.0% err |
What was checked
highestRole([])reads as member (policy/roles.ts:75), so internalroles: []tenants are not refused- Chat turns narrow on the acting tenant's roles, linked-user connector turns included (
turn-runner.ts:618) ask→denymaps in place, and last-match-wins keeps the mutating denies after the*allow (permission.ts:58)
7ea2550 · Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.
Annotations
Check notice on line 78 in apps/ai/src/mcp/server.ts
maple-review-bot / Maple / review
tests: The observer's `tools/list` hiding has no test
`app.test.ts:738` pins per-credential visibility for a read-only key, and this line extends the same gate to a role, but nothing asserts an observer's `tools/list` omits `create_*`. A revert to `tenant?.readOnly !== true` would pass every test in the change.