Skip to content

fix(client): keep Grok tool names across ACP tool_call_update patches - #2388

Open
L42y wants to merge 1 commit into
first-tree-ai:mainfrom
L42y:fix/grok-unknown-tool-merge
Open

fix(client): keep Grok tool names across ACP tool_call_update patches#2388
L42y wants to merge 1 commit into
first-tree-ai:mainfrom
L42y:fix/grok-unknown-tool-merge

Conversation

@L42y

@L42y L42y commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Grok ACP tool_call_update is a patch: later completed/failed updates often send only status + rawOutput and omit title / _meta["x.ai/tool"].
  • The Grok adapter treated each payload as a full snapshot and labeled the missing name grok:unknown. The Working lane then kept the latest row per toolUseId, so a nameless completion overwrote a good start name.
  • Merge last-known GrokToolInfo by toolCallId before emitting session events. Keep grok:unknown only when that id has never had a name or title.

This replaces #2387. Same exact head 785dc09b099777d8b54908e0f5712125b9feeb25; the previous PR was blocked because jeremy/grok-unknown-tool-merge is not a required branch prefix.

Validation

  • biome check on the four changed Grok files
  • Independent exact-head review of fix(client): keep Grok tool names across ACP tool_call_update patches #2387: git diff --check, pnpm check, build (5/5), typecheck (9/9), 91 focused Grok tests, full client suite (2,898 passed, 7 skipped)
  • pnpm --filter @first-tree/client exec vitest run src/providers/grok/__tests__/events.test.ts src/providers/grok/__tests__/handler.test.ts (91 passed)

Change Surface

  • apps/cli public CLI or help output
  • tree onboarding / binding / inspection behavior
  • shipped or planned skill topology
  • docs or contributor-facing repository metadata
  • CI / packaging / release plumbing

Notes

  • package or install behavior changes: none
  • docs or tests updated to match: Grok events + handler coverage for title-less completed updates, including preserved shell args/refs
  • follow-up work: optional Working-lane TOOL_VERBS mapping for Grok names (run_terminal_cmd / run_terminal_command) so shells render as run <command> instead of use <title>

ACP completions often omit title and x.ai/tool meta. Merge by toolCallId
so the Working lane does not overwrite a real name with grok:unknown.

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved exact head 785dc09b099777d8b54908e0f5712125b9feeb25.

The replacement PR uses the compliant fix/grok-unknown-tool-merge branch, and its CI run 33833392072 is fully green. The implementation preserves last-known Grok tool metadata across title-less ACP completion patches without weakening the existing replay/effect classification.

Independent validation on this exact head passed: git diff --check, pnpm check, build (5/5), typecheck (9/9), and 91 focused Grok tests. The same exact head also passed the full client suite (2,898 passed, 7 skipped) during the immediately preceding review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants