Skip to content

[WEB-5152] Always send a stable tool_use_id from every hook#220

Merged
pugazhendhi-m merged 5 commits into
stagingfrom
WEB-5152
Jul 23, 2026
Merged

[WEB-5152] Always send a stable tool_use_id from every hook#220
pugazhendhi-m merged 5 commits into
stagingfrom
WEB-5152

Conversation

@pugazhendhi-m

@pugazhendhi-m pugazhendhi-m commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

What

Make every tool call carry a tool_use_id so the backend dedups by id (not just content), closing the Copilot/Augment replay-duplication class for good.

tool_use_id = native id OR deterministic synthetic (unb- + sha256(replay-stable content)[:24]), computed independently at the PreToolUse emit and the completion emit so a pre command re-appearing in post derives the same id with no side file / no correlation store. Native always wins.

Per hook

Hook Change
claude-code resolve_tool_use_id() — native tool_use_id on pre+post, synthetic fallback (reference impl)
codex native tool_use_id/transcript call_id wins; _synthetic_tool_use_id(session, turn_id, tool_name, command) fallback
cursor (no native id) synthetic keyed on conversation_id + generation_id + raw tool_name + content; MCP keyed on sort_keys tool_input (server command excluded so pre==post); added to the shell/MCP pre path (previously sent none) + all completion branches
copilot forwards the native transcript toolCallId on completion (was dropped); no synthetic on pre by design — a minted pre id can't equal the transcript id and would fork the row (pretool row already dedups on content/request_id)
augment (no native id, no turn id) synthetic keyed on conversation_id + tool_name + command

Why it's safe

The backend keys on id AND content and pairs pretool↔completion on content, so the synthetic id is a strict refinement of the existing key — it can only split rows, never over-collapse — and pre/post ids don't even need to match for correctness (replay-dedup is completion-vs-completion with a deterministic id). Fail-open preserved (claude/codex main() wrap everything; cursor/augment helpers have their own try/except). Synthetic id is 28 chars → fits the JSON parameters field; no truncation.

Tests

binary/tests/test_tool_use_id_parity.py (5 tests, passing) — determinism + pre == post + native precedence, run against the real hook code. All five hooks py_compile clean.

Adversarial 4-lens review (pre/post divergence, synthetic collapse, WARN/BLOCK/AUDIT row count, rollout compat/crash) → all clear.

Notes

  • Version bumped 0.1.12 → 0.1.13 (binary/src/unbound_hook/__init__.py + packaging/unbound_discovery_entry.py, lockstep).
  • Known limitation (by design): augment has no turn id, so two genuine reads of the same unchanged file across turns of one conversation collapse to one row — mirrors the accepted terminal/MCP content-dedup tradeoff, net better than augment's history-resend over-count.
  • Backward compatible with the installed fleet (a new hook version only takes effect in a new session = new dedup scope). Backend companion (tests): ai-gateway-data#WEB-5152. ai-gateway needs no change (pure pass-through).

🤖 Generated with Claude Code

https://claude.ai/code/session_01GodcEwLB6rJhkXEiNYWj6u


Note

Medium Risk
Changes audit/dedup identity across all agent hooks; Cursor file ids key on path+generation (repeated edits to the same file in one generation can collide), but behavior is fail-open and native ids still win.

Overview
Hooks now attach a tool_use_id on every pretool and completion payload so the gateway can dedupe replays by id, not only by content. The rule is native id when present, otherwise a deterministic unb- + SHA-256 of replay-stable fields (session/conversation, tool name, canonicalized command/path).

Claude Code adds resolve_tool_use_id() and always sets it on PreToolUse and PostToolUse. Codex mints synthetics from session + turn + tool + command (including apply_patch body so patches don’t all share one id) on pretool and transcript-derived completions. Cursor adds _resolve_tool_use_id() with file-op normalization so pre vs after* events for the same path match; shell/MCP pretool paths that previously omitted ids now send them, and completion branches attach the same resolver. Augment adds _resolve_tool_use_id() and extends path extraction via file_changes[0].path so pre/post file edits hash consistently. Copilot only forwards the transcript toolCallId on completion—no synthetic pretool id by design.

New binary/tests/test_tool_use_id_parity.py asserts determinism, pre==post parity, and native precedence across hooks. Packaged hook/discovery version 0.1.12 → 0.1.13.

Reviewed by Cursor Bugbot for commit b79d8bf. Bugbot is set up for automated code reviews on this repo. Configure here.

Greptile Summary

This PR adds stable tool-call identifiers across the supported hooks. The main changes are:

  • Native IDs are preserved when available.
  • Deterministic synthetic IDs are added for Claude Code, Codex, Cursor, and Augment.
  • Copilot completion events now forward transcript call IDs.
  • Hook parity tests cover determinism, event matching, and native precedence.
  • The packaged hook version is bumped to 0.1.13.

Confidence Score: 5/5

The latest fixes look safe to merge.

  • The Cursor file-event shapes now normalize to matching operation and path values.
  • Claude synthetic IDs no longer depend on a prompt field that may be absent from completion events.
  • Tests cover the updated parity behavior and native-ID precedence.
  • No remaining issue qualifies for a new follow-up comment.

Important Files Changed

Filename Overview
cursor/unbound.py Normalizes Cursor file-event shapes and attaches stable IDs to pre-tool and completion payloads.
claude-code/hooks/unbound.py Adds native-first synthetic ID resolution without relying on the optional prompt identifier.
codex/hooks/unbound.py Adds deterministic fallback IDs for pre-tool and transcript completion records.
augment/hooks/unbound.py Adds synthetic IDs and aligns file paths across pre-tool and replayed completion events.
copilot/hooks/unbound.py Forwards native transcript call IDs on completion records.
binary/tests/test_tool_use_id_parity.py Adds cross-hook tests for deterministic IDs, event parity, and native precedence.

Reviews (5): Last reviewed commit: "[WEB-5152] Guard augment pretool tool_us..." | Re-trigger Greptile

Context used:

  • Rule used - Ensure that the confidence score is always within ... (source)

Learned From
websentry-ai/ai-gateway-data#448

Make every tool call carry a tool_use_id so the backend dedups by id, closing
the Copilot/Augment replay-duplication class for good.

tool_use_id = native id OR a deterministic synthetic 'unb-'+sha256(replay-stable
content)[:24], computed independently at the PreToolUse and completion emits so a
pre command re-appearing in post derives the SAME id with no side file. Native
always wins.

- claude-code: resolve_tool_use_id() (native on pre+post, synthetic fallback).
- codex: native tool_use_id / transcript call_id wins; synthetic fallback for
  tools with no native id.
- cursor (no native id): synthetic keyed on conversation_id + generation_id +
  raw tool_name + content (MCP keyed on sort_keys tool_input, server command
  excluded so pre==post); added to the shell/MCP pre path + all completion branches.
- copilot: forward the native transcript toolCallId on completion (was dropped);
  no synthetic on pre (would diverge from the transcript id and fork the row).
- augment (no native id, no turn id): synthetic keyed on conversation_id +
  tool_name + command.

Fail-open preserved; synthetic id is 28 chars (fits the JSON parameters field).
Version 0.1.13 (lockstep). Adds binary/tests/test_tool_use_id_parity.py proving
determinism + pre==post + native precedence across all five hooks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GodcEwLB6rJhkXEiNYWj6u
@pugazhendhi-m
pugazhendhi-m requested a review from a team July 16, 2026 08:02
Comment thread cursor/unbound.py
Comment thread claude-code/hooks/unbound.py Outdated
Comment thread augment/hooks/unbound.py
Comment thread augment/hooks/unbound.py

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Lead, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found. (reviewers: Lead, Claude, Semgrep, Gitleaks)

Previously acknowledged (not re-flagged)

  • Content-keyed synthetic IDs collapse repeated identical calls (e.g. Augment, no turn id) — accepted by design in the PR description as a known limitation mirroring existing terminal/MCP content-dedup tradeoff.
  • Pre/post tool_use_id mismatch is non-blocking — backend pairs pretool↔completion on content; PR states matching pre/post ids are not required for dedup correctness.
  • Copilot omits synthetic pre-tool id — intentional to avoid forking pretool rows against native transcript toolCallId (PR design note).
  • Semgrep insecure-file-permissions / sqlalchemy-execute-raw-query hits — pre-existing patterns on unchanged lines outside this diff; not introduced by WEB-5152.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head f6d67eba · 2026-07-16T08:15Z

Address Greptile/Cursor Bugbot review on #220:

- cursor: afterFileEdit/beforeReadFile carry no command/tool_input, so the id
  hashed empty content and every file op collapsed onto one id. Key file events
  on file_path (+edits) so each is distinct and replay-stable.
- augment: extract_command_for_pretool now falls back to file_changes[0].path
  (as the completion exchange does), so a pre event (path in tool_input) and its
  completion (path in file_changes) hash to the same id.
- claude-code: drop prompt_id from the synthetic key -- it is not guaranteed on
  PostToolUse and would fork the id; session + tool + command are on both events.
- claude-code / codex / augment: canonicalize MCP tool_input (json round-trip
  with sort_keys) so key-order variance can't diverge the pre id from the post id.

Parity test extended: MCP key-order independence, prompt_id-omitted parity,
cursor file-event determinism, augment file_changes fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GodcEwLB6rJhkXEiNYWj6u
Comment thread cursor/unbound.py Outdated
Comment thread cursor/unbound.py Outdated
Comment thread cursor/unbound.py
Comment thread augment/hooks/unbound.py Outdated

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🛡️ Automated Security Review (consensus)

2 findings — 0 high-confidence, 2 to triage. Reviewers: Claude, Semgrep, Gitleaks.

🟡 TRIAGE — Unguarded synthetic tool_use_id on audit/completion paths

claude-code/hooks/unbound.py:545, claude-code/hooks/unbound.py:1553, codex/hooks/unbound.py:1239

Impact: New resolve_tool_use_id / _synthetic_tool_use_id calls on completion/stop paths lack the per-call try/except used in cursor/augment; a malformed event (e.g. non-dict tool_input) can raise and cause the top-level fail-open handler to drop the entire turn's audit exchange—not just the id.

Fix: Wrap synthetic-id resolution in a local try/except that returns native id or None on failure, matching cursor/augment's fail-open resolver pattern.

Flagged by: Claude


🟡 TRIAGE — Augment pretool may emit explicit tool_use_id: null

augment/hooks/unbound.py:1273

Impact: process_pre_tool_use now unconditionally assigns _resolve_tool_use_id(event); on resolver failure the except branch returns native (None), sending "tool_use_id": null where the field was previously omitted—if the backend rejects null, the pretool check errors and the hook fails open, potentially skipping BLOCK evaluation.

Fix: Restore the truthiness guard used in cursor (_tuid = _resolve_tool_use_id(event); if _tuid: request_body['pre_tool_use_data']['tool_use_id'] = _tuid).

Flagged by: Claude


Previously acknowledged (not re-flagged)

  • Cursor afterFileEdit id parity — Fixed in a8a5c4b: file events now key on file_path (+ edits); parity test added.
  • Claude prompt_id synthetic-key fork — Fixed in a8a5c4b: prompt_id dropped from the synthetic key; test covers PostToolUse omitting it.
  • Augment pre/post file path mismatch — Fixed in a8a5c4b: extract_command_for_pretool falls back to file_changes[0].path; parity test added.
  • Augment MCP dict key-order nondeterminism — Fixed in a8a5c4b: MCP tool_input canonicalized with sort_keys=True before hashing.
  • Augment synthetic key has no turn id (cross-turn collapse) — Known limitation by design (mirrors accepted terminal/MCP content-dedup tradeoff; net better than history-resend over-count).

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a8a5c4bf · 2026-07-16T13:25Z

Greptile follow-up on #220: afterFileEdit was deterministic but still didn't match
the corresponding preToolUse Write/Edit (different event shapes -> different ids).
Normalize every cursor file-event shape (preToolUse Write/Read/Delete, afterFileEdit,
beforeReadFile) to (operation, path) so a pre event and its completion for the same
file hash to the same synthetic id. Distinct files / distinct ops stay distinct.

Test pairs a Write pre-event with afterFileEdit (and Read pre with beforeReadFile)
and asserts equal ids.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GodcEwLB6rJhkXEiNYWj6u

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🛡️ Automated Security Review (consensus)

2 findings — 0 high-confidence, 2 to triage. Reviewers: Lead, Claude, Semgrep, Gitleaks, Cursor.

🟡 TRIAGE

Read completions omit tool_use_id

cursor/unbound.py:1174
Impact: beforeReadFile completions still omit tool_use_id while pretool Read events mint one, leaving read replay dedup weaker than shell/MCP/write paths.
Fix: Add 'tool_use_id': _resolve_tool_use_id(event) to the beforeReadFile branch in build_llm_exchange (parity test already covers the resolver for Read↔beforeReadFile).
Reviewers: Cursor

Augment pretool may emit null tool_use_id

augment/hooks/unbound.py:1273
Impact: process_pre_tool_use always sets tool_use_id; the resolver’s fail-open path returns native (often None), so synthesis failures can ship an explicit null id instead of a stable synthetic value.
Fix: Only attach the field when _resolve_tool_use_id returns non-None, or add a last-resort synthetic fallback before emitting.
Reviewers: Cursor

Previously acknowledged (not re-flagged)

  • Augment same-file reads across turns collapse to one row — known limitation by design (no turn id; accepted content-dedup tradeoff per PR description).
  • Cursor/Claude/Augment pre↔post tool_use_id parity gaps (file-op shape mismatch, prompt_id in synthetic key, Augment file_changes path, MCP key-order) — fixed in a8a5c4b / ac1ac96 per maintainer.
  • Semgrep insecure-file-permissions / sqlalchemy-execute-raw-query hits — pre-existing hook install/SQL paths outside this PR’s tool_use_id hunks; no new exposure introduced by the diff.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head ac1ac96e · 2026-07-16T13:33Z

cursor: the beforeReadFile completion branch appended a tool_use item
without tool_use_id (the other four branches set it), so file-read rows
carried no id and could not dedup by id. Wire _resolve_tool_use_id(event).

codex: extract_command_for_pretool had no apply_patch branch, so its
synthetic key collapsed to ("apply_patch") for every patch in a turn.
Key on the patch/diff content so each call gets a distinct synthetic id.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GodcEwLB6rJhkXEiNYWj6u
Comment thread cursor/unbound.py
Comment on lines +694 to +695
if file_op and file_path:
tool_disc, content = 'file', file_op + ':' + str(file_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Repeated edits share IDs — The key contains only the operation and path after the conversation and generation fields. When one generation writes or edits the same file twice, both real calls and their completion events receive the same synthetic ID. The backend can then treat the second edit as a replay and omit its audit record. Add a replay-stable occurrence value that is present in both event shapes while preserving pre/post parity.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not a real collapse: the backend dedups by id-AND-content (sha256("id\x1f{tuid}\x1f{content}"), where content includes the full edits/params), not by id alone. Two different edits of the same file in one generation share the synthetic id but have different content → different dedup key → separate rows. Only byte-identical edits collapse, which is the intended no-op dedup and matches how identical calls are treated everywhere.

The id is keyed on (op, path) deliberately so the pretool Write event and the completion afterFileEdit for the same call hash to the same id and correlate; folding the edit content into the id would fork pre↔post and defeat that correlation. So the synthetic id is a strict refinement of the existing content key — it can only split rows further, never over-collapse.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're right, and I was wrong. My concern assumed the backend dedups on id alone, but the composite key sha256("id\x1f{tuid}\x1f{content}") means two different edits to the same file share the synthetic id but produce different dedup keys — separate rows. Only byte-identical edits collapse, which is the intended behavior.

The (op, path) keying is also the correct tradeoff: including edit content in the id would break pre↔post correlation since the pretool and completion event shapes carry different content fields.

Shall I remember this as a rule: "When reviewing synthetic tool_use_id key design, account for the backend's id-AND-content composite dedup key — do not flag id-level collisions unless content is also identical"?

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🛡️ Automated Security Review (consensus)

2 findings — 1 high-confidence, 1 to triage. Reviewers: Lead, Claude, Semgrep, Gitleaks.

Findings

1. Augment pre-tool can emit explicit null tool_use_id

augment/hooks/unbound.py:1273 · 🟡 LOW · 🔴 HIGH

Impact: On synthesis failure the resolver returns native (None), but process_pre_tool_use now always assigns the field — unlike Cursor, which guards with if _tuid: — so payloads may ship "tool_use_id": null and the backend may reject or skip policy on that call (fail-open).

Fix: Restore the conditional assignment (if _tuid:) or omit the field when _resolve_tool_use_id returns falsy, matching the Cursor hook pattern.

Flagged by: Lead, Claude, Cursor Bugbot


2. Codex Stop backfill reuses Stop turn_id for every transcript tool use

codex/hooks/unbound.py:1243 · 🟡 MEDIUM · 🟡 TRIAGE

Impact: process_stop_event mints missing completion ids with the Stop event's turn_id for all transcript items; if that differs from each call's pre turn_id, or identical commands appear across turns in one transcript, completions can share a synthetic id and replay dedup may drop a distinct audit row.

Fix: Key each backfilled id on the transcript item's own turn/call identifier, or add a per-transcript occurrence counter when no per-call turn id exists.

Flagged by: Claude


Previously acknowledged (not re-flagged)

  • Same-path repeated file ops in one Cursor generation share one synthetic id — Accepted design: backend dedups on id and content, so identical ids cannot over-collapse distinct edits; mirrors the documented content-dedup tradeoff (Greptile P1 on cursor/unbound.py:695).
  • Augment cross-turn reads of the same unchanged file collapse to one row — Known limitation by design per PR description.
  • Cursor pretool vs afterFileEdit/beforeReadFile shape mismatch — Fixed in ac1ac96 (@pugazhendhi-m): normalized to (operation, path) before hashing.
  • Claude prompt_id in synthetic key / Augment file_changes path / MCP sort_keys canonicalization — Fixed in a8a5c4b (@pugazhendhi-m).
  • Fail-open id resolver error handling — Intentional; hooks must not crash on id synthesis failure.
  • Semgrep insecure-file-permissions and sqlalchemy-execute-raw-query hits — Pre-existing code on unchanged lines; out of scope for this diff.

🤖 consensus review · reviewers: Cursor,Semgrep,Gitleaks · head 9e0d6586 · 2026-07-22T14:32Z

_resolve_tool_use_id's fail-open path returns native (None on augment,
which has no native id). Setting pre_tool_use_data.tool_use_id
unconditionally could therefore ship an explicit `tool_use_id: null` on
a synthesis failure. Guard the assignment (if _tuid:) like the cursor
hook, so a failure omits the field and falls back to content dedup
cleanly instead of writing a null id.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GodcEwLB6rJhkXEiNYWj6u

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b79d8bf. Configure here.

Comment thread cursor/unbound.py
file_op = _CURSOR_FILE_OP.get(tool_name) or _CURSOR_FILE_OP.get(hook_name)
file_path = event.get('file_path') or ti.get('file_path') or ti.get('path')
if file_op and file_path:
tool_disc, content = 'file', file_op + ':' + str(file_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same-path edits share ID

High Severity

The synthetic tool_use_id for file operations is not unique enough. It's derived from conversation_id, generation_id, operation, and file_path, but doesn't include the actual content of the write or edit. This means distinct file modifications to the same path within a generation get the same ID, causing backend deduplication to drop valid events from audit and analytics.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b79d8bf. Configure here.

@vigneshsubbiah16 vigneshsubbiah16 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Lead, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found. (reviewers: Lead, Claude, Semgrep, Gitleaks)

Previously acknowledged (not re-flagged)

  • Same-path repeated edits share synthetic tool_use_id (Cursor file ops) — Maintainer: backend dedups on id AND content (sha256("id\x1f{tuid}\x1f{content}")); distinct edits keep separate rows; only byte-identical replays collapse (intended). Greptile withdrew the concern.
  • Augment cross-turn collapse for identical reads of the same file — Known limitation by design (no turn id); mirrors accepted terminal/MCP content-dedup tradeoff per PR description.
  • Copilot pretool omits synthetic tool_use_id — By design: minted pre id cannot match transcript toolCallId and would fork rows; pretool already dedups on content/request_id.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head b79d8bfe · 2026-07-22T14:41Z

@pugazhendhi-m
pugazhendhi-m merged commit 428a0be into staging Jul 23, 2026
4 checks passed
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.

3 participants