Skip to content

fix(chat): preserve persisted tool activity details - #1111

Open
TheAngryPit wants to merge 2 commits into
openclaw:mainfrom
TheAngryPit:codex/companion-main-20260806
Open

fix(chat): preserve persisted tool activity details#1111
TheAngryPit wants to merge 2 commits into
openclaw:mainfrom
TheAngryPit:codex/companion-main-20260806

Conversation

@TheAngryPit

@TheAngryPit TheAngryPit commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Related: #1092

What Problem This Solves

Fixes an issue where users reopening persisted chat history could lose important tool activity details: failed tool calls could be hidden inside a successful activity group, tool results using callId could lose their correlation, message-level errors could be ignored, and JSON-string-encoded tool arguments could render without their safe display fields.

Why This Change Was Made

The history parser now accepts the persisted aliases and shapes already emitted by the Gateway, while the existing NativeToolProjector remains authoritative for decoding and safe argument projection. Failed tools remain individually visible; only neighboring successful tools are compacted into activity groups. This is a narrow follow-up to the compact tool activity work in #1092.

User Impact

Users can reopen older conversations and still inspect failed tools, correlated results, and safe command arguments without losing the compact presentation for successful activity.

Evidence

  • Failed-first regression tests reproduced all four persisted-history failures before the production change.
  • The focused parser, projector, and presentation tests pass after the fix: 8 passed, 0 failed.
  • Full Shared suite: 3,413 passed, 0 failed, 32 skipped.
  • Full Tray suite: 2,168 passed, 0 failed.
  • ./build.ps1 passed for Shared, CLI, WinNodeCLI, SetupEngine, and WinUI.
  • Scoped OpenClaw Auto-review found no actionable in-scope findings (patch is correct: 0.98).
  • The self-contained win-x64 candidate started successfully in a network-disabled disposable Windows Sandbox and was promoted locally with rollback preserved.
  • A real persisted agent:main:main conversation was reopened on exact head 8d0f5480: the failed Lcm Grep remained an individually inspectable failed row, neighboring successful tools remained grouped, and safe projected arguments were visible for Memory Search, Lcm Grep, Session Search, and Session History.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Validation

  • ./build.ps1 - pass
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore - 3,413 passed, 0 failed, 32 skipped
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore - 2,168 passed, 0 failed
  • Focused persisted-history and projector regressions - 8 passed, 0 failed
  • dotnet publish src/OpenClaw.Tray.WinUI -c Release -r win-x64 --self-contained --no-restore - pass
  • Current GitHub setup-connect job did not reach checkout or execute repository code: GitHub Actions failed while resolving action download metadata with Service Unavailable / Internal Server Error. Contributor rerun is unavailable; a maintainer rerun is needed.

Real Behavior Proof

  • Environment tested: Windows 11 host plus network-disabled Windows Sandbox
  • PR head or commit tested: 8d0f5480
  • Exact steps or command run: full build and test suites, self-contained win-x64 publish, disposable Sandbox launch, guarded local promotion, then reopen and expand a real persisted conversation
  • Evidence after fix: focused parser and presentation tests pass; Sandbox process remained alive; promoted DLL hashes match the reviewed artifact; Gateway connection signals resumed without new error events; persisted activity replay showed an individually visible failed tool alongside grouped successful tools and safe projected arguments
  • Observed result: the corrected build starts, reconnects, preserves existing Gateway/settings/history state, and renders persisted tool activity with the intended failure and projection behavior
  • Screenshot or artifact links verified? (Yes/No/N/A): No public media uploaded; redacted captures were reviewed locally to avoid repository bloat and private-path disclosure
  • Not verified or blocked: GitHub-hosted setup-connect rerun requires maintainer permission because its current infrastructure failure occurred before repository checkout

Security Impact

  • New permissions or capabilities? (Yes/No): No
  • Secrets or tokens handling changed? (Yes/No): No
  • New or changed network calls? (Yes/No): No
  • Command or tool execution surface changed? (Yes/No): No
  • Data access scope changed? (Yes/No): No
  • If any answer is Yes, explain the risk and mitigation: N/A

Compatibility and Migration

  • Backward compatible? (Yes/No): Yes
  • Config or environment changes? (Yes/No): No
  • Migration needed? (Yes/No): No
  • If yes, list the exact upgrade steps: N/A

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. labels Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 8:33 PM ET / August 10, 2026, 00:33 UTC.

ClawSweeper review

What this changes

The PR preserves failed tool rows, result correlation, message-level errors, and safely displayable arguments when persisted chat history is reopened.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open. Current main still omits the persisted callId aliases, message-level error fallback, string-encoded safe argument projection, and error-row grouping behavior; the PR supplies a narrow covered fix with current-head runtime replay proof.

Priority: P2
Reviewed head: 919ec7c0c74d2fff572c0405a99a93cf7720a703

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered persisted-history repair with credible current-head runtime replay proof and no actionable correctness finding.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The final-head comment provides a real disposable Gateway-to-WinUI persisted-history replay, including post-relaunch accessibility diagnostics that show both safe arguments and an error card.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The final-head comment provides a real disposable Gateway-to-WinUI persisted-history replay, including post-relaunch accessibility diagnostics that show both safe arguments and an error card.
Evidence reviewed 7 items Current-main parser gap: The current parser recognizes semantic tool IDs but not the persisted callId alias, and reads an error only from the content item rather than falling back to the enclosing message.
Current-main presentation gap: Current main groups every consecutive tool-call span, including failures, so an error can be folded into a compact activity group.
Current-main argument gap: Historical replay currently accepts only object-valued arguments before applying the existing safe projector, leaving JSON-string-encoded arguments without display fields.
Findings None None.
Security None None.

How this fits together

Gateway chat-history responses are parsed into structured tool calls and results, then replayed into the tray chat timeline. The timeline presentation groups successful activity while rendering failures and allowlisted tool details for operators.

flowchart LR
  A[Persisted Gateway history] --> B[History parser]
  B --> C[Tool call and result records]
  C --> D[Tray timeline replay]
  D --> E[Activity grouping decision]
  E --> F[Inspectable chat rows]
Loading

Before merge

  • Resolve merge risk (P1) - Merging changes how previously stored conversations display failed tools and result associations, so maintainers should preserve the supplied parser, replay, and grouping regressions as the compatibility boundary.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and regression scope production +49/-11, tests +100 across 8 files The patch is narrowly weighted toward regression coverage for persisted-history parsing and presentation behavior.

Merge-risk options

Maintainer options:

  1. Accept the bounded history-display change (recommended)
    Merge after confirming that failed persisted tools should remain standalone while adjacent successful tools retain compact grouping.

Technical review

Best possible solution:

Land the focused parser, safe-projection, and presentation changes so old Gateway transcripts retain truthful, inspectable tool outcomes without exposing arbitrary argument payloads.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main's parser and replay paths omit the exact aliases and string-argument form covered by this PR, and the contributor supplied a final-head persisted Gateway-to-WinUI replay.

Is this the best way to solve the issue?

Yes. The patch extends the existing parser and authoritative safe projector rather than adding a parallel history path, with focused coverage for each persisted shape.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 582a44713860.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The final-head comment provides a real disposable Gateway-to-WinUI persisted-history replay, including post-relaunch accessibility diagnostics that show both safe arguments and an error card.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The final-head comment provides a real disposable Gateway-to-WinUI persisted-history replay, including post-relaunch accessibility diagnostics that show both safe arguments and an error card.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove proof: 📸 screenshot: Current real behavior proof evidence kind is live_output.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This is a scoped persisted-chat-history correctness repair with limited blast radius.
  • merge-risk: 🚨 session-state: The patch changes replay and display of stored conversation tool state, including error and correlation outcomes.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The final-head comment provides a real disposable Gateway-to-WinUI persisted-history replay, including post-relaunch accessibility diagnostics that show both safe arguments and an error card.
  • proof: sufficient: Contributor real behavior proof is sufficient. The final-head comment provides a real disposable Gateway-to-WinUI persisted-history replay, including post-relaunch accessibility diagnostics that show both safe arguments and an error card.

Evidence

What I checked:

Likely related people:

  • calebeden: Merged compact-activity work introduced the current activity projection and persisted-history presentation surface. (role: introduced the compact tool-activity feature; confidence: high; commits: aac07abb1fbd; files: src/OpenClaw.Tray.WinUI/Chat/ChatToolActivityPresentation.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)
  • bkudiess: Merged correlation work maintained the shared projector and history-replay boundary, and the open related cache-correlation work is explicitly complementary. (role: recent adjacent correlation contributor; confidence: high; commits: a1d933a3c7b7; files: src/OpenClaw.Tray.WinUI/Chat/NativeToolProjector.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (16 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-08T15:40:09.674Z sha be50098 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-09T00:06:39.874Z sha be50098 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-09T03:16:16.654Z sha be50098 :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-09T09:28:16.297Z sha be50098 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T11:59:03.851Z sha be50098 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T14:51:36.314Z sha be50098 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T18:28:49.245Z sha 919ec7c :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-09T22:27:49.681Z sha 919ec7c :: needs real behavior proof before merge. :: none

@TheAngryPit

TheAngryPit commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head proof (8d0f5480)

Automated

  • Focused parser/projector/presentation regressions: 8 passed, 0 failed.
  • Full Shared suite: 3,413 passed, 0 failed, 32 skipped.
  • Full Tray suite: 2,168 passed, 0 failed.
  • ./build.ps1: passed for Shared, CLI, WinNodeCLI, SetupEngine, and WinUI.
  • Scoped OpenClaw Auto-review: no actionable in-scope findings; patch is correct 0.98.

Disposable Windows behavior

  • Published the self-contained win-x64 artifact from exact head 8d0f5480.
  • Direct Windows Sandbox smoke used networking disabled, clipboard disabled, vGPU disabled, and an isolated data directory.
  • The Companion process remained alive through the bounded startup gate and reported the exact 8d0f5480 product version.

Real persisted-history replay

  • Promoted that reviewed artifact locally with a complete file-level rollback backup.
  • Reopened a real persisted agent:main:main conversation.
  • A failed Lcm Grep rendered as its own inspectable row marked failed.
  • Neighboring successful tools remained compacted into activity groups.
  • Safe projected arguments remained visible for persisted Memory Search, Lcm Grep, Session Search, and Session History entries.
  • Gateway WSL and settings/history state were preserved; the promoted Companion restarted with no new error events.
  • Redacted captures were inspected locally rather than committed to the product repository, avoiding repository bloat and disclosure of unrelated conversation content.

Current CI exception

The current setup-connect check is an infrastructure failure before checkout or repository code execution. The runner repeatedly failed to resolve action download metadata with Service Unavailable / Internal Server Error. Other E2E shards completed successfully. The contributor account cannot rerun repository jobs, so this check needs a maintainer rerun.

@TheAngryPit

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Aug 6, 2026
@TheAngryPit

Copy link
Copy Markdown
Contributor Author

Exact-head persisted-history proof (8d0f5480)

These redacted captures come from the reviewed Companion artifact and a real persisted agent:main:main replay.

proof-failed-tool-and-safe-args proof-persisted-group-safe-projection

The first capture shows the failed Lcm Grep as a standalone failed row while adjacent successful activity remains grouped and safe projected arguments remain inspectable.

The second capture shows the persisted four-tool activity group with safe projected arguments for Lcm Grep, Memory Search, Session Search, and Session History.

No private transcript content, tokens, or filesystem paths are included.

@clawsweeper clawsweeper Bot added proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Aug 7, 2026
@TheAngryPit

Copy link
Copy Markdown
Contributor Author

Current-main reconciliation proof

Exact head: be50098bae12e6291ad60b2196ca50996837f94e
Upstream base: 9f9a8eda6340c7ec6f0acbc410de4f6b7d24a3ce

The branch was merged normally with current main. The only conflict was additive in NativeToolProjectorTests.cs: the PR's safe string-encoded argument projection test was retained alongside upstream #1114's correlation tests. Production files merged automatically. The final PR diff remains limited to the same 8 chat/history files.

Fresh exact-head validation:

  • focused Gateway parser: 1/1 passed
  • focused Tray projection/presentation: 66/66 passed
  • dependency preflight: passed; locked @microsoft/mxc-sdk 0.7.0
  • canonical sequential closeout: Shared 3,632 passed, 32 environment-only skips; Tray 2,249/2,249 passed
  • scoped Auto-review: no actionable findings; patch correct, confidence 0.93
  • git diff --check origin/main...HEAD: clean

The PR is now mergeable and remains draft while exact-head CI and automatic review run. Existing redacted behavior captures remain the supported-path evidence; this reconciliation adds no new architecture, protocol, dependency, or ownership surface.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 8, 2026
@TheAngryPit
TheAngryPit marked this pull request as ready for review August 8, 2026 14:11
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 8, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 9, 2026
@TheAngryPit
TheAngryPit force-pushed the codex/companion-main-20260806 branch from be50098 to 919ec7c Compare August 9, 2026 18:21
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Current-head persisted replay proof (919ec7c0c74d2fff572c0405a99a93cf7720a703)

  • Built the exact clean head with .NET SDK 10.0.302 (--no-restore): 0 warnings, 0 errors.
  • Ran that WinUI build against a real disposable OpenClaw Core Gateway (2026.7.2-beta.6) in an isolated WSL profile, loopback-only, with a disposable paired device. No production Gateway state, operator credentials, model, or provider request was used.
  • Seeded one deterministic persisted conversation through Core's canonical session/transcript store. The real chat.history response contained 6 messages, 2 tool calls, and 2 tool results:
    • read with JSON-string arguments → success
    • exec with object arguments → error
  • In the exact-head WinUI accessibility tree, the operator-visible replay exposed:
    • read · Done, path: proof/success.txt, fixture read completed
    • exec · Error, command: proof-command, fixture command failed as expected
  • Terminated and relaunched the app without rewriting the transcript. After reopening the same conversation, WinUI again reported Connected and reproduced the same two expanded tool cards from persisted chat.history.
  • The source worktree remained clean and git diff --check passed.

Proof level: runtime-proven for persisted Gateway → WinUI replay on the final head. This is deliberately not claimed as live model/tool execution proof.

Redacted capture SHA-256:

  • initial replay: ADC4B60D19CC6A4DF161DF65CBBFDDB665CC3947FCDBDF3F7FDC7C6ABB2459DB
  • post-reopen replay: 966AC36BE69CA8AADCC2F55150A2DF99FF7E19F84D587644E084B2EC3EE552F0

A graphical capture was not substituted: the available shell capture surface returned only the desktop wallpaper, so the proof above uses the real WinUI accessibility diagnostics.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants