You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "unbridled-41 (via GitHub)" <gi...@apache.org> on 2026/09/20 00:15:51 UTC
[PR] fix(ai): let platform tools pass the capability filter without an instance [rocketmq-dashboard]
unbridled-41 opened a new pull request, #4560:
URL: https://github.com/apache/rocketmq-dashboard/pull/4560
Fixes #4556.
## Problem / Evidence
Nine of the forty catalog tools are platform-level: addressed by a physical `clusterName`, exempt from the mandatory `instanceId` (`ToolCatalog.INSTANCE_ID_EXEMPT_TOOLS`), with input schemas that reject `instanceId` (`additionalProperties: false`) — yet every one declares non-empty `requiredCapabilities`. `ToolExecutionService.resolveTargetInstance` dispatches them with a **null** context instanceId by design, and `ToolCapabilityFilter` then calls `CapabilityResolver.resolve(null)` → `CAPABILITY_INSTANCE_REQUIRED`. Every call fails on every transport (REST `POST /api/ai/tools/{name}/execute` and MCP `tools/call`), while `ToolDiscoveryService` and `McpToolRegistrar` advertise the tools. Affected: `rmq.cluster.list`, `rmq.dashboard.summary`, `rmq.broker.list/describe/config`, `rmq.nameserver.list/config`, `rmq.proxy.list/config`.
Reproduction (red test output): `ToolExecutionException: Capability lookup requires a bound Instance` from a platform tool dispatched with `{}` input through the full filter chain.
## Root cause / Fix
`ToolCapabilityFilter` assumed every capability-gated tool is instance-scoped. The filter now proceeds when `context.instanceId()` is null **and** the tool is instanceId-exempt (the exact combination `resolveTargetInstance` produces for platform tools). Instance-scoped tools keep the lookup unchanged; a null instanceId on a non-exempt tool remains impossible (`TOOL_INSTANCE_REQUIRED` fires earlier).
## Priority & scoring
PRIORITY 82 = impact 32 (9 of 40 advertised tools dead, including the whole platform-inspection surface) + reach 14 (AI page, rmqctl, MCP clients) + repro 20 (deterministic) + maintain 16 (unblocks #4306's flagship surface; discovery/execution contract inconsistency). FIX_CONFIDENCE 90 (two-line guard + full-chain regression test).
## Tests
- New `ToolPlatformCapabilityTest.platformToolWithoutInstanceExecutesThroughTheCapabilityFilterTest` runs `rmq.cluster.list` through the real `ToolFilterChain` (AUDIT → VALIDATION → CAPABILITY → execution) with a null instanceId.
- Red (baseline): `Tests run: 1, Failures: 0, Errors: 1` — `ToolExecutionException: Capability lookup requires a bound Instance`.
- Green: `Tests run: 1, Failures: 0, Errors: 0`.
- Related suites green after the change: ClusterListToolHandlerTest 2/2, ToolCatalogTest 5/5, ToolInstanceRoutingTest 4/4, ToolGatewayServiceTest (12 total across the batch run, 0 failures).
- Full backend suite (`SPRING_PROFILES_ACTIVE=dev mvn clean test`): **2396 tests, 1 failure** — `AuthServiceSessionOverviewIntegrationTest` (a 59 s/60 s wall-clock boundary assertion), unrelated to this change; the class passes 3/3 in isolation (re-run three times with the dev profile). Baseline for `origin/master` under the same profile: 2395/0 (this branch adds the 1 new test → 2396).
## Risk
Low. The skipped capability check only applies to tools that by contract have no instance context; per-instance gating for these tools already happens in discovery, and their handlers resolve physical clusters themselves (`PlatformClusterResolver`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ai): let platform tools pass the capability filter without an instance [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #4560: fix(ai): let platform tools pass the capability filter without an instance
URL: https://github.com/apache/rocketmq-dashboard/pull/4560
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ai): let platform tools pass the capability filter without an instance [rocketmq-dashboard]
Posted by "unbridled-41 (via GitHub)" <gi...@apache.org>.
unbridled-41 commented on PR #4560:
URL: https://github.com/apache/rocketmq-dashboard/pull/4560#issuecomment-5746350374
## Verification evidence
**CI (current head)**: head SHA `2f189cb5165cbc4958e5cb88c1b4a24e5590c249` has exactly one workflow run — "CI" (pull_request), run id 35478307449, conclusion `startup_failure`, 0 check-runs / 0 statuses on the head commit. Upstream CI startup_fails for every branch/PR in this repository (infrastructure-level, not caused by this change); local verification below substitutes for it.
**Red (fix reverted, test kept)**: `mvn test -Dtest=ToolPlatformCapabilityTest` on the baseline → `Tests run: 1, Failures: 0, Errors: 1` — `ToolExecutionException: Capability lookup requires a bound Instance`, the exact production error.
**Green**: same command with the fix → `Tests run: 1, Failures: 0, Errors: 0`.
**Related suites** (same run batch): ClusterListToolHandlerTest 2/2, ToolCatalogTest 5/5, ToolInstanceRoutingTest 4/4 — total 12 run / 0 failures / 0 errors.
**Full backend suite**: `SPRING_PROFILES_ACTIVE=dev mvn clean test` (the dev profile wires the H2 datasource the integration tests need) → **2396 tests, 1 failure**. The failure is `AuthServiceSessionOverviewIntegrationTest.listActiveSessionsForUserReturnsOnlyThatUsersActiveSessionDetailsTest` (asserts `idleSeconds >= 60` and observed 59 — a wall-clock second boundary). It is pre-existing: the identical failure reproduces on pristine `origin/master` (verified in a clean worktree), and the class passes 3/3 when isolated on this branch.
**Baseline reference**: `origin/master` @ d50ffecc under the same profile = 2395 tests / 0 failures on the baseline run earlier this session; this branch adds 1 test → 2396.
**Diff self-check**: `git show --numstat 2f189cb5` → `ToolCapabilityFilter.java +8/−0`, `ToolPlatformCapabilityTest.java +107/−0`. Two files, no unrelated changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ai): let platform tools pass the capability filter without an instance [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins commented on PR #4560:
URL: https://github.com/apache/rocketmq-dashboard/pull/4560#issuecomment-5812251395
Closing this one: the premise it guards against no longer exists on the current baseline, and the guard itself contradicts a design decision that is documented in the code.
**1. The premise is false on trunk.** The PR adds a bypass in `ToolCapabilityFilter.filter` for the case where `context.instanceId() == null` and the tool is instance-id-exempt, on the basis that both entry points pass `null` for exempt tools. They do not. `ToolExecutionService.resolveTargetInstance` (`server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/service/ToolExecutionService.java:139-152`) computes `boolean exempt = ToolCatalog.isInstanceIdExempt(definition.name())` at `:144` and, when the tool takes no `instanceId` argument, returns the transport-bound instance at `:148`:
```java
if (exempt) {
return targetInstanceId == null || targetInstanceId.isBlank() ? null : targetInstanceId;
}
```
Its own Javadoc says why: exempt tools' "Instance comes from the transport binding (the signed MCP header, or the target the console operator selected) so that the capability gate still has something to resolve against". So an exempt tool normally arrives at the gate *with* an instance, and the `null` case is the exceptional one — not the routine one this PR assumes.
**2. The guard weakens a gate that is deliberately closed.** `ToolCapabilityFilter`'s class Javadoc (`:33-38`) states the intent explicitly:
> Platform-level tools (`ToolCatalog.isInstanceIdExempt`) take no `instanceId` argument, but the gate is deliberately *not* skipped for them … An exempt tool therefore still runs only against an Instance that supports it, and an exempt tool with no binding at all fails loudly with `CAPABILITY_INSTANCE_REQUIRED` instead of running unscoped.
That error is real and reachable (`CapabilityResolver.java:41` throws `ToolError.CAPABILITY_INSTANCE_REQUIRED`, defined at `ToolError.java:63`). Skipping the capability check whenever `instanceId` is null turns "fail loudly" into "run unscoped", i.e. it removes the only signal that an exempt tool was invoked without a resolvable instance.
The same Javadoc also records why the binding cannot leak data: "Exempt handlers never read `context.instanceId()` — only this gate and `ToolAuditFilter` do — so binding one cannot widen the data a platform-level tool returns."
If you are hitting a concrete path where an exempt tool reaches the gate with no binding and you need it to succeed, please open an issue with the reproduction (which transport, which tool, which auth state) — the fix would be to bind the instance on that path, as `resolveTargetInstance` already does for the MCP and console paths, rather than to exempt the path from the gate. No rework is expected on this PR as it stands.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org