unbridled-41 opened a new pull request, #4257:
URL: https://github.com/apache/rocketmq-dashboard/pull/4257
Fixes #4256.
## Problem / Evidence
`rmq.message.query` and `rmq.message.trace` read their `cluster` input into
the instance-id slot (`MessageQueryToolHandler.java:48`,
`MessageTraceToolHandler.java:47`) and pass it to `MessageService`, which
resolves a registered instance via `providerRegistry.byInstanceId` →
`RuntimeAdminClientResolver.resolveInstance` →
`instanceRepository.findByIdentifier` (unique instance name or numeric id). The
consumers of these tools supply a cluster id instead:
- the AI page's tool dialog prefills every field named `cluster` with the
selected cluster id (`buildToolInputTemplate` ← `listClusters()` →
`cluster.id`);
- no tool output exposes an instance id, so the model cannot know one either;
- the catalog declares no description for the field, and
`requiredCapabilities: []` skips the gateway's capability check, so the wrong
value flows straight to the handler.
Result: opening the AI tool dialog for `rmq.message.query` prefills
`"cluster": "<clusterId>"` and every run fails with `404 Instance not found:
<clusterId>` — the same failure mode commit 206314b9 (#4115) recorded and fixed
for the two list tools, whose cluster-scoped branches the message tools do not
have.
## Root cause / Fix
The two runtime tools were the only tools whose required input names a
different entity than the field name implies. This change gives them an honest
contract:
- `rmq-tools.yaml`: the two tools' required input is renamed `cluster` →
`instance`, with a description naming the expected identifier (unique name or
numeric id).
- both handlers read `instance`.
- `ToolCatalog.validateClusterConvention` now accepts `cluster` or
`instance` as the declared target field, so the "every remote tool declares
what it is addressed by" invariant keeps holding for instance-addressed runtime
tools; `ToolGatewayService.requiresCluster` (renamed `requiresTargetContext`)
applies the same rule so discovery keeps hiding the message tools until a
cluster context resolves.
- the AI page needs no code change: a required field no longer named
`cluster` simply stops receiving the cluster id; a test pins that.
## Priority & scoring
PRIORITY 72 = impact 27 (both message tools unusable from the manual tool
dialog — every run fails with a misleading 404 — and the catalog contract
actively misleads chat runs) + scope 12 (two handlers, their catalog entries,
one validation predicate) + reproducibility 18 (deterministic UI repro, covered
by regression tests at catalog, gateway and UI-template layers) + maintenance
value 15 (aligns the tool framework with its own runtime tools; unblocks the
message-tool improvement line around #4148). FIX_CONFIDENCE 85: deterministic
behavior, verified red→green at three layers; the residual chat-mode limitation
(the model must ask the operator for the instance identifier when unknown) is
inherent to the deployment model and stated in the Risk section for the
maintainer to weigh.
## Tests
- New `ToolCatalogTest.messageToolInputsAreInstanceAddressed` asserts the
two tools require `instance` (not `cluster`) and describe it.
- New
`ToolGatewayServiceTest.messageQueryToolTargetsTheInstanceNamedByItsInput` /
`messageTraceToolTargetsTheInstanceNamedByItsInput` execute both tools through
the real gateway with an `instance` input and verify `MessageService`
delegation.
- Red on the unfixed code: `Tests run: 43, Failures: 1, Errors: 2` — the
gateway tests fail with `BusinessException: Tool input validation failed for
rmq.message.query: [: required property 'cluster' not found, : property
'instance' is not defined…]` and the catalog test fails on `required contains
cluster`.
- Green after the change: `ToolCatalogTest` 7/7, `ToolGatewayServiceTest`
36/36, `MessageQueryToolHandlerTest` 6/6, `MessageTraceToolHandlerTest` 1/1
(re-run after the final commit: `Tests run: 50, Failures: 0`).
- Full server suite `mvn -o test`: 2154 tests, 2 failures = the standing
baseline set (`AuthCorsIntegrationTest` ×2), i.e. 2151 baseline + 3 new tests
with zero new failures.
- Web: new `AiPage` test pins that an instance-addressed tool's template is
`{"instance": ""}` — never the selected cluster id (`AiPage.test.tsx` 18/18).
Full `npx vitest run`: 982 tests, 3 failures all in untouched files
(`ClusterPage` ×1, `ConsumerPage` ×2 — known load-fragile under the parallel
suite); both files re-run in isolation: 56/56 pass. `npx tsc -b` clean, `npx
eslint .` 0 errors, `npx vite build` succeeds.
## Risk
The input contract of the two tools changes (old `cluster`-keyed payloads
are rejected by input validation with an explicit error instead of failing
later with a misleading 404 — no working consumer can be affected since the old
path never succeeded). The framework's target-field invariant is relaxed to
accept `instance`; `ToolGatewayService.enforceCapabilities` still reads the
`cluster` field for capability checks, which remains unreachable for these
tools (`requiredCapabilities: []`) but is flagged here as a coupling to revisit
if capabilities are ever required of the message tools. In chat mode the model
must obtain the instance identifier from the operator when unknown; the field
description now states what is expected.
Head: 4f78b5ab (fix/ai-message-tool-instance-input, 1 commit).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
