Skip to content

fix(desktop): preserve tool details in history#3286

Merged
esengine merged 1 commit into
main-v2from
codex/fix-history-tool-details
Jun 6, 2026
Merged

fix(desktop): preserve tool details in history#3286
esengine merged 1 commit into
main-v2from
codex/fix-history-tool-details

Conversation

@SivanCola

Copy link
Copy Markdown
Collaborator

Summary

  • preserve assistant tool calls and tool-result linkage in desktop history payloads
  • rebuild historical ToolCard entries for desktop History, Preview, and Resume flows
  • preserve and replay tool cards from reasonix serve /history

Related

Verification

  • go test ./internal/serve -count=1
  • go test . -run 'TestHistoryMessages|TestPreviewSessionMessages' -count=1 from desktop/\n- git diff --check\n\nNote: frontend npm run typecheck is blocked in this worktree by missing generated Wails bindings (wailsjs/go/main/App and wailsjs/runtime/runtime), unrelated to this change.

@SivanCola SivanCola requested a review from esengine as a code owner June 5, 2026 15:56
@github-actions github-actions Bot added the v2 Go rewrite (1.x) — main-v2 branch, active development label Jun 5, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03ffefa688

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +148 to +149
const result = resultByID.get(tc.id);
if (tc.id) consumedToolIDs.add(tc.id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Pair historical tool results by turn

This uses one global toolCallId -> result map for the entire session, so histories with repeated call IDs render the wrong result. Repeated IDs are possible across separate model responses (for example OpenAI-compatible streams that omit IDs are synthesized as call_0 per response), and in that case every later call_0 card gets the first result while the later tool result is treated as consumed. Reopening or previewing such a session shows stale tool output; pair only the tool messages adjacent to each assistant turn, or otherwise make the lookup position-aware.

Useful? React with 👍 / 👎.

Comment thread internal/serve/index.html
Comment on lines +670 to +671
const result=resultById.get(tc.id);
if(tc.id)consumed.add(tc.id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid globally matching duplicate tool call IDs

The standalone history renderer has the same session-wide toolCallId lookup, which mis-renders sessions where separate turns reuse the same tool call ID. With repeated IDs such as synthesized call_0, this line pulls the first result for later tool cards and consumed then suppresses their actual result messages, so /serve history displays incorrect tool output after resume/reload. The match needs to be limited to the tool results belonging to the current assistant turn.

Useful? React with 👍 / 👎.

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

Labels

v2 Go rewrite (1.x) — main-v2 branch, active development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: 切换会话后工具调用详细信息丢失

2 participants