cloudflare / cloudflare/cloudflare-os

A pending MCP approval cannot be approved or denied once its account's connection breaks, permanently locking the chat

Open
#100 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
9.9k
Forks
1.2k
Avg merge
1d 20h
Merged PRs (30d)
107

Description

## Summary

When an MCP tool call is queued for approval and the bound account's connection subsequently stops working, the pending action becomes unresolvable. **Approve** fails with "Failed to approve action" and **Deny** fails with "Failed to deny action". The composer stays disabled with "Approve or reject the pending action above to continue", so the conversation cannot be continued, abandoned, or cleared. The chat is permanently unusable.

Approve failing is understandable: it must reach the server. Deny failing is not — rejection is designed to be a purely local state change, and the UI itself reports that nothing has been sent yet.

This is the same end state as #97 ("Image attachment sent to a non-vision model permanently locks the chat") but a different trigger, so PR #98 would not address it.

## Evidence

Rejecting a staged action is local-only by construction. `packages/mcp-shared/src/action-store.ts` (around lines 201-209) implements `ActionStore.reject()` as a single SQL update against the gatekeeper's own storage, with no network call and no use of the MCP connection:

```ts
reject(id: number): void {
const stored = ...;
if (!stored || stored.state === "rejected") return;
...
this.#sql.exec("UPDATE mcp_actions SET state = 'rejected' WHERE id = ?", id);
}
```

`state` is constrained to `('pending', 'applying', 'applied', 'rejected', 'failed')` (same file, around line 58), and `packages/mcp-shared/src/facet.ts` (around line 169) exposes `rejectAction(action: number): Promise` over that primitive.

So the deny path needs nothing from the remote server. The failure appears to be upstream of it: reaching `rejectAction` requires resolving a gatekeeper facet for an account whose connection is broken, and that resolution fails before the local update is ever attempted. I have confirmed the reject primitive is local; I have **not** traced the exact client-to-facet call that errors, so the precise failure point is inferred rather than verified.

The queued call was classified as an action rather than an observation because the server does not declare `readOnlyHint`, which is expected behaviour per `classifyTool` in `packages/mcp-shared/src/tools.ts`. The UI stated "Treated as an action because the server did not declare it read-only. Nothing has been sent yet."

## Steps to reproduce

1. Connect an MCP server through the MCP gatekeeper (the user-supplied `byo` tier).
2. In a chat, invoke a tool from that server which is not declared `readOnlyHint`. It is queued and the chat shows Approve / Deny with the composer disabled.
3. Break the bound account's connection before deciding. In our case the account's OAuth grant could no longer be exchanged, and the gatekeeper logged:
```
component: gatekeeper.mcp
event: connect.oauth.failed
message: oauth code exchange failed
error: Error: The provided authorization grant is invalid
at async handleOAuthCallback (mcp.js:15310:22)
```
Disconnecting and re-adding the account produces the same end state.
4. Return to the chat and press **Approve**. It fails with "Failed to approve action".
5. Press **Deny**. It fails with "Failed to deny action".
6. *Bug*: the pending action cannot be cleared by any means available in the UI, the composer remains disabled, and the conversation is permanently unusable. Reconnecting the account afterwards does not release it.

## Impact

The affected conversation is unrecoverable, including all history in it. There is no user-visible escape hatch: the only remaining option is deleting the chat.

The trigger is not exotic. Any account whose credentials expire or are revoked between queueing and deciding will reproduce it, and a queued action is by definition waiting on a human, so the window can be arbitrarily long.

## Suggested fix

### Primary

Make Deny always succeed. Rejection should resolve against `ActionStore.reject()` without requiring a healthy connection or a resolvable remote facet, since the record is local and nothing has been sent. A user declining a call should never depend on the server the call would have gone to.

### Secondary

Provide a general escape hatch for a wedged pending action — for example, allowing it to be abandoned from the chat UI regardless of gatekeeper state. #97 shows that a chat can also be locked by an unrelated cause, so a generic way out would cover both, and any future path that leaves a conversation blocked on state that can no longer be resolved.

---

I am not proposing a PR, because the fix touches gatekeeper facet resolution and the pending-action state machine rather than being a small localised change, and `CONTRIBUTING.md` asks external contributors to report rather than submit design-level patches. Happy to supply more logs if useful.

Contributor guide

Open the contributing guide

Research direction

Start with packages/mcp-shared/src/action-store.ts, facet.ts, and tools.ts, then trace the client-to-facet path used by the pending action's Deny control under a broken account connection. Done means rejection reaches the local ActionStore.reject() without requiring the remote connection, the pending action clears, and the composer is usable again; consider the broader escape hatch described in the issue separately.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.