Skip to content

fix(example): keep multiple Claw chats and restore history when opening a session - #3105

Open
xy-ygz wants to merge 1 commit into
agentscope-ai:mainfrom
xy-ygz:fix/claw-multi-session-inbox-and-transcript
Open

fix(example): keep multiple Claw chats and restore history when opening a session#3105
xy-ygz wants to merge 1 commit into
agentscope-ai:mainfrom
xy-ygz:fix/claw-multi-session-inbox-and-transcript

Conversation

@xy-ygz

@xy-ygz xy-ygz commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

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.
paw修复前0
paw修复前

When clicking sessions, only one session is visible. After entering it, no historical content is rendered and the session appears empty:
paw修复前2

Expected: one agent can keep multiple independent chats; opening a session should show the same history as the files on disk.

Root causes

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

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

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

  • Put the conversation id on a Thread peer (|t:xxx) so each Chat tab maps to its own backend session.
  • Frontend “New chat” mints a new UUID; /reset is only for clearing the current chat.
  • Read transcripts with the same sessionId used on write.
  • When opening a session from the list, always load turns by conversation id; do not wait on the existence check.
    after fix:
paw修复后1 Historical content can be rendered correctly when opening a past session: paw修复后2

Changes

File Change
ChatController Route by conversation id (buildConversationInbound); echo the conversation id (not the internal storage key)
SessionController Match inbox rows by agent id; return conversationId; read jsonl with sessionId
ChatPanel / SessionInboxList / chat & sessions APIs New chat mints a UUID; clicking a row loads that conversation into Chat
ChatControllerConversationRoutingTest Distinct conversation ids produce distinct routing keys with |t:
SessionControllerHistoryPathTest Agent matching ignores the thread segment; read context carries sessionId

Built 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

  1. Run unit tests:

    mvn -pl agentscope-examples/agents/agentscope-paw -am \
      test -Dtest=ChatControllerConversationRoutingTest,SessionControllerHistoryPathTest \
      -Dskip.installnodenpm=true -Dskip.npm=true

    Expected: BUILD SUCCESS.

  2. Rebuild frontend, then start Paw:

    mvn -pl agentscope-examples/agents/agentscope-paw -am package -DskipTests

    Do not skip the npm plugin. IDEA “Run Java only” will not pick up the Chat/New-chat UI fixes.

  3. Manual verification

    • Set DASHSCOPE_API_KEY, start Paw (http://localhost:8090/), hard-refresh the browser
    • Send messages in Session A → Sessions shows 1 row with a preview
    • Click New chat, send messages in Session B → Sessions shows 2 rows
    • Click Session A → Chat shows A’s history (not empty)
    • Open “View transcript” for A → full turns, not “No turns recorded”
    • Refresh, then click A and B again → both remain, histories do not mix

Checklist

  • mvn spotless:apply(如需要)
  • 单测通过(ChatControllerConversationRoutingTestSessionControllerHistoryPathTest,BUILD SUCCESS)
  • 设计文档已补充(docs/codeMonkey/specs/2026-09-11--15-claw多会话覆盖与详情为空修复方案.md
  • Ready for review

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@xy-ygz
xy-ygz force-pushed the fix/claw-multi-session-inbox-and-transcript branch 2 times, most recently from ec85e59 to 93270bf Compare September 11, 2026 09:28

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@xy-ygz
xy-ygz force-pushed the fix/claw-multi-session-inbox-and-transcript branch from 93270bf to d860144 Compare September 11, 2026 15:13
@xy-ygz

xy-ygz commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

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.已修复 #2735 问题:通过 |t:<conversationId> 线程将每条 Claw 聊天消息路由到相应的网关会话中;在读取聊天记录时,使用与写入时相同的 sessionId 标识;同时,始终确保 Chat 面板的正常显示。经过彻底的根源分析,新增的三个测试用例也有效覆盖了所有可能的故障场景。

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.请在合并之前修复这个无效的对话 ID:该 ID 会被直接用于生成路由键,而 sessionMatchesAgent 随后会使用该路由键来进行授权。这样一来,路由键中的分隔符就完全由攻击者控制了。

Automated review by github-manager-bot由 github-manager-bot 自动完成审核流程

Thanks for the review. Three points:

  1. Conversation id — agreed, we’ll reject ids that aren’t [A-Za-z0-9_-]{1,64} with 400 so | can’t be injected into the routing key. The exact requireSession("other", "foo|x:agentId=other") path 404s (parser stops at |), but a poisoned id can still list a session under the wrong agent. That’s what the check closes.

  2. Authorizing MAIN sessions by agent id instead of the full gate key — required for multi-chat. Exact key match would collapse the inbox to one row again. Paw is a local, unauthenticated example; a real owner/user dimension is out of scope for this PR.

  3. /new — ChatPanel already takes sessionKey from the done frame. The New chat button is the intended way to start a conversation. Slash /new is leftover; not changing it here.

@oss-maintainer oss-maintainer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • ChatResponse now carries identity.echoKey() rather than findSessionKeyByGate(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.
  • /new deliberately 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.
  • CommandResult gaining newSessionKey is 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

如何加载历史对话?

2 participants