Skip to content

fix(mcp-apps): let the consent message reach the toast - #1013

Merged
philmerrell merged 1 commit into
developfrom
fix/app-tool-consent-message-reaches-the-toast
Sep 9, 2026
Merged

fix(mcp-apps): let the consent message reach the toast#1013
philmerrell merged 1 commit into
developfrom
fix/app-tool-consent-message-reaches-the-toast

Conversation

@philmerrell

Copy link
Copy Markdown
Contributor

Follow-up to #1009, found by validating it on dev after deploy.

What #1009 fixed, and what it didn't

#1009 did what it claimed — the status and message now survive the AgentCore Runtime boundary intact:

POST /api/mcp-apps/proxy-call → 409
{"error":"Authorization required for 'google-tasks'. Connect the account, then try again."}

409, not 424, and the real message is in the body. That part is confirmed working on dev.

But the user still didn't see it. The toast read:

Conflict — The request conflicts with the current state.

Root cause

ErrorService.handleHttpError looks for a message under detail, error.detail, error.message, or message — in that order. app-api returns error as a plain string, which matches none of them (priority 2 requires error to be an object), so it falls through to createErrorMessageFromStatus and renders the generic per-status text while the real message sits unread in the body.

This also explains the pre-#1009 symptom, and it's the part worth noticing: AgentCore's 424 body used the key message, which did match priority 3. So the useless "check your CloudWatch logs" text was rendered for exactly the reason the useful text was not.

The fix

app_tool_error_body() emits the same text under both keys:

  • errorMcpAppProxyService reads err.error?.error and hands it to the iframe's JSON-RPC reply (unchanged behaviour).
  • detail — what ErrorService actually renders, and FastAPI's own HTTPException shape, which the rest of this router already emits.

No SPA change. The fix is to speak the key the SPA already reads, rather than to teach the SPA a fifth shape.

Testing

  • 2 new guards on the body shape; both relay tests now assert detail alongside error.
  • Mutation-tested: dropping detail fails 4 named tests across all three files; the mutant compiles.
  • 46 passed across the envelope and both route suites. ruff clean on every changed file.

Still open after this

The dev account genuinely isn't connected to Google Tasks, so the consent path is the live behaviour there — that's what made it testable. Once this deploys I'll confirm the toast reads the real message.

Unrelated and still unexplained from #1009's investigation: no POST /invocations appears in the runtime log group that serves dev, so I still can't say which of the three 409 paths fires. Worth its own issue.

🤖 Generated with Claude Code

Follow-up to #1009, from validating it on dev. #1009 did what it claimed —
the status and message now survive the AgentCore boundary intact:

    POST /api/mcp-apps/proxy-call → 409
    {"error":"Authorization required for 'google-tasks'. Connect the
     account, then try again."}

But the user still didn't see it. The toast read:

    Conflict — The request conflicts with the current state.

`ErrorService.handleHttpError` looks for a message under `detail`,
`error.detail`, `error.message`, or `message`. app-api returns `error` as a
plain **string**, which matches none of those, so it fell through to the
generic per-status fallback while the real text sat unread in the body.

That also explains the pre-#1009 symptom: AgentCore's 424 body used the key
`message`, which *did* match — so the useless "check your CloudWatch logs"
text was rendered for exactly the reason the useful text was not.

## The fix

`app_tool_error_body()` emits the same text under both keys:

- `error` — `McpAppProxyService` reads `err.error?.error` for the iframe's
  JSON-RPC reply (unchanged).
- `detail` — what `ErrorService` renders, and FastAPI's own `HTTPException`
  shape, which the rest of this router already emits.

No SPA change: the fix is to speak the key the SPA already reads.

## Testing

- 2 new guards on the body shape; both relay tests now assert `detail`.
- Mutation-tested: dropping `detail` fails 4 named tests across all three
  files; the mutant compiles.
- 46 passed across the envelope + both route suites. `ruff` clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@philmerrell
philmerrell merged commit edd45f4 into develop Sep 9, 2026
4 checks passed
@philmerrell
philmerrell deleted the fix/app-tool-consent-message-reaches-the-toast branch September 9, 2026 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant