[Security 4/9] MCP tools require confirmation unless positively known-safe

From the PR description

[Security 4/9] MCP tool calls pause for the user's approval unless positively trusted

Part of the split of #227 into single-topic PRs. Index: tracking comment on #227.

Rebased onto main 6a62d01a (2026-08-26); branch tip 3577a54d. Clean rebase - no textual conflicts - but three things merged into main in the meantime interact with this branch on purpose rather than by accident, and each got a deliberate reconciliation. See "Reconciliation with what merged into main" below.

This description was rewritten once before to match the branch (it originally described only the first commit, a one-function annotation flip). See "What changed since the first version of this description" at the bottom for the full commit ledger.

Housekeeping: between 2026-08-25 05:37 and 05:58 UTC this PR's body was accidentally overwritten with #351's description by a scripted PATCH. The text below is the real #247 description, recovered from GitHub's edit history and brought up to date. #351 is unaffected.

TL;DR

On main, an MCP connector tool runs without asking anyone unless the external server volunteers destructiveHint: true or readOnlyHint: false. A tool that says nothing about itself - the common case - executes silently. And the handful of tools that do get flagged are not gated, they are bricked: refresh force-disables them, the enable toggle throws, and tool discovery filters them out. There is no confirmation UI anywhere in the product.

This PR replaces both halves:

1. The policy flips to fail-safe. A tool may auto-run only if its annotations positively declare it safe and the user has separately marked that connector as trusted. Two independent signals, because the server writes its own annotations. Since the trust flag defaults to off, the shipped default is every MCP tool call asks - the annotation classification only starts mattering once a user opts a connector in.
2. "Requires confirmation" becomes a real thing a user can do. The exact proposed call (tool + arguments) is written to a short-lived, single-use ledger row, streamed into the chat as an inline Approve / Decline prompt, and executed - if approved - from the stored payload.

Risk to user data

Severity: medium-high, and this is a key defense-in-depth layer behind prompt injection (see [Security 5/9], spotlighting). Tool annotations (readOnlyHint / destructiveHint / openWorldHint) are advisory and controlled by the external MCP server - a hint, not a guarantee. If the LLM is tricked (or a connector is malicious or mis-annotated) into calling a side-effecting tool, the cost in a legal product is severe: exfiltrating a privileged document, mutating a matter, hitting an unknown external system. Requiring positive proof-of-safety means a bad or missing annotation fails toward a confirmation click, not silent execution.

This is the "human in the loop for consequential actions" mitigation from the OWASP Top 10 for LLM Applications (excessive agency / insecure output handling). The general lesson - never trust a security decision to a field the other side controls - is the same failure behind confused-deputy bugs.

Replicate the base case (on main)

The point of these steps is to see the two distinct failures on main: an unannotated tool runs unasked, and an annotated-destructive tool is permanently unusable.

Setup (both cases): run the stack, sign in, go to Settings → Connectors, add any MCP server you control, and hit Refresh tools.

Case A - silent execution of an unannotated tool

1. Point the connector at an MCP server whose tool declares no annotations (annotations: {} - the default for most real servers).
2. git show origin/main:backend/src/lib/mcp/client.ts → toolRequiresConfirmation() (~line 174) returns truthyAnnotation(annotations, "destructiveHint") || annotations?.readOnlyHint === false. With {} both terms are false, so the tool is cached with requires_confirmation = false.
3. In a chat, ask the model to use that tool. Observe: it just runs. In the SSE stream (DevTools → Network → the /chat request → Response) you get an mcp_tool_call event with status: "ok" and never anything resembling a confirmation. Confirm in the DB: select tool_name, requires_confirmation from user_mcp_connector_tools; → false.

Case B - a flagged tool is bricked, not gated

1. Point the connector at a tool declaring destructiveHint: true (or readOnlyHint: false) and Refresh tools.
2. Watch it flip itself off in the tools list: refreshUserMcpConnectorTools in backend/src/lib/mcp/servers.ts (~line 379) runs .update({ enabled: false }).eq("requires_confirmation", true) after every sync.
3. Try to switch it back on. The request fails with "This MCP tool needs human confirmation before Mike can expose it to chat." (setUserMcpToolEnabled, ~line 428).
4. Ask the model to call it anyway. buildUserMcpTools selects with .eq("requires_confirmation", false), so the model was never told the tool exists; and if you invoke the name directly, resolveCallableTool (same .eq(...) filter) returns null and the model gets "MCP tool is not available or is disabled."
5. Grep the frontend for any approval surface - git grep -l "mcp_confirmation_required\|mcp-pending-calls" origin/main → no matches. There is nothing to click.

That combination is the actual problem: the only thing the old policy could do to a suspicious tool was disable it forever, which is why the default had to be lenient.

Replicate this PR

Setup: check out the branch, apply backend/migrations/20260802_01_mcp_pending_tool_calls.sql (or bootstrap from backend/schema.sql, which now carries the table), start the stack, add an MCP connector, Refresh tools.

1 - The gated tool is now usable, and says so

1. Settings → Connectors → your connector. The unannotated tool is enabled and toggleable
   • the toggle is live where on main it was permanently disabled, and refresh no longer silently flips it off. Its amber badge now reads "Asks for your approval before each run" (was "Confirmation required").
2. New control in the connector details modal: "Trust this server's safety annotations", default off, with the copy "Off (recommended): every tool call asks for your approval." Leave it off.

2 - The approval prompt (the happy path)

1. In a chat, ask the model to call that tool.
2. The assistant message renders an inline block: <connector>: <tool> wants to run, followed by a <pre> showing the exact arguments JSON that will execute, plus Approve and Decline buttons.
3. Proof in DevTools → Network → the streaming /chat request → Response tab: an SSE frame data: {"type":"mcp_confirmation_required","id":"<uuid>","tool_name":..., "arguments_json":...,"expires_at":...} (emitted from backend/src/lib/chat/tools/toolDispatcher.ts).
4. Proof in the DB, while the prompt is on screen: select id, status, tool_name, arguments, expires_at from user_mcp_pending_tool_calls; → one row, status = 'pending', expires_at ≈ now + 2 min.
5. Click Approve. Network shows POST /user/mcp-pending-calls/<id> with body {"decision":"approve"} → 200 {"status":"approved"}. Note the request body carries only the verb - the payload is not resendable. The button reads Approving... and stays there until the backend notices: resolution is driven by the server's next poll (≤1.5 s), not by the POST response.
6. The block turns green and reads "Approved - running"; a second SSE frame {"type":"mcp_confirmation_resolved","id":...,"decision":"approved"} arrives, then the normal mcp_tool_call result. Re-query the row: it walked pending → approved → executing → executed and executed_at is set.

3 - Deny actually blocks execution

1. Ask for the tool again; this time click Decline.
2. POST /user/mcp-pending-calls/<id> with {"decision":"deny"} → {"status":"denied"}.
3. The block turns red and reads "Declined". The model receives the tool result {"ok":false,"error":"The user declined this tool call."} and the MCP server is never contacted - confirm at the server's own log, or by pointing the connector at a URL that would fail loudly if reached.
4. DB row is terminal denied; there is no executing row and no executed_at.

4 - Timeout, replay, and ownership (the security properties)

• Timeout: trigger a prompt and walk away. After MCP_APPROVAL_WAIT_MS (90 s) the turn stops waiting, the row is conditionally moved to expired, the block reads "Expired without a decision", and the model gets "The user did not approve this tool call in time." A late click then returns 410 Gone - {"detail":"This tool call approval has expired."}.
• Replay: approve a call, then re-issue the same POST with the same id → 404, because the conditional UPDATE requires status='pending'. An approval spends itself.
• Ownership: copy a pending id and POST it from a second signed-in account → 404, never a decision. The UPDATE is keyed on id + user_id + status + not expired.
• Stale-cache bypass (the fix in 6380de05): hand-set a row to requires_confirmation = false with annotations = '{}' (exactly what a pre-upgrade cache looks like) and turn the trust toggle on. The call still pauses, because toolRowRequiresConfirmation recomputes from the annotations at call time rather than reading the column.

5 - What "trusted" buys you

Turn "Trust this server's safety annotations" on for a tool whose server declares {readOnlyHint: true, openWorldHint: false}. That call now runs with no prompt. Turn the toggle off again → prompted. Neither signal alone is enough.

6 - The pause does not silently die behind a proxy (de8b56bf, 26c200f3)

While a prompt is open the turn writes no SSE data for up to 90 s. Trigger a prompt and watch the /chat response in DevTools → Network → Response: every 15 s a line : mcp-approval appears. It is an SSE comment, so no reader in the repo surfaces it and nothing lands in the transcript - but it is bytes on the wire, which is what keeps an intermediary from reaping the response as idle. curl -N against the same endpoint shows the same frames.

7 - Declining does not brick the rest of the chat (a1d1ad9d)

1. Decline a call, then send any follow-up message in the same chat.
2. On the pre-fix branch both sends failed with the generic stream error, permanently: the denied turn's narration lived only in its events, so the client replayed it as content: "", and Anthropic rejects any request containing an empty assistant message.
3. Now the follow-up streams normally. Confirm the mechanism in the request payload (Network → the /chat request → Payload): the denied turn is either carried with its text recovered from its content events, or dropped - never sent as "".

Reconciliation with what merged into main

The rebase itself was clean. These are the places where "no conflict" would have been the wrong answer, and what was decided instead.

#366 (Word add-in client tool loop) - shared the keep-alive instead of mirroring it

de8b56bf was written while #366 was still an open PR and deliberately mirrored its keep-alive pattern. Now that #366 is on main, mirroring would leave the tree with two independent answers to one question, and main has no shared helper to reuse - its cadence is a module-private KEEP_ALIVE_INTERVAL_MS in wordClientTools.ts with the frame text inline.

26c200f3 extracts backend/src/lib/sse.ts (SSE_KEEP_ALIVE_INTERVAL_MS, sseKeepAliveFrame(reason)) and points both sites at it: the Word tool loop's pane round-trip and this branch's approval wait. It sits at the lib root because both layers consume it and neither should depend on the other.

The cadence is only correct relative to proxy idle timeouts, so it has to move as a unit - two copies means someone lowering one for a 20s-timeout deployment fixes half the product and leaves the other half failing in the hardest way to diagnose. The frame's comment-ness carries the same hazard: a "cleanup" that turns one into a data frame pushes an unknown event type into a user's transcript, and the untouched copy gives no hint the rule existed.

main's design wins on structure. Each site keeps its own ticker, because the ticker's lifetime is site-specific and load-bearing in both (wordClientTools clears it in the finally around one forwarded call; executeMcpToolCall clears it around the approval wait so a throwing status poll cannot leave a timer writing into a dead stream). What is shared is the contract, not the control flow. The MCP module also stays transport-agnostic: it still takes onApprovalWaitKeepAlive as a callback, and the bytes are produced at the dispatcher, which is the layer that owns write.

#379 (loud empty model completions) - complementary, and now pinned

main now emits a visible "The model returned an empty response. Try again, or pick a different model." when a turn ends with no text and no error event. That lands directly on this branch's newest surface: declining a tool call legitimately produces a turn with no model prose. If the guard fired on it, every denial would tell the user the model broke and invite them to retry or switch models - the one control whose purpose is to make refusal safe would look like the thing that breaks the product, and the advice it gives is the opposite of what a security decision should teach.

The two mechanisms are complementary, for different reasons, and neither subsumes the other:

• #379 is about the turn just produced: empty output with nothing to show for it means something went wrong upstream, so say so.
• a1d1ad9d's filters (withoutEmptyAssistantReservations + runLLMStream) are about history: an assistant turn whose narration lives only in events must not be replayed to a provider as content: "". #379 cannot stop an already-persisted turn from being replayed; the filters cannot tell a user that this turn came back empty.

Decision: keep both, unchanged, and pin the seam. What actually keeps #379 off a denial is implicit - the guard skips any turn where some event has an error key, and the denial path returns an mcp_tool_call event carrying error: "The user declined this tool call." while the success path returns the same event type without it. That coupling lives in a file neither PR touches and no test named. 3577a54d adds a route-level test asserting a denied-only turn does not emit the empty-response error; it was verified to fail (and to produce the misleading message) when the error field is removed from the event, so it guards the real dependency rather than the current output.

#382 (HttpOnly cookie auth) - no change needed, verified

respondMcpPendingCall goes through mikeApi's apiRequest, which #382 rewrote to use authenticatedFetch against the same-origin /api gateway. The approval POST therefore inherits the cookie-based auth pattern with no branch-side change; the endpoint wrapper tests and the 100% src/app/lib/** coverage floor both still pass against the rewritten module.

Also on this pass

7be91042 - the migration was missing the REVOKE ALL ... FROM anon, authenticated that schema.sql already had and that every sibling MCP table got in 20260613_04_user_mcp_connectors.sql. Fresh installs were correct; upgrades were not, which is both a hardening gap and a drift-check divergence. RLS-with-no-policies already denied the rows, so this is defence in depth - but on the wrong table to rely on one control: arguments holds the verbatim proposed payload and status is the authorization decision. The filename is untouched (20260802_01 is early-sorted on purpose and already verified to land after user_mcp_connectors); REVOKE is idempotent, so the file stays re-runnable.

Tradeoffs & design decisions (explicit)

1. The gating policy

Read the implemented predicate honestly:

mcpCallNeedsApproval = requiresConfirmation || !connectorTrustsAnnotations(toolPolicy)

Because trust_annotations defaults to absent, the shipped default is "confirm everything, always" - the friction concern above is real, and the escape hatch is the toggle, not a lenient default. That is a deliberate choice for a legal product: the safe state is the one you get by doing nothing.

The annotation half is deliberately strict, per the MCP spec's defaults:

readOnlyHint === true  &&  openWorldHint === false  &&  destructiveHint !== true

openWorldHint omitted defaults to true in the spec, so {readOnlyHint: true} alone is an open-world reader and stays gated. Absence is never safety.

⚠️ Correction to the earlier version of this description, which advertised readOnlyHint===true && !destructive && !openWorld (i.e. absence of openWorldHint counted as closed-world). That was a misread of the spec; a2ad4f88 reversed it, and reversed the test that had asserted the old behavior.

2. Annotations are never sufficient on their own

A malicious server can simply write readOnlyHint: true. So the second signal is locally controlled: trust_annotations lives in the connector's tool_policy, defaults to off, and is revocable from the UI. Rejected alternative: a global "trust all annotations" preference - one wrong click would disarm every connector at once.

3. Approval is bound to the payload, not to the moment

The decision endpoint carries only {"decision":"approve"|"deny"}. It cannot alter the tool or the arguments, and execution reads the arguments back out of the stored row. The obvious alternative - echo the payload back with the approval - would let a compromised page approve one thing and run another. Cost: an extra table.

4. Poll, don't push

waitForMcpApprovalDecision polls the row every 1.5 s inside the already-open streaming turn (90 s bound, deliberately under the row's 120 s TTL). Rejected: a websocket/pubsub channel - new infrastructure for a flow that already has an open SSE stream and a two-minute horizon. Cost: up to 1.5 s of latency after a click.

Corrected: an earlier version of this note (and the comment in approvals.ts) claimed the 90 s bound also sat "under the global stream watchdog". There is no such watchdog, and there was no keep-alive either - the claim was aspirational and it was hiding the real failure mode that reviewer item 2 used to flag. de8b56bf makes the comment true instead of deleting it; see the next entry.

4b. The silent pause is covered by SSE comment frames, not a shorter timeout (de8b56bf)

The approval pause is the one point in a turn where the server deliberately goes quiet, and that is exactly what a proxy, load balancer, or CDN reaps for inactivity (nginx's proxy_read_timeout and an ELB idle timeout both default to 60 s; 30 s is a common hardened setting). Reaping it would kill the confirmation the user is still looking at, and it fails in the shape that reads as a product bug: "I clicked Approve and nothing happened."

Chosen: an SSE comment frame every 15 s for exactly the duration of the wait. Comment frames are free - every reader in this repo keeps only lines starting with data: - and the bytes do a second job: on a half-open TCP peer a periodic write is what surfaces the reset and fires the abort path instead of parking the turn on a socket that will never answer. Rejected: shortening the 90 s wait to fit the tightest plausible timeout, which would trade a rare infrastructure failure for a common "you took too long" failure.

The ticker is injected as a callback rather than a writer, so the MCP module stays transport-agnostic and a non-streaming caller simply omits it, and it is wrapped in try/finally so a throwing poll cannot leak a timer into a stream whose turn has ended. Cadence and frame shape are shared with #366's client tool loop via lib/sse.ts - see the reconciliation section.

Still not addressed: a confirmation does not survive a page reload, because the pending state lives only in the open stream. That needs event persistence and is tracked separately (reviewer item 3).

5. Claim-then-record, not record-then-run (be520b9b)

Two jobs were originally squeezed into one status write. Single-use safety must happen before execution; truth about the outcome can only be known after. So the state machine has an intermediate claim:

pending → approved → executing → executed   (call completed)
pending → approved → executing → failed     (call errored)
pending → denied
pending → expired

executing is honest at every instant ("an attempt is in flight"). failed is terminal, so a failed attempt still spends the approval - an error can never be retried into a replay.

6. Close the check-then-act race at the database (66ed8e31)

If the user's click commits between the last poll and the expiry UPDATE, the row would sit approved forever while the chat reported "expired". The fix makes the act conditional (UPDATE ... WHERE status='pending' RETURNING id) and uses the affected-row count as the race detector: 0 rows ⇒ a decision won ⇒ re-read once and honor it.

7. Opportunistic sweep instead of a cron job (ad17dfcb)

Terminal rows retain the full argument payload - potentially privileged matter data - so they get a bounded 24 h life. Cleanup piggybacks on the next INSERT (the same shape the repo already uses for the OAuth state store) rather than standing up job infrastructure for a low-traffic table. Best-effort: a sweep failure is logged and swallowed, because a user is waiting on the prompt that insert serves. Deliberately global, not per-user, so retention still holds for users who never return.

8. requires_confirmation demoted to a display cache (6380de05)

A security gate that reads a cached verdict is only as strong as its oldest entry. Rows written under main's lenient policy would keep false. The column is still written (it drives display/API summaries) but the authoritative decision recomputes from the stored annotations at call time, and an explicit stored true is still honored - the gate can only ever be strictly tighter than either signal alone.

9. Gated tools are now visible to the model - a deliberate widening

⚠️ Reviewers should decide on this one explicitly. On main, a gated tool was filtered out of buildUserMcpTools and resolveCallableTool entirely: the model never learned it existed. This PR removes both .eq("requires_confirmation", false) filters, so gated tools are advertised and reachable - the runtime prompt is what stops them, not invisibility.

That is strictly more surface exposed to a prompt-injected model. It is the necessary price of having an approval flow at all (you cannot approve a call that was never proposed), and the mitigation is that the human is now the gate rather than a cache column. But it is a loosening as well as a tightening, and it belongs in the reviewer's ledger.

Gated tools carry an appended note in their description: "This tool pauses for the user's explicit approval before it executes; the user may decline." Hiding the gate would make the model treat a decline as an unexplained failure and retry.

10. Sequential dispatch: one pending approval stalls the rest of the turn

runToolCalls dispatches tool calls in a sequential for loop, so a pending approval blocks every later tool call in the same turn for up to 90 s. Parallelising the loop was not attempted here - it is a larger change to an existing code path, and a turn that is waiting on a human is arguably supposed to stop.

Testing evidence

Local gates re-run on the rebased tip 3577a54d (on main 6a62d01a), all green:

CI's remaining checks (Supabase stack, fresh-vs-upgraded drift, playwright, CodeQL, eval harness, gitleaks, CLA) were green on the pre-rebase tip and are not re-runnable locally; they need a CI pass on 3577a54d. The drift check is the one to watch, since 7be91042 changes the migration - it should now converge more closely, because the missing REVOKE was itself a fresh-vs-upgraded divergence.

The 97.61% → 97.69% frontend branch figure is not a change from this PR: #382's rewrite of mikeApi.ts moved it. The comment in frontend/vitest.config.mts is refreshed to the new measurement; the floors are unchanged.

New tests added by this PR (exact counts):

• backend/src/lib/mcp/__tests__/confirmation.test.ts - 20 cases: every annotation combination including the missing-hint case, the strict-type edges ("true" is not true), the reversed omitted-openWorldHint expectation, and the both-signals approval matrix.
• backend/src/lib/mcp/__tests__/approvals.test.ts - 12 cases: ownership binding, decision single-use, expiry, exactly-one-winner execution claim, timeout retirement, the poll/expiry race, the executing → executed vs executing → failed split, and the retention sweep (aged terminal rows deleted; a recent denied and an old-but-live pending survive).
• backend/src/lib/mcp/__tests__/servers.approvalGate.test.ts - 7 cases: end-to-end executeMcpToolCall against an in-memory Supabase stand-in, driven with the exact regression row (requires_confirmation=false, annotations={}, trusted connector). The connector URL is a private IP on purpose - if the gate ever leaks, the SSRF guard fires and the test fails loudly rather than silently passing. de8b56bf adds the keep-alive contract on fake timers: frames appear on cadence while the row is pending, stop the moment it is decided, and the ticker is cleared even when the wait itself throws. Both were verified to fail against the unfixed code.
• backend/src/lib/sse.test.ts - 3 cases (26c200f3): the frame is a comment, is blank-line terminated, contains no data:, and two ticks fit inside the tightest 30 s idle window. These invariants were previously asserted only against the MCP copy; #366's Word tool loop is now covered by them too.
• backend/src/lib/chat/__tests__/routeStreaming.test.ts - 3 cases (a1d1ad9d): null reservations, empty/whitespace assistant content, and the negative case (real content and all user messages survive).
• backend/src/__tests__/integration/chat.routes.test.ts - 1 case (3577a54d): a denied MCP turn is not reported as an empty upstream completion. See the #379 reconciliation above for why this one is load-bearing rather than decorative.
• frontend/src/app/lib/mikeApi.test.ts - 2 new rows in the table-driven endpoint-wrapper suite, pinning respondMcpPendingCall for both approve and deny. Both arms deliberately: a wrapper that ignored its argument and always sent "approve" would pass a single-case test while converting every deny into an approval.

Coverage ratchet: the new export dropped src/app/lib/** below its 100% functions/lines floor and turned the frontend job red; 5967d185 fixes it with tests, then raises the statements floor 99 → 100 per the "floors only go up" policy in docs/frontend-testing.md. Re-measured on the rebased tip: 741/741 statements, 509/521 branches, 235/235 functions, 629/629 lines.

Verified live: the approval, denial, expiry, replay, and ownership paths above were exercised against a local stack during development. a1d1ad9d in particular was live-replicated both ways - the previously bricked chat streams again, and a fresh deny → follow-up → approve cycle completes end to end. Not yet verified against a third-party production MCP server - see below.

What changed since the first version of this description

The original body described commit c5b0d048 only (one predicate in client.ts + a 9-case test) and stated the openWorldHint rule incorrectly. Everything below was missing from it:

SHAs above are post-rebase. The pre-rebase equivalents were c5b0d048, a2ad4f88, 6380de05, 66ed8e31, be520b9b, ad17dfcb, ef885201, 6254f98f, 4fbb5d8b, b101e57a; older comments on this PR reference those.

For a human reviewer to check

1. A real third-party MCP server. Everything above is proven against unit fakes and a local stack. One pass against a genuine connector (Drive/Slack/GitHub) would confirm the annotations real servers actually emit land on the side of the policy we expect - in particular how many useful tools declare openWorldHint: false.

2. Does a 90 s silent SSE gap survive your real infrastructure? Addressed by de8b56bf. The finding was right and the code comment was wrong: there is no global stream watchdog anywhere in backend/src, and there was no keep-alive either. The pause now emits an SSE comment frame every 15 s for exactly its duration, sharing #366's cadence via lib/sse.ts (26c200f3). Still worth a reviewer's eye: confirm 15 s clears your deployment's tightest idle timeout - the constant is chosen against nginx/ELB defaults (60 s) and a common hardened setting (30 s), not against measurements of this org's infrastructure.

3. Does the prompt survive a page reload? mcp_confirmation events are not among the McpToolEvents persisted by streaming.ts, so reloading mid-prompt probably loses the Approve/Decline block from chat history while the row is still pending. Worth eyeballing in the browser - the row expires safely either way, but the UX is a dead end.

4. Should approving require MFA step-up? The Settings trust toggle goes through runSensitiveAction and its PATCH carries requireMfaIfEnrolled; the new POST /user/mcp-pending-calls/:id carries only requireAuth. Defensible (it is a click inside an authenticated live chat, and a step-up modal over a streaming turn is nasty), but it is a product call, not an oversight to be assumed.

5. Polling load. One row polled every 1.5 s per pending call per turn, for up to 90 s, against Supabase. Unmeasured under concurrency.

6. Migration date ordering. 20260802_01_... sorts before main's 20260821-20260825 migrations. Harmless here (an independent CREATE TABLE IF NOT EXISTS, and the drift check is green), but if the convention is that a merged migration is always newer than everything already deployed, the file should be re-dated before merge. The grants half of this item is now fixed - 7be91042 adds the REVOKE ALL ... FROM anon, authenticated the file was missing - but the RLS claim is still asserted in SQL text only: nothing here proves at runtime that anon/authenticated cannot read the ledger. A stack-level test against a real Postgres would.

7. UX of the argument dump. The prompt shows raw arguments JSON in a max-h-40 scroll box, with no countdown even though expires_at is carried in the event. For a large payload that is honest but not friendly; worth a product opinion on truncation, a summary line, and whether a visible timer would help.

8. The #379 exemption is implicit, and the test is the only thing naming it. 3577a54d pins that a denied turn does not trigger main's empty-completion error, but the mechanism is that the denial's mcp_tool_call event happens to carry an error key while the success event does not. A cleaner contract - an explicit "this turn intentionally has no prose" signal, or #379 keying off event type rather than key presence - would be more honest than a test guarding a coincidence. Worth a maintainer's opinion on whether to do that here or as a follow-up on main.

Reading

OWASP Top 10 for LLM Applications · Confused deputy problem · MCP tool annotations