fix(example): keep multiple Claw chats and restore history when opening a session - #3105
fix(example): keep multiple Claw chats and restore history when opening a session#3105xy-ygz wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ec85e59 to
93270bf
Compare
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Fixes #2735: routes each Claw chat to its own gateway session via a |t:<conversationId> thread peer, reads transcripts with the same sessionId used on write, and hydrates the Chat panel unconditionally. Root-cause analysis is thorough and the three new tests cover the mutation paths well.
Verdict
Please fix the unvalidated conversation id before this is merged: it is echoed straight into the routing key that sessionMatchesAgent later authorises against, so the delimiter characters in a key are attacker-controlled.
Automated review by github-manager-bot
| return new ChatIdentity(minted, minted, true); | ||
| } | ||
|
|
||
| private static String normalizedConversationId(String key) { |
There was a problem hiding this comment.
normalizedConversationId only trims — the caller-supplied sessionKey (from POST /chat/stream, POST /chat/send and GET /chat/session) is passed unchanged into Peer.thread(conversationId), and MsgContext.canonicalKey() concatenates it verbatim (...|t:<id>|x:agentId=<agent>). Because SessionController.extractGatewayAgentId() takes the first |x:agentId= occurrence, a conversation id containing the delimiter forges that segment: e.g. sessionKey = "foo|x:agentId=other" yields a session whose gateKey is authorised as agent other, so requireSession("other", "foo|x:agentId=other") passes and reset / delete / turns operate on another agent's session. Even without multiple agents, |t: / |r: / |x: in the id lets one conversation's routing key collide with another's. Suggest validating at this single entry point — e.g. reject anything not matching ^[A-Za-z0-9_-]{1,64}$ with a 400 — and adding a regression test for the delimiter case.
| return gateKey != null && gatewayAgentId.equals(extractGatewayAgentId(gateKey)); | ||
| } | ||
| return e.gateKey() == null || Objects.equals(e.gateKey(), expectedGateKey); | ||
| return gateKey == null || gatewayAgentId.equals(extractGatewayAgentId(gateKey)); |
There was a problem hiding this comment.
Loosening MAIN authorisation from "gateKey equals the expected chat gate key" to "the |x:agentId= segment equals the expected agent" is required by the new |t:<conversationId> component, but it also makes any session of that agent addressable by its conversation id, and combined with the unvalidated id above the compared value is itself derived from user input. If the chat UI is ever exposed beyond localhost, please add a real owner dimension (the key already carries r:__anonymous__; a per-user value would restore the previous exact-match guarantee).
| return switch (cmd) { | ||
| case "/new", "/reset" -> { | ||
| String gateKey = resolveGateKey(agentId); | ||
| case "/new" -> |
There was a problem hiding this comment.
/new returns a freshly minted UUID as newSessionKey, but the frontend never consumes the sessionKey field of the done frame (ChatPanel only appends tokens), so typing /new keeps writing into the current conversation while the javadoc promises "mints a fresh conversation id". Either have ChatPanel adopt the returned key on done, or drop /new and point users at the New chat button.
93270bf to
d860144
Compare
Thanks for the review. Three points:
|
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Gives the claw2 example app per-conversation chats: a conversation id now rides on a Peer.thread(...) so dmScope=MAIN still produces a distinct |t: routing key per conversation, /new mints a fresh id, /reset clears the resolved session, and opening a session from the inbox restores its history. The frontend mints/persists an id and the controller echoes a stable echoKey back instead of re-deriving a storage key at response time.
The part worth calling out positively: requireSafeConversationId rejects anything outside [A-Za-z0-9_-]{1,64}, and the tests assert specifically that foo|t:bar and foo|x:agentId=other are refused. A caller-controlled string that is concatenated into a routing key is a forgery surface — being able to inject |x:agentId=... would let a request claim a different agent's session lane — so validating at the boundary rather than relying on the router to survive it is the right instinct, and covering the forgery cases (not just the happy path) is what makes it stick.
The legacy escape hatch is also handled carefully: an id that already resolves to a real stored session keeps the old direct-peer routing so existing chats are not orphaned, and the comment states explicitly that such keys never reach the pattern check. That ordering is what makes the change backward compatible, and it is easy to get wrong.
Notes
ChatResponsenow carriesidentity.echoKey()rather thanfindSessionKeyByGate(gateKey), which is the correct source of truth for a thread-routed conversation (the gate key is not a storage key), but it does mean the two are no longer the same value on the legacy path — worth a sentence in the PR description for anyone reading the wire contract./newdeliberately only mints an id and defers the actual fresh session to the next message ("Your next message opens a new chat"), which matches the frontend's state handling but is a behaviour change from a command that previously resolved a session eagerly. The user-visible message is honest about it, which is the main thing.CommandResultgainingnewSessionKeyis a clean way to get the minted id back to the client without a second round trip.
Example-app scope, no core or extension surface touched, so no cascade risk. CI green, CLA signed, and this is a genuinely well-guarded change.
Automated review by github-manager-bot
AgentScope-Java Version
2.0.3-SNAPSHOT
Background
Fixes #1626
After Claw (Paw) is launched locally, the Sessions list only ever shows one conversation. Starting a new chat keeps writing into the same session instead of adding a new row.
Clicking that session also fails to restore the transcript: the Chat panel stays empty, and the session detail page shows “No turns recorded”, even though the jsonl files on disk are complete. After a later routing fix, the Sessions row preview could show the last message, but clicking the row still did not fill the Chat panel.


When clicking

sessions, only one session is visible. After entering it, no historical content is rendered and the session appears empty:Expected: one agent can keep multiple independent chats; opening a session should show the same history as the files on disk.
Root causes
All chats collapsed into one session
The frontend already mints a unique conversation id, but the backend did not put it into the gateway routing key. Chat UI treats every private message as one MAIN session, so the inbox can only return a single row. “New chat” also sent
/reset, which cleared the current session instead of creating a new one.History file path mismatch
Writes store jsonl under a path prefixed by
sessionId. Reads used an empty runtime context and looked in the unprefixed path, so the UI thought there were no turns.Chat restore was blocked by a false “session missing” check
Inbox preview reads jsonl directly (so the row can show the last message). The Chat panel first asked “does this session exist?” by recomputing the routing key. That check often returned false, so Chat never fetched turns.
Repair ideas
Align with the historical session rendering approach in
examples/dataagent#3095:|t:xxx) so each Chat tab maps to its own backend session./resetis only for clearing the current chat.sessionIdused on write.after fix:
Changes
ChatControllerbuildConversationInbound); echo the conversation id (not the internal storage key)SessionControllerconversationId; read jsonl withsessionIdChatPanel/SessionInboxList/ chat & sessions APIsChatControllerConversationRoutingTest|t:SessionControllerHistoryPathTestsessionIdBuilt frontend assets (
index.html,static/assets/*.js) are not in this PR. Reviewers must rebuild the frontend (see below) to see the new UI. Java-only restart still serves the old Chat page.How to test
Run unit tests:
mvn -pl agentscope-examples/agents/agentscope-paw -am \ test -Dtest=ChatControllerConversationRoutingTest,SessionControllerHistoryPathTest \ -Dskip.installnodenpm=true -Dskip.npm=trueExpected:
BUILD SUCCESS.Rebuild frontend, then start Paw:
Do not skip the npm plugin. IDEA “Run Java only” will not pick up the Chat/New-chat UI fixes.
Manual verification
DASHSCOPE_API_KEY, start Paw (http://localhost:8090/), hard-refresh the browserChecklist
mvn spotless:apply(如需要)ChatControllerConversationRoutingTest、SessionControllerHistoryPathTest,BUILD SUCCESS)docs/codeMonkey/specs/2026-09-11--15-claw多会话覆盖与详情为空修复方案.md)