yyqdbngt opened a new pull request, #4685:
URL: https://github.com/apache/rocketmq-dashboard/pull/4685
<!-- Base branch: `rocketmq-studio`, the RocketMQ Studio trunk. -->
### Brief Description
A conversation remembers the agent CLI's session id
(`rmq_ai_conversation.runtime_session_id`) and
`AiRunService.resolveResume` hands it back to the CLI as `claude --resume
<id>` on **every** later
turn. `ResumeRecovery` documents why that id can stop existing — the
per-conversation workspace
(`studio.ai.conversation.workspace-dir`, default
`/tmp/rocketmq-studio-ai/conv-<id>`) holds `HOME`
and `$HOME/.claude/projects/<cwd-hash>/<id>.jsonl`, so one container restart
is enough — and it also
documents the required recovery:
> When `shouldRetryWithoutResume` returns true the caller must, exactly
once: … **clear
> `conversation.runtime_session_id`, so the next turn does not repeat the
failure** and the new
> session id from the retry's `result` frame becomes the one that is
remembered.
That class had **no caller anywhere in `src/main`** (only its own unit test
in
`RmqctlWorkspaceTest`), so nothing ever cleared the id. A resumed run whose
session is gone fails
with `error_during_execution`; `AiRunExecutor.finalizeRun` then wrote
`run.setRuntimeSessionId(terminal.outcome().runtimeSessionId)` — the id
*echoed back by the failed
frame*, which the parser explicitly warns is "the requested id … not a live
one"
(`ClaudeCodeStreamParser#providerErrors`) — and `rememberRuntimeSession`
left that same id on the
conversation. The web client never sends `resume: false`
(`web/src/api/aiConversations.ts` declares
the field, nothing sets it) and there is no "start a new session"
affordance, so every subsequent
message resumed the same dead id and failed identically: **the conversation
was permanently
unusable**.
`AiRunExecutor` now applies the documented recovery. A failed run that was
resuming a session
forgets that session instead of recording the echoed id, so the next turn
starts a fresh provider
session:
- `AiRunExecutor#lostResumeSession` / `#forgetLostResumeSession` — the run
row keeps `resumed_from`
as history but no longer records the dead id as its session, and the
conversation's stored id is
dropped.
- `ResumeRecovery#isLostResumeSubtype` — the subtype arm of
`isLostResumeSignal` is now reusable:
the run worker sees projected frames only, and the exit code and the
stderr line that carry the
other arm die with the subprocess (a run that reached this path already
failed, which is the
exit-code half of that signal).
- `AiConversationRepository#clearRuntimeSessionId` — `updateById` omits null
entity fields, so a
cleared column needs an explicit assignment; the same idiom
`MybatisPlusInstanceRepository` and
`MybatisPlusAclRepository#clearColumn` already use. `gmt_modified` is
deliberately untouched: this
is a state repair, not user activity, and bumping it would reorder the
conversation list on the
strength of a failure.
Not done, deliberately: the immediate one-shot retry that `ResumeRecovery`
also specifies. Turning
it on needs the CLI's exit code and stderr, which only
`ClaudeCodeAgentProvider` ever sees, so it is
a separate change to the provider/executor seam. Clearing the id alone
already removes the permanent
breakage — the next message works instead of failing forever.
### How Did You Test This Change?
New test:
`AiRunExecutorTest#aLostResumeSessionShouldNotBeResumedByTheNextTurnTest` —
scripts the
measured frame of a lost resume (a `result` frame that echoes the requested
id with
`subtype=error_during_execution`) and asserts what is written.
Before the fix (red) — the run row kept the echoed id, so the next turn
would resume it again:
```
$ cd server && mvn -B -ntp test
-Dtest='AiRunExecutorTest#aLostResumeSessionShouldNotBeResumedByTheNextTurnTest'
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed:
2.589 s <<< FAILURE! -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiRunExecutorTest
[ERROR]
org.apache.rocketmq.studio.ops.ai.conversation.AiRunExecutorTest.aLostResumeSessionShouldNotBeResumedByTheNextTurnTest
-- Time elapsed: 2.283 s <<< FAILURE!
org.opentest4j.AssertionFailedError:
expected: null
but was: "gone-session"
at
org.apache.rocketmq.studio.ops.ai.conversation.AiRunExecutorTest.aLostResumeSessionShouldNotBeResumedByTheNextTurnTest(AiRunExecutorTest.java:480)
```
The red run is that same test method without its last line,
`verify(conversationRepository)
.clearRuntimeSessionId(CONVERSATION_ID)`: the port method it verifies is
introduced by this change,
so on the unmodified tree it would not compile. The assertion quoted above
is the first one that
fails and it is unchanged by the fix's shape — it is the run row, i.e. the
value the next turn
resumes.
After the fix (green) — the new test plus the whole neighbouring AI
conversation surface:
```
$ cd server && mvn -B -ntp test
-Dtest='AiRunExecutorTest,AiConversationPersistenceIntegrationTest,AiRunServiceTest,AiEventSeqAllocationTest,AiTimelineRepositoryTest,AiConversationServiceTest,AiStreamControllerTest,AiConversationControllerTest,AiConversationVoAssemblerTest,RmqctlWorkspaceTest'
[INFO] You have 0 Checkstyle violations.
[INFO] Tests run: 16, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiRunExecutorTest
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiConversationPersistenceIntegrationTest
[INFO] Tests run: 33, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiTimelineRepositoryTest
[INFO] Tests run: 27, Failures: 0, Errors: 0, Skipped: 0 -- in
org.apache.rocketmq.studio.ops.ai.conversation.AiConversationControllerTest
[INFO] Tests run: 198, Failures: 0, Errors: 0, Skipped: 3
[INFO] BUILD SUCCESS
```
The durable half is proved against a real database rather than a mock:
`AiConversationPersistenceIntegrationTest#clearingTheRememberedProviderSessionShouldReachTheColumnTest`
(`@SpringBootTest @ActiveProfiles("dev")`, i.e. H2 in MySQL compatibility
mode, so it runs in CI)
sets `runtime_session_id`, clears it and reads the row back as `null`, and
asserts an unknown id is a
zero-row update rather than an exception. The whole module test-compiles with
`mvn -B -ntp test-compile` (BUILD SUCCESS, `0 Checkstyle violations`).
Note on the full suite: on a clean `rocketmq-studio` checkout `mvn -B -ntp
test` already reports
`Tests run: 3051, Failures: 6, Errors: 25, Skipped: 4`. The 11 red classes
are the MySQL 8 backed
Spring integration tests (`AuthServiceBootstrapIntegrationTest`,
`AuthServiceConcurrencyIntegrationTest`,
`AuthServiceSessionOverviewIntegrationTest`,
`HealthProbeIntegrationTest`, `QueryHistoryServiceIntegrationTest`,
`NativeAlertEvaluationTransactionTest`,
`NotificationOutboxMapperIntegrationTest`,
`RmqAlertStateMapperIntegrationTest`, `StudioApplicationTest`) plus the
external-CLI ones
(`CliAgentProviderTest`, `ClaudeCodeAgentProviderTest`). None of them are
touched by this change.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix(ai): …`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(no UI text in this change)
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks; the new dependency is `conversation` -> `conversation.agent`, which
already exists)
- [x] New source files carry the ASF license header (no new files)
- [x] Documentation touched where behaviour changed (no user-visible
contract change: the conversation keeps behaving as documented, it just stops
being unusable after a workspace is lost)
--
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]
