isAllCapsPlaceholder() classified ANY string of >=3 chars made only of A-Z and
underscore as an LLM placeholder, and stripEmptyOptionalArgs() then removed it
from the outbound CallTool arguments. Real business identifiers match that shape:
bank codes BKASH, NAGAD, ROCKET, VCB
currency codes BDT, VND, USD, PKR, MMK
enum values SUCCESS, PENDING, ALL
Impact is silent and severe: dropping an optional filter turns a filtered query
into an unfiltered one. The MCP server still answers 200 with the full result set,
so nothing errors — the model reports totals for the entire dataset as if they were
the filtered result, with full confidence.
Observed in production: asking "how many BKASH proxies are active" returned 6,610
(all banks) instead of 1,263. Three different bank codes returned byte-identical
results, which also drove the model into a retry loop until the loop guard fired.
Diagnosis was slow because the removal is invisible: the outbound log records
args_len BEFORE stripping, and stripEmptyOptionalArgs logs nothing at all — while
normalizeArgsForSchema right below it does emit mcp.tool.args.coerced. Confirmed
the argument never reaches the server by echoing the raw JSON-RPC arguments from
RequestContext[CallToolRequestParams] on the server side.
Changes:
- isAllCapsPlaceholder: require multi-word (contains "_") or membership in a known
single-word placeholder list. Keeps the original intent — the doc comment's own
examples (SHOULD_NOT_BE_HERE, DO_NOT_SEND, NOT_APPLICABLE) are all multi-word —
while letting business codes through.
- stripEmptyOptionalArgs: emit slog.Warn("mcp.tool.args.stripped") whenever an
argument is dropped, so this class of bug is one grep away instead of a
multi-hour investigation.
- Tests: guard both layers against regression.
Verified: go build ./... clean; go test ./internal/mcp/ passes.
* feat(mcp): filter tools at registration + detect FastMCP session reset with force-reconnect
Two foundational MCP reliability improvements:
(1) Tool allow/deny filtering at BridgeTool registration: Previously the runtime grant-check at execute time surfaced "grant revoked" errors when the LLM called a tool it wasn't allowed to call. Filter upfront in both the per-agent registration path (manager_connect.go) and per-user registration path (loop_mcp_user.go). Adds tool_filter.go with IsToolAllowed + tests. This eliminates the "registered then runtime-denied" loop.
(2) FastMCP session reset detection + force-reconnect: FastMCP/Python mcp servers reject tools/call as "invalid during session initialization" when the upstream session lifecycle resets but our pool still holds the old Mcp-Session-Id. Detector matches three known phrasings (FastMCP, mcp-go, mcp-go transport ErrSessionTerminated) in session_reset.go. On detection, BridgeTool.Execute requests a force-reconnect via atomic CAS dedup so N concurrent failing calls collapse to one reconnect. Health loops skip ping while pending so a server answering ping in "initializing" state cannot clobber connected=true before the fresh Initialize completes. Includes 30s timeout, structured slog telemetry, concurrent CAS dedup test.
Files: tool_filter.go + tool_filter_test.go (new), session_reset.go + session_reset_test.go (new), manager_connect.go (connectServer/connectViaPool signatures + registerBridgeTools/registerPoolBridgeTools filter logic + reconnPending skip), manager.go (connectAndFilter + connectServer call signature changes), loop_mcp_user.go (filter-at-register block + WithForceReconnect wiring), pool.go (reconnPending skip), bridge_tool.go (session reset detection + WithForceReconnect callback).
* fix(mcp): self-heal grant cache + bypass per-user grant for system/empty userID
Two production fixes for "MCP tool: grant revoked" recurring on song-nhi-v2.
(1) System-user bypass in ListAccessible: Registration uses LoadForAgent(ctx, agentID, "") while execute uses IsAllowed(ctx, agentID, "system", ...). The LEFT JOIN on mcp_user_grants could match a stale disabled row keyed user_id='system' and silently filter the server out only at execute. Skip the join entirely for synthetic owner identities (userID="" or "system") so registration and execute see the same set. Applied to both PostgreSQL (mcp_servers_access.go) and SQLite (mcp_servers_access.go).
(2) Grant-checker no-cache on empty allowByServer: grant_checker.loadEntry now skips the cache write when allowByServer is empty. Without this, a single transient empty result pinned permanent denial until a bus invalidate fired. Re-queries until the empty condition clears, then caches normally. Includes TestStoreGrantChecker_EmptyEntryNotCached.
Resolve the PR #8/#9/#10 stack on current dev, including Bitrix24 install callback hardening, migration renumbering, duplicate-domain fail-closed routing, UI textarea/mobile cleanup, and review hardening.
- Add GrantChecker interface with cache + event-bus invalidation for revalidating MCP tool grants at execution time (not just dispatch)
- Move toolGroups from package-level global to per-Registry field; Clone() gives each agent Loop isolated tool groups, eliminating cross-agent race condition
- BridgeTool.Execute rechecks grant validity before executing MCP methods; uses serverID field for cache key
- Wire MCPGrantChecker through ResolverDeps to MCP Manager and LoopConfig
- Add integration test for grant revocation scenarios
- Update policy functions to accept *Registry parameter
* fix(mcp): full reconnect for SSE/HTTP after server-side restart
When an MCP server using SSE or streamable-http transport restarts
(container redeploy, crash, OOM), the old client holds a stale session
ID. tryReconnect only called Ping() on the dead client, which keeps
POSTing to /message?sessionId=<dead> — the server returns 404, and
after maxReconnectAttempts the connection is permanently dead.
For pool connections, poolHealthLoop only set connected=false without
attempting any reconnect at all.
Fix:
- Store connection params (url, headers, command, args, env) in
serverState so tryReconnect can create fresh connections.
- tryReconnect now has two phases: fast path (ping existing client for
transient blips) and slow path (close old client, create fresh one
via createClient + Start + Initialize, swap ss.client).
- Add updateBridgeToolClients to propagate the new client pointer to
all registered BridgeTools after a full reconnect.
- Add poolTryReconnect with the same two-phase pattern for
pool-managed connections.
- Add BridgeTool.swapClient for safe client pointer replacement.
Closes#810
* fix(mcp): address review findings — atomic client pointer, close-after-swap, cooldown
Red-team review found critical issues in the initial reconnect fix:
1. Data race: BridgeTool.client written by healthLoop, read by Execute
concurrently without synchronization. Fix: BridgeTool now holds
*atomic.Pointer[mcpclient.Client] shared with serverState.clientPtr.
Execute uses atomic Load(), reconnect uses atomic Store().
2. Close-before-swap: old client was closed before new one was ready.
If createClient failed, ss.client pointed to closed client permanently.
Fix: create and validate new client first, swap atomically, then close old.
3. Pool BridgeTool orphaning: poolTryReconnect swapped ss.client but
existing BridgeTools held old pointer with no update mechanism.
Fix: atomic.Pointer propagates automatically — no explicit update needed.
4. Permanent death: after maxReconnectAttempts, server was permanently
dead with no recovery. Fix: 5-minute cooldown then reset attempts.
5. healthFailures not reset in fast path: minor inconsistency fixed.
Also fixes external caller in loop_mcp_user.go and adds ClientPtr()
accessor to poolEntry for atomic pointer sharing.
* refactor(mcp): extract fullReconnect helper, add nil guard, clarify dual-pointer design
- Extract shared fullReconnect() used by both tryReconnect and
poolTryReconnect — eliminates 30-line duplication (-17 lines net)
- Add nil guard on clientPtr.Load() in BridgeTool.Execute to prevent
nil deref in edge cases (test fixtures, initialization race)
- Add doc comment on serverState explaining dual-pointer design
(client for healthLoop, clientPtr for BridgeTools)
Replace direct sentinel error comparisons (==, !=) with errors.Is()
across the codebase. Direct comparison breaks when errors are wrapped
with fmt.Errorf("...: %w", err), which can cause missed matches for
sql.ErrNoRows, io.EOF, context.DeadlineExceeded, and context.Canceled.
Affected packages:
- store/pg: sql.ErrNoRows in secure_cli, agents_export_team_*, mcp_export_queries
- store/sqlitestore: sql.ErrNoRows in schema migration
- tools: context.DeadlineExceeded in shell, credentialed_exec
- mcp: context.DeadlineExceeded in bridge_tool
- providers: context.Canceled in acp_provider, anthropic_stream_test
- updater: io.EOF in tar extraction
Also removes unused @xyflow/react dependency from web UI — it was
replaced by react-force-graph-2d but never cleaned up from package.json.
Co-authored-by: Luvu182 <208665161+Luvu182@users.noreply.github.com>
- Enable merge UI for linking channel contacts to tenant_users
- Contact → tenant_user resolution with cached lookup (60s TTL)
- MCP per-user credentials via user-keyed connection pool
- Secure CLI per-user credentials with AES-256-GCM encryption
- Unified UserPickerCombobox searching contacts + tenant_users
- Group contact collection with chat title in all channels
- Group permission inheritance via wildcard user_id="*"
- Fix heartbeat using wrong userID in group chats
- Filter internal senders from contact collection
- Add contact_type column (user/group) to channel_contacts
- SQLite schema v2 migration for desktop edition
LLMs often send empty strings, "null", "SHOULD_NOT_BE_HERE" etc. for optional
MCP tool parameters instead of omitting them, causing servers to reject invalid
values (e.g. UUID validation fails on "").
- Add stripEmptyOptionalArgs() to clean optional args before MCP calls
- Detect known placeholders (null/none/nil) and all-caps patterns
- Add system prompt guidance for optional parameter handling
- Empty string "" is intentionally NOT stripped (may be valid for text fields)
MCP tools that accept no parameters return {type: "object"} without a
properties field, which is valid per JSON Schema spec. However, OpenAI
API strictly requires "properties" on object schemas and rejects
requests with HTTP 400: "object schema missing properties".
Add default empty properties map when type is "object" and the MCP
server provides none.
Co-authored-by: Luvu182 <208665161+Luvu182@users.noreply.github.com>
- Update go.mod and Dockerfile to Go 1.26
- Apply `go fix ./...` stdlib modernizations across 170+ files
- Add `go fix` to post-implementation checklist in CLAUDE.md
- Fix go fix misapplied rewrite in loop_history.go
Security: fix cross-agent MCP tool leak by cloning tool registry before MCP registration.
MCP: enforce mcp_ prefix on all tool names, add cache invalidation on server/grant changes,
add grant management endpoints, add group:mcp policy support for per-agent allowlisting.
Skills: persist full YAML frontmatter, auto-promote/demote visibility on grant/revoke,
simplify versioning, handle ZIP wrapper directories, expand tilde in skillsDir path.
Fixes: wrap DeleteSkill cascade in transaction, use atomic NOT EXISTS for revoke-demote,
create cancel context before storing server in map.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Multi-agent AI gateway with WebSocket RPC, HTTP API, and messaging channel integrations.
Go port of OpenClaw with multi-tenant PostgreSQL, per-user isolation, security hardening,
and production observability.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Phan Thong IT