Implement 055_web_search_legacy_keys_to_config_secrets migration hook that:
- Extracts inline API keys from config.json5 (tools.web.{exa,tavily,brave})
- Migrates inline api_key fields from builtin_tool_tenant_configs settings JSON
- Inserts all keys into config_secrets under appropriate tenant scope
- Strips inline fields from settings blobs to prevent re-migration
Hook is idempotent, tracked in data_migrations table, PostgreSQL-only.
Guard unit coverage:
- MessageTargetEnforced helper table (10 session shapes)
- TestMessageToolCrossTargetGuard — 9 scenarios: DM/group same-target
pass, cross-target block, forward opt-in pass with notice; cron,
heartbeat, subagent, team FREE
- TestMessageToolCrossTargetGuard_TraceReplay — anchors the 019d9fcf
production trace against regression
- NoticeFallbackSender — msgBus-less deployment still gets the audit
breadcrumb via t.sender
- NoNoticeOnSendFailure — failed forward does not post a phantom notice
System prompt coverage:
- <current_reply_target> emitted for direct + group, omitted when
ChatID is empty
Pre-existing TestSelfSendGuard/text_to_different_chat_allowed updated
to pass forward=true — the test now correctly isolates the self-send
guard from the new cross-target guard.
Give the LLM an unambiguous reference for the currently bound chat so
it can compare against any target it plans to pass. Block lists channel,
chat_id and kind (direct/group) with a one-line policy: omit target to
reply here, only set target when forwarding per explicit user request.
Threads ChatID through SystemPromptConfig, IdentityData,
BridgePromptBuilder, and buildMessages so the pipeline populates the
block from pipeline.RunInput.ChatID.
Production trace 019d9fcf: agent in DM session called
message(target="-1003787954683:topic:1") and leaked a user's image to
the wrong Telegram group topic. No guardrail caught it.
Add explicit forward opt-in: in ENFORCED sessions, cross-target sends
require forward=true + non-empty forward_reason. Otherwise blocked with
a self-explanatory error that tells the LLM how to retry correctly.
Successful forwards emit slog.Warn("message.cross_target_forward") and
post a user-facing notice back to the origin chat via noticeOnSuccess
wrapper — notice only fires when the send itself succeeds, so failed
forwards no longer announce phantom deliveries.
Schema gains forward and forward_reason fields; description explains
when to set target vs omit.
Classifies session kinds into FREE (cron, heartbeat, subagent, team) vs
ENFORCED (DM, group, ws, default, unknown, empty). FREE kinds compose
targets per job spec; everything else must match the ctx-bound origin.
postCrossTargetNotice posts the audit breadcrumb back to the origin
chat when an explicit forward succeeds: msgBus preferred, falls back to
direct sender, else slog.Warn so the skip is traceable.
User-facing notice posted back to origin chat when the agent forwards a
message to a different target per explicit user request. Translations
added for en/vi/zh.
zenika/alpine-chrome is stuck at v124 — upstream stopped publishing
newer tags — and v124 with default headless mode doesn't support
--load-extension, so users couldn't load Chrome extensions during
automation.
Switch to chromedp/headless-shell:latest (Chrome 147+ at time of
commit, auto-tracks Chrome for Testing) and add --headless=new so
extension loading works. Rod connects to the same raw-CDP
ws://chrome:9222 endpoint — no client code change.
The image's entrypoint already configures the debugging port and
fronts it with socat, so our explicit --remote-debugging-*
flags have to be removed (otherwise Chrome fails with
"bind() failed: Address already in use").
Healthcheck switched from wget (absent in the minimal image) to a
bash /dev/tcp probe (bash is present and sufficient for a liveness
check on port 9222).
Verified: image pulls, Chrome starts, /json/version returns, health
probe succeeds.
Closes#722
golang-migrate's file source driver parsed "file://F:\\project\\..."
with "F" as host and ":\\..." as port → "invalid port" error, so
upgrade/migrate commands crashed on Windows before applying any SQL.
Format the source URL per RFC 8089: forward-slash separators and a
leading "/" before the drive letter → "file:///F:/project/...".
POSIX paths pass through unchanged (leading "/" already present).
Shape-driven, not OS-driven: no runtime.GOOS branch needed, which
also lets the unit test cover both shapes on a single runner.
Closes#872
Two pieces of the review-hardening turned out to be risky in production:
1. deduplicateMedia used filepath.Abs on each MediaResult.Path to
normalize the dedup key. filepath.Abs depends on the process CWD,
which varies across deployment environments. In containers with a
non-standard CWD, relative paths from parseMediaResult were being
rewritten into keys that could collide with or differ from
absolute paths from tools — leading to legitimate media entries
being silently eaten.
2. extractMediaFromContent overwrote the stored path with the
EvalSymlinks-resolved path. When the workspace is a bind-mount or
contains ancestor symlinks, the stored path diverged from what the
tool originally wrote and queued into rs.mediaResults, so dedup
against tool-origin entries failed and the same file could be
delivered twice (or fail altogether if the resolved path wasn't
reachable from the downstream reader's FS view).
Revert both to exact-string behavior. Keep EvalSymlinks for the
containment check only — the symlink-escape P0 remains closed, but
the stored path now matches what the tool wrote.
When tasks are dispatched via ticker (deferred/unblocked), the context
has no real sender — only synthetic "ticker:system". This caused
workers to fail file writes in group chats with "permission denied:
system context cannot write files".
- team_tool_dispatch.go: fallback to task.Metadata["origin_sender_id"]
when context sender is empty or synthetic
- task_ticker.go: propagate origin_sender_id and origin_role from task
metadata when building ticker dispatch message
Fixes#915 deferred dispatch flow.
Photo/video/audio/document uploads were hitting "context deadline
exceeded" mid-flight on slow or mobile networks. Two layers capped
the budget at 60s:
- sendOverallTimeout (wraps the whole retry sequence)
- http.Client.Timeout (caps a single HTTP request)
Both are fine for text sends but too tight for multi-MB uploads
(sketchnote images, PDFs, long voice clips).
Split the retry budget by method: text sends keep 60s, media sends
(sendPhoto/Video/Audio/Voice/Document) get a new sendMediaOverallTimeout
of 3 min. Bump the shared http.Client.Timeout to 3 min so a single
upload attempt can actually consume that budget — getUpdates long-poll
is 25s, so the extra headroom doesn't affect polling.
Closes#628
Review of the medium-batch landed this session surfaced three issues:
P0 — symlink-ancestor escape in extractMediaFromContent. The lexical
filepath.Rel check passed "<ws>/shared/secret" when "shared" was a
symlink pointing outside the workspace. EvalSymlinks the resolved path
(and the workspace root, since macOS uses /var → /private/var) before
the Rel containment check. Leaf symlinks still rejected by Lstat.
P1 — duplicate attachments from path aliases. parseMediaResult stores
raw tool output ("./tts/x.mp3"); extractMediaFromContent stores Abs+
Clean ("<ws>/tts/x.mp3"). deduplicateMedia's exact-string map let both
through. Normalize the dedup key via filepath.Abs + Clean so the
sources collapse.
P1 — TTS Result.Media used the non-standard "audio/mp3" via
"audio/"+Extension. SynthResult.MimeType is already populated by every
provider with the RFC-3003 "audio/mpeg" (or "audio/ogg" for opus).
Prefer it; keep the extension concat as empty-string fallback.
Tests: add symlink-leaf-rejected and ancestor-symlink-escape cases to
lock the P0 behavior.
pnpm build under a restrictive umask produces 0600 files; when copied
into nginx:alpine, the non-root nginx worker hits EACCES and serves
403 for every static asset. Force mode via COPY --chmod=0755 (Docker
BuildKit applies uniformly — directories need the exec bit for
traversal, static files don't care about the extra bit).
Closes#761
TTS relied solely on ForLLM "MEDIA:<path>" parsing. claude_cli's MCP
bridge reshapes ForLLM before the agent sees it, so rs.mediaResults
stayed empty and mp3s never attached to outbound messages.
Populate Result.Media alongside the legacy ForLLM prefix (matching
create_audio.go). The agent loop prefers explicit Result.Media over
the ForLLM fallback.
Closes#806
When a tool returns its artifact via ForLLM "MEDIA:<path>" and the LLM
echoes that path in its final response (PDF from exec/weasyprint, TTS
bridged through claude_cli MCP, etc.), sanitize stripped the token
without populating rs.mediaResults — attachments never reached the
channel.
Add extractMediaFromContent() and call it right before
SanitizeAssistantContent. Tight security: workspace must be set,
relative paths resolve against workspace, absolute paths must stay
inside workspace (blocks ../ escapes and sibling-tenant access),
file must exist and be a regular file. Dedup prevents duplicates
when a tool already populated Result.Media.
Closes#882
'make up' now pulls the latest published image instead of rebuilding
from local context, which was overwriting freshly pulled images with
stale cache. New 'make up-build' target preserves the build-from-source
flow for developers iterating on the backend.
Closes#934
Resolve each IANA name via Intl.DateTimeFormat().resolvedOptions() so
aliases like Asia/Saigon collapse into their canonical form
(Asia/Ho_Chi_Minh). Prevents cron/heartbeat pickers from saving a
deprecated alias that diverges from the Go-side canonical name.
Closes#760
List up to 20 wikilink/reference outlinks as "## Links" after the doc
body, deduped by target, scope-filtered via existing allowed() matrix.
Non-fatal — store errors log and skip the footer.
Transcription-only models (whisper-1, gpt-4o-transcribe,
gpt-4o-mini-transcribe) were sent to /chat/completions with an
input_audio content part, which OpenAI rejects with "not a chat model".
Add isTranscriptionModel detection and a multipart/form-data call to
/v1/audio/transcriptions for those models; chat-audio models
(gpt-4o-audio-preview) continue to use /chat/completions.
Also fix providerTypeFromName to treat "openai-*" prefixed providers
as "openai" type so a dedicated "openai-audio" provider reaches the
OpenAI-specific path instead of falling through to the generic chat
fallback.
Forum topic routing (Telegram supergroup threads) relies on local_key
carried through team task events to pick the correct thread for
notifications. Four broadcast sites dropped local_key by relying on
WithContextInfo(ctx) alone — but the member agent's isolated context
does not carry the originating local_key.
Read task.Metadata[TaskMetaLocalKey] explicitly and pass via
WithLocalKey() to match the already-correct pattern in
team_tool_dispatch.go and team_tasks_blocker.go.
Covered sites:
- team_tasks_lifecycle.executeComplete → EventTeamTaskCompleted
- gateway_consumer_post_turn agent-error → EventTeamTaskFailed
- gateway_consumer_post_turn loop-kill → EventTeamTaskFailed
- gateway_consumer_post_turn auto-complete → EventTeamTaskCompleted
Line 52 parses agent_id into agentUUID but line 59 passed the raw
event.AgentID string to ExistsBySourceID. PR #826 fixed the emission
side (loop_finalize.go) so event.AgentID is now always a valid UUID,
but the consumption side kept the inconsistent parse-then-discard
pattern.
Use agentUUID.String() to keep the parse-once-use-parsed invariant,
matching the tenantUUID pattern two lines above. Fixes the
"invalid input syntax for type uuid" error reported in #809 and the
downstream "..." Telegram symptom in #824 where episodic failure
cascaded into empty agent replies.
claude-cli emits API errors (rate limits, usage limits) via stream-json
stdout result events, not stderr. Previous code only checked
ev.Subtype == "error" and used an empty stderr buffer in the final
wrap, so users saw a generic "something went wrong" instead of the
real cause.
- Add IsError and Error fields to cliStreamEvent to parse both signals
- Capture stream error message when FinishReason flips to "error"
- Substitute captured message when stderr is empty in cmd.Wait path
- Extend gateway rate-limit classifier to recognize "usage limit"
filterCLIEnv stripped all CLAUDE_* env vars except OAUTH_TOKEN,
preventing CLAUDE_CODE_EFFORT_LEVEL and CLAUDE_CODE_DISABLE_ADAPTIVE_THINKING
from reaching the subprocess. Switch to an explicit allowlist map that
preserves auth + behavioral tuning vars.
Add effort parameter to buildArgs and plumb OptThinkingLevel from
req.Options through Chat/ChatStream so per-agent reasoning level maps
to --effort flag. When an explicit flag is set, strip the env var to
avoid precedence ambiguity. Validate effort with isAlphaOnly to reject
malformed input.
response.failed was lumped with response.completed/incomplete, causing
silent empty responses with no retry. Split into its own case that
returns a descriptive error from processSSEEvent, propagated up to
ChatStream caller for proper error surface and retry logic.
Added codexErrorDetail to codexAPIResponse to parse the error payload.
Part of the vault_read feature. Amend coreToolSummaries so the LLM sees:
- read_file: workspace-scoped, redirect to vault_read for vault discovery
- vault_search: explicit chaining hint to pass doc_id to vault_read
- vault_read: new summary describing shared/personal/team access
Agents could not read vault docs registered at the tenant workspace root
because read_file enforces a per-agent workspace boundary. vault_read
fetches content by doc_id (from vault_search) with scope-based access
checks (shared/personal/team), a 3-layer text-only gate (DocType +
extension blocklist + UTF-8 sniff), 500KB default cap / 1MB ceiling,
and symlink/hardlink guards shared with read_file. Updated system-prompt
summaries and boundary-error hints so the LLM chains vault_search →
vault_read instead of retrying read_file.
Root .dockerignore did not exclude local dependency folders, so
COPY ui/web/ . shipped ui/web's local install into the web-builder
stage. Stale artifacts (e.g. leftover rollup@4.57.1 from older vite@6)
then overrode the result of `pnpm install --frozen-lockfile`,
breaking the musl build on Alpine with:
Cannot find module @rollup/rollup-linux-arm64-musl
Add explicit ignores for ui/web dependency and dist folders plus a
catch-all for any nested dep folder so Docker builds are
deterministic and match CI regardless of local dev state. Also
speeds up build context transfer.
Fixes a 3-vuln chain enabling unauthenticated/underprivileged access to
admin surfaces:
1. Auth bypass (router.go): invalid tokens silently fell through to
anonymous role. Now reject on token presence with invalid/expired.
2. Default-permit RBAC (policy.go): unknown methods returned a writable
role. Now fail-closed: unknown -> RoleNone.
3. Missing admin classifications: several sensitive methods weren't in
any allowlist and slipped through default-permit. Now explicitly
classified (admin/write/read).
Additional hardening:
- Remove isWriteMethod prefix fallbacks; use exact allowlist only to
prevent silent scope creep when new methods are added.
- Classify Phase-3 methods (TTS, browser, zalo, whatsapp) into
admin/write/read per sensitivity.
- Gate empty MethodRole in router dispatcher.
Tests:
- TestMethodRole_ApprovalsList_IsViewer: asserts exec.approval.list is
viewer and approve/deny are operator.
- TestMethodRole_DriftCoverage_AllProtocolMethodsClassified: AST-parses
pkg/protocol/methods.go at test time; asserts every Method* constant
resolves to a non-None role. Permanent drift guard for future adds.
Expose the original acting sender ID in agent events so group-chat
telemetry and audit logs stay aligned with the real Telegram user
instead of only the group-scoped UserID.
The repo already validates provider base URLs during provider CRUD, but
TTS test-connection built ephemeral providers from user-supplied api_base
without reusing that protection. This change applies the same validation
at test time and adds regression coverage for blocked loopback targets.
Constraint: Must preserve current local-provider SSRF policy semantics from provider validation
Rejected: Restrict test-connection to admins only | larger behavior change than needed for the bug
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep TTS test-connection URL validation aligned with validateProviderURL behavior for provider CRUD
Tested: go test ./internal/http -run 'TestTestConnection_(MissingProvider|UnsupportedProvider|MissingAPIKey|BelowOperator|BlockedAPIBase)$' -count=1 -v
Tested: go build ./...
Tested: go build -tags sqliteonly ./...
Not-tested: Full repo race test suite
Co-authored-by: viettranx <viettranx@gmail.com>
The new per-tenant TTS config surface diverged from the established TTS
schema after the audio refactor, dropping UI alias fields and root-level
settings during round-trip. It also returned ok=true even when non-secret
system config writes failed. This patch accepts backward-compatible field
aliases, round-trips timeout and Edge settings, reuses tenant timeout/rate
when building ephemeral providers, and returns errors on failed saves.
Constraint: Must preserve the existing TTS config schema expected by dashboard flows and legacy config docs
Rejected: Fix only the UI payload shape | backend would still break legacy aliases and direct API callers
Confidence: high
Scope-risk: moderate
Reversibility: clean
Directive: When extending per-tenant config handlers, keep request/response structs aligned with the established config schema and do not ignore store write errors
Related: #935
Related: #936
Tested: go test ./internal/http -run 'Test(TTSConfig|TestConnection_)' -count=1 -v
Tested: go build ./...
Tested: go build -tags sqliteonly ./...
Not-tested: Full repo race test suite
* fix(tests): resolve integration test compile errors
- Remove duplicate allowLoopbackForTest in hooks_pipeline_test.go (canonical version lives in v3_test_helper.go)
- Remove unused fakeClient assignment in mcp_grant_revoke_test.go; fakeMCPClient type retained for future use
* ci: bump unit test timeout from 90s to 5m
The internal/hooks/handlers package binary under `-race -coverpkg=./...`
now runs up against the 90s cap because HTTPHandler retry uses a real
`time.After(1 * time.Second)` backoff across three HTTP test cases, and
goja-based memory-bomb sandbox tests have large allocations that run
inline before the sandbox deadline kicks in.
Recent main CI has been red with this timeout firing on different slow
tests each run (TestHTTP_5xxRetriesOnce, TestCorpus_MemoryBombString).
Bumping to 5m keeps the deadlock safety net (still half the 10-minute
Go default) while giving slow-but-non-deadlocked packages room.
Followup: make HTTPHandler backoff configurable so tests can override
with ms-scale delays and the 90s cap can come back.
Also update `make test` in Makefile to match.
* test(mcp): skip RevokeUserGrant test pending Phase 02 implementation
Commit 8b8da3a3 added this test alongside the grant-checker, but the
user-grant revocation semantics were never implemented: ListAccessible's
SQL treats an absent mcp_user_grants row as "allowed by default"
(WHERE mug.id IS NULL OR mug.enabled = true), so RevokeFromUser's DELETE
leaves the server accessible.
The test never actually ran in CI until the prior compile fix in this PR
unblocked the integration test binary. It's safe to skip — the agent-grant
counterpart test still exercises the grant-recheck code path. Re-enable
once user-grant-required semantics land.
Add useLayoutEffect hook to sync textarea height whenever value changes
or component mounts. Prevents browser's default rows=1 height from
leaving a gap above the icons when textarea content is externally
reset (e.g., after send). Adjust icon padding for better vertical
alignment with multi-line textarea.
Add missing ResetStuckSummoning method stub to mock AgentStore
implementations in test files. Follows from recent agent recovery
feature (d7342fbd) which added this method to the AgentStore interface
but missed updating these 4 test mock implementations.
Add pipBuildFailHint helper that suggests root causes when pip wheel build fails.
Detects missing system deps (pg_config for psycopg, libssl for cryptography, etc).
Hints suggest apt/brew/yum package names or pip alias alternatives (psycopg2→psycopg2-binary).
Wired to dep_installer stderr/stdlog post-build-failure for visibility.
Add manifest-origin dependency declarations in SKILL.md frontmatter.
New fields: deps (pip/npm/system) and exclude_deps (override search results).
Validates dependencies against per-category regex allowlists to prevent injection.
Manifest overrides can extend or reduce auto-discovered deps.
Backward compatible: skills without deps:/exclude_deps: behave identically.
Includes regression guard (bundled_smoke_test.go) for 5 bundled skills.
Add pip package alias resolutions for commonly-misnamed or distribution-variant packages.
psycopg2 shipped as sdist-only on PyPI (requires pg_config); aliases to psycopg2-binary.
Other entries handle distribution variants (MySQLdb→mysqlclient, Crypto→pycryptodome, etc).
Follow-up to #942. The HTTP onboarding path (registerInMemory) still diverged
from the startup path in two ways that #942 deliberately deferred:
1. Anthropic: startup wires WithAnthropicRegistry(modelReg) (cmd/gateway_providers.go:349),
HTTP skipped it — forward-compat model resolution + token counting used static
defaults for HTTP-onboarded providers.
2. Claude CLI: startup validates filepath.IsAbs + exec.LookPath before registering
(cmd/gateway_providers.go:290-297), HTTP skipped both — DB could be poisoned
with relative paths (resolves against CWD) or non-existent binaries that would
pass verify but crash on first spawn.
Changes:
- Add ProvidersHandler.modelReg field + SetModelRegistry setter
- Thread modelReg through wireHTTP in cmd/gateway_http_handlers.go
- Anthropic branch: append WithAnthropicRegistry when modelReg non-nil
- Claude CLI branch: mirror startup validation (reject non-absolute, skip if LookPath fails)
Tests added:
- TestProvidersHandlerRegisterInMemorySkipsClaudeCLIWhenBinaryMissing — asserts
provider NOT registered when binary path cannot be resolved
- TestProvidersHandlerRegisterInMemoryAnthropicUsesModelRegistry — reflection-probe
verifies registry wiring (no public getter on AnthropicProvider)
- Updated existing CLI test to use a real temp binary so exec.LookPath succeeds
regardless of whether \`claude\` is installed in the runner's PATH
White text on bg-primary (oklch 0.62) scored ~3:1 and failed WCAG AA.
Switch the user badge to an outline with a soft primary tint so the
label stays readable in light mode while still visually distinct from
the group (secondary) and topic (outline) types.
- user_search: return tenant_user UUID in new "uuid" field so callers
forwarding a tenant_user foreign key receive the primary key instead
of the user_id string (root cause of HTTP 400 "invalid JSON" on
/v1/contacts/merge when linking to an existing tenant user)
- use-user-picker: add opt-in valueMode prop ("user_id" default | "uuid")
so only merge-contacts-dialog commits UUID; other pickers (tenant
add-user, MCP/CLI credentials) keep returning user_id string
- pg.MigrateUserDataOnMerge: split tClauseIns/tClauseDel with correct
placeholder offsets (N+2 for INSERT/UPDATE, N+1 for DELETE) — reusing
one clause caused "no parameter \$N+2" errors that silently skipped
every cleanup DELETE, leaving orphan per-user rows after a merge
Add Cancel button in SummoningModal after 60s threshold. Call POST
/v1/agents/{id}/cancel-summon via getApiClient(). Update agent status
to summon_failed. Add i18n strings for en/vi/zh in desktop locale.
Add Cancel button in SummoningModal after 60s threshold. Call POST
/v1/agents/{id}/cancel-summon via useHttp hook. Update agent status
to summon_failed. Add i18n strings for en/vi/zh in agents locale.
Add ResetStuckSummoning store method (PG + SQLite) + startup sweep to
flip ghost rows to summon_failed on boot. Drop 409 guards on resummon
and regenerate to allow retry. Add POST /v1/agents/{id}/cancel-summon
endpoint + audit event agent.summon_cancelled + i18n (en/vi/zh).
Closes credential-scope bypass where an agent without a `secure_cli_agent_grants` row could still invoke a registered binary via shell fallthrough and pick up inherited env (`$GH_TOKEN`) or on-disk OAuth state. Registration is now an authorization boundary, not only a credential-injection hint.
- Add `SecureCLIStore.IsRegisteredBinary` on PG + SQLite backends — case-insensitive, tenant-scoped.
- New gate branch in `internal/tools/shell.go` (after normalization, before exec approval) with shell-wrapper unwrap depth 3 (sh/bash/zsh/dash -c, env K=V, nohup, stdbuf, timeout), 2s DB-lookup timeout, fail-CLOSED on error. New log events: `security.credentialed_binary_denied`, `security.credentialed_binary_gate_error`, `security.credentialed_binary_wrapper_too_deep`.
- Env scrubbing on host fall-through (`internal/tools/env_scrub.go`) strips static credential keys (GH_TOKEN, AWS_*, OPENAI_API_KEY, …) plus dynamic keys from tenant's registered binaries; preserves HOME/PATH/TERM/LANG/USER/TZ.
- Subagent ExecTool registration (`cmd/gateway_agents.go` → `buildSubagentToolsRegistry`) now receives the same `SecureCLIStore` — parent can't delegate to a child to bypass the gate.
- Binary names lowercased on Create/Update for symmetry with case-insensitive lookup.
- Thread `pgStores.SecureCLI` into `setupSubagents` in `cmd/gateway.go`.
- 25+ unit tests (gate, env-scrub, SQLite IsRegisteredBinary) + 5 PG integration tests (deny-ungranted, allow-granted, unregistered-unchanged, is_global-not-denied regression, shell-wrapper-bypass denied). All green.
- Remove duplicate allowLoopbackForTest in hooks_pipeline_test.go (canonical version lives in v3_test_helper.go)
- Remove unused fakeClient assignment in mcp_grant_revoke_test.go; fakeMCPClient type retained for future use