feat(openclaw): load chat history on agent chat page#790
Open
shivammittal274 wants to merge 1 commit intodevfrom
Open
feat(openclaw): load chat history on agent chat page#790shivammittal274 wants to merge 1 commit intodevfrom
shivammittal274 wants to merge 1 commit intodevfrom
Conversation
- Add listSessions() and getChatHistory() to OpenClawCliClient - listSessions: runs `sessions --json` (file-based, no gateway auth) - getChatHistory: runs `gateway call chat.history` (gateway WS RPC) - Add GET /claw/agents/:id/history route with server-side filtering of heartbeats, system events, and context-replay extraction - AgentChat loads history on mount, shows user messages and assistant text responses (no tool cards in history, matching Telegram display) - Handles proactive/cron responses without user messages
Contributor
✅ Tests passed — 798/801
|
Contributor
Greptile SummaryThis PR adds chat history loading to the AgentChat page by wiring two new CLI methods ( Confidence Score: 5/5Safe to merge; all remaining findings are P2 style/minor-logic suggestions that do not block correctness on the primary path. No P0 or P1 defects found. The three P2 findings (session sort key, limit=0 edge case, missing assertGatewayReady comment) are quality improvements that don't affect the happy path or cause data loss. AgentChat.tsx (sort key fix) and openclaw.ts (limit parsing) have the most actionable P2 suggestions. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as AgentChat (React)
participant Hook as useOpenClaw
participant Server as /claw/agents/:id/history
participant Svc as OpenClawService
participant CLI as OpenClawCliClient (podman exec)
UI->>Hook: fetchAgentHistory(agentId)
Hook->>Server: GET /claw/agents/:id/history?limit=10
Server->>Svc: listSessions(agentId)
Svc->>CLI: sessions --json --agent id
CLI-->>Svc: OpenClawSessionEntry[]
Svc-->>Server: filtered & sorted sessions
loop Per session (Promise.all)
Server->>Svc: getChatHistory(sessionKey)
Svc->>CLI: gateway call chat.history --params
CLI-->>Svc: OpenClawChatMessage[]
Svc-->>Server: raw messages
Server->>Server: filterSystemMessages()
end
Server-->>Hook: { entries: ChatHistoryEntry[] }
Hook-->>UI: ChatHistoryEntry[]
UI->>UI: historyToTurns() → setTurns()
UI->>UI: setHistoryLoaded(true) → render history
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/app/agents/AgentChat.tsx
Line: 59-63
Comment:
**Sort key falls back to 0 for all timestamp-less sessions**
Sessions are sorted by `messages[0]?.timestamp ?? 0`, but `timestamp` is an optional field on `ChatHistoryMessage`, so every session without timestamps collapses to the same key and ordering becomes undefined. The session object already carries a reliable `updatedAt` field — prefer that as the sort key.
```suggestion
const sorted = [...entries].sort((a, b) => {
const aTs = a.session.updatedAt ?? a.messages[0]?.timestamp ?? 0
const bTs = b.session.updatedAt ?? b.messages[0]?.timestamp ?? 0
return aTs - bTs
})
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/api/routes/openclaw.ts
Line: 413
Comment:
**`limit=0` silently becomes 10**
`Number('0') || 10` evaluates to `10` because `0` is falsy. A caller explicitly passing `?limit=0` would get 10 sessions back, not 0. Use `Number(...) || 10` only when `NaN` is the expected bad input — otherwise check for `NaN` explicitly.
```suggestion
const limitParam = Number(c.req.query('limit'))
const limit = Number.isNaN(limitParam) || limitParam <= 0 ? 10 : limitParam
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/api/routes/openclaw.ts
Line: 416-420
Comment:
**Redundant filter after agent-scoped `listSessions` call**
`listSessions(id)` already passes `--agent ${id}` to the CLI, so the result should already be scoped to that agent. The subsequent `.filter((s) => s.agentId === id || s.key.includes(...))` is a no-op in the happy path but could silently drop sessions if the CLI returns them with an unexpected `agentId` format. If the filter is meant as a defensive guard, a brief comment would clarify intent; if the CLI guarantee is reliable, the second filter can be removed.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/api/services/openclaw/openclaw-service.ts
Line: 580-583
Comment:
**`listSessions` skips `assertGatewayReady` — intentional but worth a comment**
Every other service method that invokes the CLI client goes through `assertGatewayReady()` (e.g., `listAgents`, `getChatHistory`). The PR description says session listing is "file-based, no gateway auth needed", which would justify skipping the check — but the asymmetry is surprising to future readers. A short inline comment would prevent accidental re-introduction of the guard or its unintentional removal elsewhere.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(openclaw): load chat history on age..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+59
to
+63
| const sorted = [...entries].sort((a, b) => { | ||
| const aTs = a.messages[0]?.timestamp ?? 0 | ||
| const bTs = b.messages[0]?.timestamp ?? 0 | ||
| return aTs - bTs | ||
| }) |
Contributor
There was a problem hiding this comment.
Sessions are sorted by messages[0]?.timestamp ?? 0, but timestamp is an optional field on ChatHistoryMessage, so every session without timestamps collapses to the same key and ordering becomes undefined. The session object already carries a reliable updatedAt field — prefer that as the sort key.
Suggested change
| const sorted = [...entries].sort((a, b) => { | |
| const aTs = a.messages[0]?.timestamp ?? 0 | |
| const bTs = b.messages[0]?.timestamp ?? 0 | |
| return aTs - bTs | |
| }) | |
| const sorted = [...entries].sort((a, b) => { | |
| const aTs = a.session.updatedAt ?? a.messages[0]?.timestamp ?? 0 | |
| const bTs = b.session.updatedAt ?? b.messages[0]?.timestamp ?? 0 | |
| return aTs - bTs | |
| }) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/entrypoints/app/agents/AgentChat.tsx
Line: 59-63
Comment:
**Sort key falls back to 0 for all timestamp-less sessions**
Sessions are sorted by `messages[0]?.timestamp ?? 0`, but `timestamp` is an optional field on `ChatHistoryMessage`, so every session without timestamps collapses to the same key and ordering becomes undefined. The session object already carries a reliable `updatedAt` field — prefer that as the sort key.
```suggestion
const sorted = [...entries].sort((a, b) => {
const aTs = a.session.updatedAt ?? a.messages[0]?.timestamp ?? 0
const bTs = b.session.updatedAt ?? b.messages[0]?.timestamp ?? 0
return aTs - bTs
})
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| .get('/agents/:id/history', async (c) => { | ||
| const { id } = c.req.param() | ||
| const limit = Number(c.req.query('limit')) || 10 |
Contributor
There was a problem hiding this comment.
limit=0 silently becomes 10
Number('0') || 10 evaluates to 10 because 0 is falsy. A caller explicitly passing ?limit=0 would get 10 sessions back, not 0. Use Number(...) || 10 only when NaN is the expected bad input — otherwise check for NaN explicitly.
Suggested change
| const limit = Number(c.req.query('limit')) || 10 | |
| const limitParam = Number(c.req.query('limit')) | |
| const limit = Number.isNaN(limitParam) || limitParam <= 0 ? 10 : limitParam |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/api/routes/openclaw.ts
Line: 413
Comment:
**`limit=0` silently becomes 10**
`Number('0') || 10` evaluates to `10` because `0` is falsy. A caller explicitly passing `?limit=0` would get 10 sessions back, not 0. Use `Number(...) || 10` only when `NaN` is the expected bad input — otherwise check for `NaN` explicitly.
```suggestion
const limitParam = Number(c.req.query('limit'))
const limit = Number.isNaN(limitParam) || limitParam <= 0 ? 10 : limitParam
```
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
listSessions()andgetChatHistory()toOpenClawCliClientusing existingpodman execCLI patternGET /claw/agents/:id/historyroute that fetches sessions + chat history, filters system noise, and returns unified entriesHow it works
listSessions()runssessions --json(file-based, no gateway auth needed)getChatHistory()runsgateway call chat.history(gateway WS RPC via CLI)Known limitations
[Current message - respond to this]marker) — could break if OpenClaw changes the formatchat.historymixes user content with OpenClaw internal messages — long-term solution should track conversations locally in BrowserOS rather than reconstructing from OpenClaw's session formatTest plan
🤖 Generated with Claude Code