Skip to content

fix(harness): make session JSONL offload idempotent#1774

Merged
chickenlj merged 5 commits into
agentscope-ai:mainfrom
guslegend0510:fix/issue-1769-session-jsonl-duplication
Jun 17, 2026
Merged

fix(harness): make session JSONL offload idempotent#1774
chickenlj merged 5 commits into
agentscope-ai:mainfrom
guslegend0510:fix/issue-1769-session-jsonl-duplication

Conversation

@guslegend0510

Copy link
Copy Markdown
Contributor

现象

HarnessAgent 使用 DistributedStore + RemoteFilesystemSpec 时,同一个 session 多轮请求后,agents/<agentId>/sessions/<sessionId>.jsonl 会持续变大,历史消息会被重复写入。

复现步骤

  1. 创建启用了分布式存储的 HarnessAgent
  2. 同一 session 连续发送 2 次及以上消息
  3. 检查 contextFile / logFile / DB 镜像内容

预期行为

  • 已经 offload 过的历史消息不应再次被追加
  • flush() 后的 session JSONL 应保持幂等,不应随请求次数重复膨胀

实际行为

  • 第 2 次及后续请求会把历史消息再次写入文件末尾
  • 文件和 DB 镜像都会随请求次数增长

影响

  • session 文件持续膨胀
  • 后续 load() / syncFromRemote() 成本越来越高
  • 长时间运行的 agent 容易出现存储和性能问题

验收标准

  • 连续多轮请求后,session JSONL 不应重复追加已有历史消息
  • 回归测试应覆盖重复 flush、sync 去重和 DB 镜像一致性

@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@AgentScopeJavaBot AgentScopeJavaBot added bug Something isn't working area/harness agentscope-harness (test/runtime support) labels Jun 16, 2026

@AgentScopeJavaBot AgentScopeJavaBot 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.

🤖 AI Review

This PR fixes a real and impactful bug: repeated offloadMessages() calls for the same session would re-append all historical messages, causing the session JSONL file to grow unboundedly. The fix is clean and minimal — it snapshots existing entry IDs after syncFromRemote(), then skips any incoming Msg whose stable ID is already present. The parent chain (lastId) is correctly initialized from the last existing entry. The regression test validates the core idempotency scenario well. One notable concern: when Msg.getId() is null or blank, dedup is silently bypassed, which could reintroduce the exact duplication bug under certain upstream conditions.

@AgentScopeJavaBot AgentScopeJavaBot 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.

🤖 AI Review

This PR fixes a real and impactful bug: repeated offloadMessages() calls for the same session would re-append all historical messages, causing the session JSONL file to grow unboundedly. The fix is clean and minimal — it snapshots existing entry IDs after syncFromRemote(), then skips any incoming Msg whose stable ID is already present. The parent chain (lastId) is correctly initialized from the last existing entry. The regression test validates the core idempotency scenario well. One notable concern: when Msg.getId() is null or blank, dedup is silently bypassed, which could reintroduce the exact duplication bug under certain upstream conditions.

@guslegend0510

Copy link
Copy Markdown
Contributor Author

Addressed in the latest update on this branch. I also added a regression test for null/blank message IDs and re-ran MemoryFlushManagerOffloadTest.

@AgentScopeJavaBot AgentScopeJavaBot 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.

All previously raised review comments have been addressed.

@chickenlj chickenlj 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.

LGTM.

@chickenlj chickenlj merged commit 9120dc0 into agentscope-ai:main Jun 17, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/harness agentscope-harness (test/runtime support) bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants