fix(chat): preserve history cache correlation - #1130
Conversation
|
Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 12:04 AM ET / 04:04 UTC. ClawSweeper reviewWhat this changesThe PR changes Windows native chat-history replay so cached tool metadata cannot make a flattened tool-output row correlate with a structured tool call. Merge readinessKeep open for a maintainer merge decision. Current main still lets flattened history reuse cached IDs, while this head separates synthetic flattened IDs from verified structured IDs and supplies focused regression coverage plus visible isolated-app proof. Priority: P2 Review scores
Verification
How this fits togetherThe Windows tray chat provider combines Gateway history with locally cached tool metadata, then sends the reconstructed timeline to the native WinUI chat renderer. Correct identity matching keeps tool names, states, and output attached to the right historical row. flowchart LR
A[Gateway history] --> B[Chat history provider]
C[Cached tool metadata] --> B
B --> D[ID allocation and cache matching]
D --> E[Timeline reducer]
E --> F[Tool activity projection]
F --> G[WinUI chat history]
Decision needed
Why: The patch appears correct and proof-positive, but persisted-history behavior is compatibility-sensitive and sequencing adjacent work is a maintainer ownership decision. Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the narrow cache-correlation repair while retaining exact keyed matching, synthetic flattened IDs, and the provider-to-UI regression coverage. Do we have a high-confidence way to reproduce the issue? Yes. The added isolated provider/cache fixture gives a high-confidence path that reproduces the structured-ID plus flattened-output collision, though this read-only review did not execute it. Is this the best way to solve the issue? Yes. Separating synthetic flattened IDs from verified structured IDs and matching keyed cache entries exactly is a narrow repair that preserves the existing persisted cache format. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 582a44713860. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (17 earlier review cycles; latest 8 shown)
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
e1be9df to
128bc29
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Related to #894
Scope
This PR contains five focused layers across 11 files:
History/cache correlation
(RunId, ToolCallId, LegacyTurn)identities while preserving reused IDs across distinct runs and legacy turns.Collision-safe history identity allocation
history-tool-*IDs for flattened history and unkeyed structured fallbacks.RunIdandToolCallIdvalues from correlating flattened output to an active structured row.Exact keyed structured-cache matching
ToolCallIdexactly matches using ordinal comparison.Provider-backed production UI proof
OpenClawChatDataProvider.LoadHistoryAsync.IChatGatewayBridgehistory response and the real tool metadata cache path.OPENCLAW_ACCESSIBILITY_TEST_CHAT=1, fixturehistory-collision, and an exact isolatedOPENCLAW_TRAY_DATA_DIRmatch.Regression and provenance coverage
history-tool-0remains interrupted with no output.history-tool-1.clean:tree=<tree>proof provenance while retaining validated dirty descriptors with arbitrary positive file counts.hideFromChannelProgressremains intentionally out of scope.Exact head and base
72f81b76211828cba447148e69b35b98e83afd203a03dfddbed6323fa5cd970f41b7bb0a0ab2ea0cfix(chat): preserve history cache correlation683517894bd13dfeeb9996d3934bdac5b4b4863bfix(chat): avoid synthetic history ID collisions128bc29b3adc7263d0ad60d3ef9af285c49078d9test(chat): prove history ID collision rendering338f67575e259d132f0f1d44d8640e695f245bf2test(chat): route collision proof through history provider72f81b76211828cba447148e69b35b98e83afd20fix(chat): match structured history cache by call ID4712e227d110746400e93107c60616544d68f7b4maintarget:582a44713860409bc34c5acbf6da54a09cf2b10a+1740/-52mainadvanced after the completed rebase. GitHub reports the current PR mergeable; exact-head CI is the source of truth for the target merge result.Validation
git diff --check: passed.\build.ps1: passed, 5 of 5 projectsdotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 3,631 passed, 0 failed, 32 skippeddotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 2,268 passed, 0 failedNativeToolIdentityScreenshotProofTests,HistoryCollisionScreenshotProofTests, andReactorToolActivityProofTests: 3 passed, 0 failed withwin-x64Real behavior proof
The isolated fixture makes one real
RequestChatHistoryAsynccall throughOpenClawChatDataProvider.LoadHistoryAsync. The returned history contains an unfinished structured call with verified IDhistory-tool-0, followed by flattened tool output. The real metadata cache supplies the visible tool identities and safe arguments. The provider's allocator produces exactly two rows:history-tool-0:Interrupted, no output.history-tool-1:Done, ownsflattened output owned by history-tool-1.The screenshot and matching UIA diagnostics were generated from a clean detached checkout of exact product head
72f81b76211828cba447148e69b35b98e83afd20, tree60079b82a853ab063407189c2d062372ac5f06c4. No harness patch was required.8D01DC46D02B2DFD3521FA442AB338C4DCDD90012E9B47B057138144B16F84C6D530A81D33C768672820BE5505A0B612A9E26941BDF93BE4C27AD02B92DAEE3D7343CFE624D263E5A51EC3020AA6319A83F37E8EC2D912D45C3E6BBB4A66551DA9041AACFA915B16A0E0CA30C8D6806C633B59F06EA9E178163D19AB1884C45BD98CB1DA8CC157931AEC424915E86DB180135F0CB86A0C2E2459380DCAF91440All immutable raw URLs returned HTTP 200 with matching hosted bytes. The artifacts contain no blank media, credentials, user paths, real settings, or real Gateway data. No PR was opened from the proof-only branch.
Review
72f81b76211828cba447148e69b35b98e83afd20.PR #1111 interaction
PR #1111 remains complementary and was not modified by this work. Its current head is
919ec7c0c74d2fff572c0405a99a93cf7720a703.Its semantic patch owns persisted call ID aliases, message-level errors, string-encoded safe arguments, and failed-tool grouping. This PR owns positional cache correlation, flattened row identity, synthetic/structured ID collision handling, and exact keyed cache consumption. There is no direct changed-line overlap in the source or tests.
Release boundary
#894 is OPEN and remains open until the follow-up lands and a release includes #1078, #1114, and #1130. This PR does not close the issue and intentionally uses
Related to #894rather than a closing keyword.