fix(mcp): surface catalog failures instead of serving an empty tool list by sheagreco · Pull Request #40 · mwtcmi/frogman

sheagreco · 2026-07-30T19:03:05Z

What

tools/list reported success with an empty tool array whenever the FreePBX ajax endpoint was unreachable. It now returns a JSON-RPC error naming the URL it tried.

Why

get_mcp_tools() returned [] on any backend failure. Two things followed from that:

The failure was invisible. tools/list answered {"tools": []}, so the MCP client showed a happily-connected frogman server with zero tools and no reason given. Diagnosing it meant reading the server's stderr, which most MCP clients bury.
The failure was cached for the life of the process. [] is not null, so $toolCache got populated with the empty result on the first call. Every later tools/list short-circuited on that cache and kept serving an empty catalog — even after the backend came back.

I hit this on a stock Sangoma-layout FreePBX 17 box. There, port 80 is the Let's Encrypt vhost with DocumentRoot /invalid/folder/name and the admin control panel is on 8080, so the default http://localhost/admin/ajax.php gets Apache's 403 HTML page. The MCP server connected fine and offered zero tools with no explanation.

Changes

mcp-server.php — get_mcp_tools() now returns ['tools' => [...]] or ['error' => string]. tools/list sends -32603 with the attempted URL and the underlying error, and leaves $toolCache at null so the next request retries. Also guards a catalog response that has no tools key, which previously hit an unchecked foreach.
mcp-config.example.json — FROGMAN_FREEPBX_URL was set in an env block. Over the SSH transport that sets the variable on the local ssh process; sshd never forwards it to the remote PHP unless the server is configured with AcceptEnv. It silently did nothing and the server always fell back to its built-in default. Moved inline into the remote command.
README.md — documents both traps in the MCP section, with a one-liner to check which port serves the ACP.

I also added -o BatchMode=yes to the example ssh args so a missing or locked key fails fast instead of blocking the stdio pipe on a password prompt. Happy to drop that if you'd rather keep the example minimal — it's the one change here that isn't strictly the bug.

No version bump, per CONTRIBUTING.

Testing

Real FreePBX 17 box: Frogman 2.8.1, PHP 8.2.32, Debian 12. The patched server was run from /tmp so the installed module stayed untouched.

$ php -l mcp-server.php
No syntax errors detected in /tmp/mcp-server-pr.php

Failure path — two consecutive tools/list against the wrong port:

$ printf '%s\n' \
  '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"probe","version":"1.0"}}}' \
  '{"jsonrpc":"2.0","method":"notifications/initialized"}' \
  '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' \
  '{"jsonrpc":"2.0","id":3,"method":"tools/list","params":{}}' \
| FROGMAN_FREEPBX_URL=http://localhost/admin/ajax.php php /tmp/mcp-server-pr.php

{"jsonrpc":"2.0","id":2,"error":{"code":-32603,"message":"Cannot reach the Frogman tool catalog at http://localhost/admin/ajax.php — Invalid JSON response (HTTP 403): <!DOCTYPE HTML ...
{"jsonrpc":"2.0","id":3,"error":{"code":-32603,"message":"Cannot reach the Frogman tool catalog at http://localhost/admin/ajax.php — Invalid JSON response (HTTP 403): <!DOCTYPE HTML ...

id: 3 erroring as well is the part that matters — before this change the second call returned the cached {"tools": []}.

Success path — same probe plus a tools/call, against port 8080:

id1 initialize -> frogman-mcp v1.0.0
id2 tools/list -> count=255  first=fm_add_allowlist
id3 tools/call -> fm_list_extensions returned count=25

Catalog endpoint directly, for reference:

$ curl -s "http://localhost:8080/admin/ajax.php?module=frogman&command=catalog"
{"status":"success","module":"frogman","version":"2.8.1","tool_count":255,...}

Note on process

CONTRIBUTING asks for an issue first on behavior changes. I read this one as an obvious bug fix — the current path caches a failed request and reports it as success — but if you'd rather see it as an issue first, say the word and I'll move it over.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CWe9ds1uAffXsVvE3LZUXM

sheagreco · 2026-07-30T19:57:23Z

Pushed one more doc commit (96311a3) covering a third failure mode I hit while setting this up on Windows. Same theme as the rest of the PR — a silent failure that looks like Frogman's fault and isn't.

Electron-based MCP clients spawn servers with a curated environment that drops ProgramData. Windows OpenSSH resolves its system config through a __PROGRAMDATA__ token, so without that variable ssh.exe aborts during startup: exit 255 in under 20ms, zero bytes on stderr, no log file even with -vv -E. The client shows only Server transport closed unexpectedly.

Reproduced and confirmed by elimination — spawning ssh with a stripped environment, then removing variables one at a time from a working set:

baseline (full env):          exit=0   stdout="CONNECTED"
minimal (PATH+USERPROFILE):   exit=255  19ms  stderr=0b
  REQUIRED: ProgramData       exit=255        stderr=0b
minimal + ProgramData:        exit=0   stdout="CONNECTED"

And end to end through the client's own config, with and without the fix:

without env block:  EXITED code=255  18ms  stderr=0b  responses=[]
with env block:     alive            8s    responses=[id1=ok id2=tools:255]

Worth documenting because nothing in the failure points at ssh — there's no message anywhere to search for. mcp-config.example.json is unchanged since the env block is Windows-specific; the README now carries a full Windows example.

This is also the one case where an env block is correct, which is worth stating explicitly next to the "env blocks don't reach the remote" note earlier in the PR — it sets the local ssh process's environment, not the remote PHP's. Both are true and they're easy to confuse.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CWe9ds1uAffXsVvE3LZUXM

mwtcmi · 2026-07-31T02:16:01Z

Thanks for this — really good catch. The cache-poisoning half is the subtle bit: [] isn't null, so once a backend failure lands, the process serves an empty catalog for its whole life. That would be miserable to debug from a client. Verified against current main, the fix path is correct and the "leave $toolCache = null on error" comment is exactly the right lesson to leave behind for the next reader.

The README additions are genuinely useful. The AcceptEnv / env-block-doesn't-forward trap catches a lot of people, and the Windows ProgramData note ("exit 255 in under 20ms, no stderr") reads like it cost you real time to find, which is the writing that pays forward. Please leave both in.

Keep BatchMode=yes too. You flagged it as optional and it isn't, actually. A key needing a passphrase hanging on a stdio pipe with no way to answer is the exact failure mode people can't diagnose.

To your process note: this is fine as a PR. CONTRIBUTING's "issue first" is for behavior changes, and this is a bug fix where the current behavior is "silently lie about a healthy connection." No issue needed.

One small ask before merge:

Strip the Claude trailers from both commit messages. Both commits carry Co-Authored-By: Claude Opus 5 ... and Claude-Session: ... lines. Frogman doesn't credit Claude as a contributor on the repo, so those need to go before merge.

Paste-ready:

On the fix/mcp-catalog-error-surfacing branch, remove the Co-Authored-By: Claude ... and Claude-Session: ... lines from both commit messages.

Easiest way: git rebase -i main, mark both commits as reword, and delete those two lines from each message when git opens the editor. Then git push --force-with-lease origin fix/mcp-catalog-error-surfacing.

Once that's in, I'll verify on the dev box (I can reproduce the failure path with a bad FROGMAN_FREEPBX_URL) and ship as v2.8.2. Patch bump since it's MCP-only, no tool count change. Credit to you in the tag annotation.

Really appreciate this. Welcome to the repo.

When the FreePBX ajax endpoint is unreachable, get_mcp_tools() returned an empty array. That had two consequences: - tools/list answered {"tools": []} with no error, so the client showed a connected server with zero tools and no indication of why. - [] is not null, so $toolCache was populated with the failure and every later tools/list served the empty catalog for the life of the process, even after the backend recovered. get_mcp_tools() now returns ['tools' => [...]] or ['error' => string], and tools/list sends a JSON-RPC -32603 naming the URL it tried. Failures are left uncached so the next request retries. Also fixes mcp-config.example.json: FROGMAN_FREEPBX_URL was set in an "env" block, which applies to the local ssh process and is never forwarded to the remote PHP unless sshd is configured with AcceptEnv. Moved inline into the remote command so it actually takes effect.

Electron-based MCP clients spawn servers with a curated environment that omits ProgramData. Windows OpenSSH resolves its system config through a __PROGRAMDATA__ token, so ssh.exe aborts during startup without it — exit 255 in under 20ms with no stderr and no log output. The client surfaces only "Server transport closed unexpectedly", which points at Frogman rather than at ssh. Documents the env-block workaround, and notes why this is the one case where an env block is correct (it sets the local ssh process environment, not the remote one).

sheagreco · 2026-08-07T18:49:25Z

Trailers stripped from both commits and force-pushed — messages only, the diff is unchanged and authors/dates are preserved. New SHAs: 761dcd7 (fix) and 3c43b24 (docs).

Agreed on BatchMode=yes — the README already frames it as non-optional ("worth keeping on any platform... the client hangs instead of failing"). The "optional" was my PR description overreaching, not the doc. Leaving both README sections as-is.

One note for anyone else following the rebase steps: in my clone origin is this repo and my fork is a separate fork remote, so the force-push had to target fork, not origin.

sheagreco added 2 commits August 7, 2026 14:44

sheagreco force-pushed the fix/mcp-catalog-error-surfacing branch from 96311a3 to 3c43b24 Compare August 7, 2026 18:45

Navigation Menu

fix(mcp): surface catalog failures instead of serving an empty tool list - #40

fix(mcp): surface catalog failures instead of serving an empty tool list#40
sheagreco wants to merge 2 commits into
mwtcmi:mainfrom
sheagreco:fix/mcp-catalog-error-surfacing

sheagreco commented Jul 30, 2026

Uh oh!

sheagreco commented Jul 30, 2026

Uh oh!

mwtcmi commented Jul 31, 2026

Uh oh!

sheagreco commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

sheagreco commented Jul 30, 2026

What

Why

Changes

Testing

Note on process

Uh oh!

sheagreco commented Jul 30, 2026

Uh oh!

mwtcmi commented Jul 31, 2026

Uh oh!

sheagreco commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants