unbridled-41 opened a new pull request, #4796: URL: https://github.com/apache/rocketmq-dashboard/pull/4796
Fixes #4795. ## Problem / Evidence `AiRunService.sendMessage` inserts the run row (`QUEUED`) and only afterwards calls `workspace.prepare(...)`, outside any try/catch. `prepare` deliberately throws `BusinessException` for configuration problems — an unusable `rmqctl-server-url`/`workspace-dir`, or an instance whose credential no longer resolves (`InstanceCredentialResolver.resolveByName` → 404/422). The exception left the inserted row `QUEUED` with no owner: admission's one-active-run guarantee then refused every later message for the conversation with 409 until the scheduled orphan sweep reaped the row — up to 24 hours (`cleanup-interval:PT24H`), or a restart. Line references in the linked issue are as of trunk `4c697f07`; references below are as of the patched file at this PR's head. ## Root cause / Fix The admission ordering (insert row, then prepare, so the caller hears configuration errors synchronously) is right; the missing piece is a failure path for the row the ordering implies. The catch now finalizes the run through the exactly-once terminal path — `runExecutor.terminate(detachedContext(run), FAILED, PROVIDER_ERROR, ai.run.internal_error, …)`, the same mechanism the stop of an owner-less run already uses — and rethrows so the caller still hears the original reason. `registry.register` has not run yet at this point, so a detached context (no handle, fresh sink) is the correct terminal writer, exactly as in `stop`'s owner-less branch. ## Priority PRIORITY 70 / 100 — impact 28 (a conversation becomes unusable for up to a day; the user-visible 409 contradicts the visible state, since no answer is actually in progress), blast radius 14 (every native conversation bound to an instance whose binding breaks — the trigger is an ordinary admin action), reproducibility 14 (deterministic given the trigger), maintenance value 14 (fills the one unguarded admission step in a lifecycle the codebase otherwise treats as exactly-once everywhere). FIX_CONFIDENCE 92: the fix reuses the project's own detached-finalize idiom verbatim; the only judgement call is the terminal reason (`PROVIDER_ERROR`/`ai.run.internal_error`, consistent with the executor's unexpected-failure vocabulary). ## Tests - `AiRunServiceTest.aFailingWorkspacePreparationShouldFinalizeTheRunInsteadOfStrandingItTest` (new): stubs `prepare` to throw 404, asserts the exception still propagates, the row was inserted once as `QUEUED`, the final row is `FAILED/PROVIDER_ERROR`, and nothing is left live in the registry. - Teeth, measured on this machine: with only the test change on trunk source, it fails with the terminal update list empty (`Expecting actual not to be empty` at `lastRun()` — the stranded-QUEUED shape); with the fix, `AiRunServiceTest` 21/21. - Related suites: `AiRunExecutorTest` 18/18, `AiConversationServiceTest`, `AgentRunRegistryTest` 11/11, `AgentRunHandleStopTest` 9/9, `RmqctlWorkspaceTest` 31/31 — 108/108 total. `mvn checkstyle:check` passes. ## Risk Low. The catch only adds a terminal write on a path that previously left inconsistent state; the rethrow preserves the existing API contract (caller sees the original `BusinessException`). If the terminal write itself were to fail, the exception would replace the original — acceptable, since the orphan sweep remains the backstop and the log line records the cause. -- 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]
