unbridled-41 opened a new pull request, #4738:
URL: https://github.com/apache/rocketmq-dashboard/pull/4738

   Fixes #4736.
   
   ### Problem
   
   The manual tool playground kept an in-flight execution alive across a tool 
or scope change and rendered its response in the output pane of whatever tool 
was selected afterwards. The pane is labelled with the generic `ai.tool.output` 
and never names the tool that produced the text, so tool A's output was 
presented under tool B's name, risk level and permission tags.
   
   Chain: `handleExecuteTool` 
(`web/src/pages/ai/components/ToolPlaygroundModal.tsx:200-225`) awaits 
`executeTool(selectedToolName, parsedInput, selectedClusterId)` and assigns the 
response to `toolResult` with no generation check. The paths that clear the 
panel do not invalidate that call:
   
   - `selectTool` (`:127-135`) clears the panel but leaves the request running, 
and the tool `Select` (`:267-281`) stays enabled while executing — only the Run 
button takes `loading` (`:247-250`).
   - `loadTools` (`:137-161`, reached from a cluster-scope change via 
`:191-198`) clears the panel the same way.
   - `handleClose` (`:227-233`) bumps only `toolLoadRequestRef`, the catalog 
guard, so a response can also land after the modal was closed and reopened (no 
`destroyOnHidden`, so the state survives).
   
   Trigger (deterministic): open 工具 on the AI page, run a tool that takes a few 
seconds (the RocketMQ tools do), switch the tool in the selector before it 
returns. The panel is cleared by the switch, then repopulated by the late 
response, with the success toast and the provider payload shown under the new 
tool.
   
   ### Root cause
   
   The component guards its *catalog* load against a stale response 
(`toolLoadRequestRef`, documented at `:40-43`) but the execute path has no 
equivalent generation: `toolExecuting` prevents a second *start*, not a late 
*landing*.
   
   ### Fix
   
   Add `toolExecuteRequestRef`, bumped by every path that changes what the 
panel belongs to (tool selection, catalog reload for a scope change, close) and 
captured before the await in `handleExecuteTool`. The result and its success 
toast are applied only when the request is still the current one; the `finally` 
still clears `toolExecuting` unconditionally, so the Run button can never stay 
stuck. A failed execution keeps reporting its error only while it is still the 
current request — a stale failure would otherwise interrupt the operator with 
an error about a tool they are no longer looking at.
   
   ### Priority
   
   **68** = impact 24 + reach 10 + reproducibility 20 + maintenance value 14.
   
   - Impact 24/40: misattributed tool output. The component is documented as 
the escape hatch for verifying what a transcript's tool call returns, so a 
result shown under the wrong tool is a false reproduction rather than a missing 
one.
   - Reach 10/20: one dialog, but every execution slower than the operator's 
next click, in both real and mock mode.
   - Reproducibility 20/20: deterministic — a deferred `executeTool` and one 
selector change.
   - Maintenance value 14/20: completes the stale-guard pattern the same file 
already documents for the catalog load.
   
   `FIX_CONFIDENCE`: **90** — the failing test and the sibling guard fix the 
expected behavior; the change is a ref, four increments and one comparison.
   
   ### Tests
   
   Red (source at base commit `7ce9a682`, new test added):
   
   ```
   npx vitest run src/pages/ai/__tests__/AiPage.test.tsx -t 
dropsAnInFlightToolResult
       × dropsAnInFlightToolResultWhenAnotherToolIsSelectedTest
   Error: expect(element).not.toBeInTheDocument()
   expected document not to contain element, found <pre
   ```
   
   Green (this branch):
   
   ```
   npx vitest run src/pages/ai/__tests__/AiPage.test.tsx          → 16 passed 
(16)
   npx vitest run src/pages/ai src/api/ai                         → 18 files, 
253 passed (253)
   npx vitest run --maxWorkers=4                                  → 134 files, 
1236 passed (1236)
   npx tsc -b                                                     → clean
   npx eslint <changed files>                                     → clean
   npm run build                                                  → ✓ built in 
9.28s
   ```
   
   The regression test resolves the deferred call *after* the tool switch and 
asserts the pane stays empty; the pre-existing 
`opensTheToolPlaygroundLoadsTheCatalogAndExecutesAToolTest` still pins the 
happy path (the result does render while the tool stays selected).
   
   ### Risk
   
   Low. Only an execution whose tool/scope changed (or whose modal was closed) 
is discarded; a normal run is untouched, and the loading state is cleared 
unconditionally so no button can stick. Behavior worth naming: a request that 
is superseded mid-flight no longer reports its outcome at all — deliberately, 
because the discarded request's error would describe a call the operator is no 
longer looking at.
   
   ### Base branch
   
   Targets `rocketmq-studio`, the development trunk. Note the repository's 
default branch is still `master`; GitHub interprets the closing keyword only 
for PRs that target the default branch, so the `Fixes` link above does not 
auto-close #4736 when this merges — the issue needs to be closed by hand.
   


-- 
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]

Reply via email to