pilichoumao opened a new pull request, #4665:
URL: https://github.com/apache/rocketmq-dashboard/pull/4665

   ### Which Issue(s) This PR Fixes
   
   No tracking issue. Closes a reliability gap between the documented and the 
implemented confirmation semantics: [docs/ai-agent-architecture.md 
§9](https://github.com/apache/rocketmq-dashboard/blob/5b761df8362eaf464c0b86ebea27e7e9576dd08f/docs/ai-agent-architecture.md)
 states that a dry-run returns `confirm_token` and "apply consumes it", but 
`ToolTokenService.verify()` never recorded usage, so one valid token could 
drive the same non-idempotent mutation (message send, offset reset, resource 
delete) repeatedly.
   
   ### Brief Description
   
   Make confirmation tokens single-use with an atomic consume-before-mutate 
step.
   
   - **Unique token id.** Each preview embeds a random 16-hex-char id in the 
signed payload; the token format moves `v1.<expiry>.<sig>` → 
`v2.<expiry>.<tokenId>.<sig>`. Two previews of the same operation within the 
same second now yield distinct tokens, so consuming one never invalidates a 
legitimately separate confirmation.
   - **Atomic consumption.** `verify()` becomes `verifyAndConsume()`: expiry, 
signature and tool/caller/instance/input bindings are checked first (unchanged 
`INVALID_ARGUMENT` rejections, which do **not** consume), then the id is 
consumed with a single `ConcurrentHashMap.putIfAbsent`. `ToolMutationFilter` 
calls it before generating the execution plan and before `chain.proceed`, so 
concurrent or repeated applies of one token admit exactly one mutation.
   - **Identifiable rejection.** A consumed token is rejected as `409 CONFLICT` 
/ `CONFIRMATION_TOKEN_ALREADY_USED` with a re-preview hint; 
expired/tampered/rebound tokens keep their existing `400 INVALID_ARGUMENT` 
result, and expiry is checked before the consumed state so neither rejection 
leaks information about the other.
   - **Deployment-model-consistent storage.** Consumed ids live in an in-memory 
store (`ConsumedTokenStore`) inside the singleton service — no Redis, no schema 
migration, matching the single-node confirmation flow. Entries are purgeable 
once their token expires (opportunistic purge past a size bound); a post-expiry 
replay is rejected by the existing expiry check, so purging never widens the 
acceptance window. Legacy `v1` tokens (≤10 min old at deploy time) are rejected 
as invalid; the recovery hint is to re-preview.
   
   **Scope boundary, deliberately:** this defines one-time consumption only. If 
a mutation starts but the response is lost, resubmitting the same token is 
rejected and the caller must preview again — cross-request idempotency, restart 
recovery and distributed consumption are separate storage/product decisions, 
not claimed here.
   
   `rmqctl` and the web console treat the token as an opaque string and always 
fetch a fresh preview token per execution, so no client change is needed.
   
   ### How Did You Test This Change?
   
   - **Fail-before regression on `rocketmq-studio@5b761df8`:** a new 
`ToolTokenSingleUseTest` (real `ToolTokenService` + real `ToolMutationFilter` 
chain) failed 3/3 on the unfixed code — sequential replay executed twice, 8 
concurrent applies executed 8 mutations, and two previews produced 
byte-identical tokens.
   - **New/updated tests, all passing:**
     - `ToolTokenSingleUseTest` — replay rejected before re-execution; 8-thread 
concurrent apply executes the mutation exactly once (`[200, 409]`); distinct 
previews stay independent.
     - `ToolTokenServiceTest` — v2 format/size, fresh id per issue, 
tool/caller/instance/input binding, expiry boundary (599s vs 600s), tampered 
version/expiry/id/signature bytes, malformed and legacy-v1 tokens, 
consumed→`CONFLICT` with expiry precedence, 8-thread concurrent consume elects 
one winner, signatures containing dots/non-UTF8 bytes still parse.
     - `ConsumedTokenStoreTest` — first-consumer-wins, replays stay rejected 
within TTL, post-expiry entries become purgeable, 16-thread contention elects 
exactly one winner.
     - `ToolTokenConfigurationTest` — full Spring-wired chain: preview→apply 
executes once, replay is rejected `CONFLICT` without touching the handler; 
missing-secret behavior unchanged (`UNAVAILABLE`).
     - `ToolControllerTest` — HTTP mapping: replayed token surfaces as `409` 
with `code=CONFLICT`, message and re-preview hint.
     - `ToolMutationFilterTest` — read-only tools still never touch the token 
service or the store.
   - **Build:** Java 21, `mvn -B -ntp -Dspring.profiles.active=dev package` — 
**3,045 tests passed**, 0 failures, **0 Checkstyle violations** (including test 
sources), packaged successfully, dependencies resolved into a clean local 
repository.
   - **End-to-end against a real local Apache RocketMQ 5.5.0 NameServer/Broker 
cluster:** the unfixed jar built from `5b761df8` and the fixed jar ran side by 
side against the same cluster. Via the real `POST 
/api/ai/tools/rmq.topic.update/execute` API: baseline replay of one token 
returned `200 EXECUTED` **twice**; the fixed build returned `200 EXECUTED` then 
`409 CONFLICT`, concurrent applies of one fresh token produced exactly `[200, 
409]`, a tampered copy and a token rebound to another topic returned `400 
INVALID_ARGUMENT`, tokenless read-only `rmq.topic.list` kept working, and the 
topic was visible on the broker (the admitted apply really executed). All 
temporary JVMs were shut down afterwards.
   
   Not exercised: multi-node deployments (the confirmation flow is single-node 
by design, no shared store introduced), MySQL profile (local credentials; clean 
baseline reproduces the same context errors), ACL/TLS.
   
   ### CI Status
   
   The repository's Actions policy currently rejects action versions referenced 
by the existing workflow (`docker/setup-buildx-action@v3`, 
`docker/build-push-action@v6`), so GitHub-hosted runs end in `startup_failure` 
before any job starts (same as #4550). This PR does not change the workflow; 
all results above are local.
   
   ### Checklist
   
   - [x] One coherent change; no unrelated modifications
   - [x] Conventional commit subject; regression tests use `...Test`
   - [x] Fail-before regression demonstrated on the unfixed baseline
   - [x] New user-visible error text carries a recovery hint (single English 
string, matching existing `ToolError` style)
   - [x] Architecture and formatting checks pass (0 Checkstyle violations)
   - [x] No new dependency, port, service or configuration; storage lifecycle 
matches the single-node model
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


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