Skip to content

perf(adapters): use native SHA-256 for tool identity digests - #7670

Merged
lidge-jun merged 11 commits into
lidge-jun:devfrom
luvs01:perf/bun144-tool-identity-hashes
Oct 25, 2026
Merged

lidge-jun merged 11 commits into
lidge-jun:devfrom
luvs01:perf/bun144-tool-identity-hashes

Conversation

@luvs01

@luvs01 luvs01 commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Tool identity normalization constructs a streaming Node-compatible hash object even when the complete identifier is already available. Use Bun.SHA256.hash(input, "hex") for the four request-scoped Anthropic, Google, Kiro and Muse suffix calculations.

Preserve SHA-256, UTF-8 encoding, eight lowercase hexadecimal suffix characters, conforming-name fast paths, truncation, collision salts and restoration. Kiro's machine fingerprint and authentication/HMAC helpers are unchanged.

Add known-answer vectors, lone-surrogate and real UTF-8 collision regressions, with the required layout registration and Structure invariant.

Verification

  • Current head 1505798acd4e936415bdb0cbcc06ca9dfa34e3a4 normally merges the dev baseline and the shared chore(test): align Bun runner with the 1.4.4 security baseline #7627 transition. Runtime and test runner both use Bun 1.4.4. The dev comparison contains eight hashing/registration/documentation files plus eleven shared transition files.
  • Official native Windows Bun 1.4.4, CI=true bun test --isolate --timeout 60000, fresh processes for the tool-identity, Anthropic tool-call ID, Google wire-compiler, Muse alias and Kiro adapter files: 143 passed / 0 failed / 823 assertions.
  • Layout and oracle checks add 3 passed / 5 assertions, for 146 passed / 0 failed / 0 skipped / 828 assertions. Reference TypeScript, official structure and skill checks, and nineteen-file privacy/size/diff checks passed. All eleven units returned native exit 0 and complete child output; owned processes and fixtures were cleaned up.
  • The conflict resolution preserves both this PR's SHA-256 inventory entry and dev's AWS eventstream CRC32 entry. Independent source review verified the registration union, preserved implementation/test blobs and dependency inputs. Matching installed dependencies were reused without reinstalling.
  • Earlier independent review checked known SHA-256 answers, UTF-8 replacement behavior, collision resolution and restoration. Historical Linux/Bun 1.4.4 measurements reported 1.36–1.53× faster mixed-catalog normalization with an approximately unchanged conforming-name control. Those are request-scoped measurements, not current-head throughput, model-latency or process-memory claims.
  • Earlier broad test:changed coverage was not fully green: 52 of 60 failures reproduced on the untouched base, and the remaining eight selected cases had identical per-case outcomes on both trees, including three timeouts. That remains historical comparison evidence, not a full-suite success.
  • PR CI on this exact head completed SUCCESS: 17 successful jobs and 6 conditional skips, including the aggregate ci check. The skipped jobs do not establish native Windows/macOS matrix execution.
  • Focused local checks used the shared-host resource exception to avoid repeating unrelated suites. Live-provider behavior and the complete native-platform matrix remain unverified. No live installation, account or setting changed.
  • This is ready for review, not merge approval. Remaining enforce-target and metadata checks, maintainer review and any required security acceptance are tracked separately before merge. If the still-unmerged chore(test): align Bun runner with the 1.4.4 security baseline #7627 runner transition is introduced ahead of its parent, its exact-candidate full cross-platform promotion proof remains required; the PR CI result above does not supply that proof.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Summary by CodeRabbit

  • Improvements
    • Tool identity handling remains consistent across Anthropic, Google, Kiro, and Muse integrations for punctuation, Unicode, long names, and naming collisions. Rewritten tool names and call IDs remain predictable, and original names can be restored.
    • Distinct names with equivalent encoded bytes remain distinguishable within request-scoped identity handling, with coverage for these cases across the integrations.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 41d5662 and ec56abd.

Files selected for processing: 1
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

Four tool-identity hash call sites now use Bun’s SHA-256 API instead of Node’s createHash. Hash inputs and digest-prefix lengths remain unchanged. New tests cover generated names, collisions, aliases, and restoration.

Changes: Tool identity digests

Layer / File(s) Summary
Bun SHA-256 call sites
src/adapters/google-wire-compiler.ts, src/adapters/kiro-wire.ts, src/adapters/tool-call-id.ts, src/responses/muse-tool-name-alias.ts
The four call sites use Bun.SHA256.hash instead of Node’s createHash. Hash inputs and digest-prefix lengths remain unchanged.
Identity compatibility coverage
tests/adapters/tool-identity-hash.test.ts, tests/fixtures/test-layout-expected.json, scripts/test-layout/layout-adapters.json, structure/transports/inventory.md
Tests specify wire-name vectors and check collisions, aliases, and restoration across adapters. Test-layout mappings include the new test, and the transport inventory describes digest behavior.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

  • lidge-jun/opencodex#7606: Updates the shipped Bun runtime to 1.4.4, which provides the runtime API used by these hash call sites.
Merge Risk: Minimal · up to ec56a

The change presents no established merge-blocking risk under the supported Bun runtime contract.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "perf(adapters): use native SHA-256 for tool identity digests" matches the main change. Four adapter and response modules replace node:crypto createHash with Bun.SHA256.hash(input, "hex") fo…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "perf(adapters): use native SHA-256 for tool identity digests" matches the main change. Four adapter and response modules replace node:crypto createHash with Bun.SHA256.hash(input, "hex") for tool-name and tool-call-id suffix digests. The PR objectives confirm the same scope: Anthropic, Google, Kiro, and Muse suffix calculations. The title is concise, specific, and follows a conventional-commit style. It does not describe the added tests or layout registration, which is expected for a title.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 9 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Oct 24, 2026
@luvs01
luvs01 marked this pull request as ready for review October 24, 2026 19:28
lidge-jun and others added 2 commits October 25, 2026 12:00
# Conflicts:
#	scripts/test-layout/layout-adapters.json
#	tests/claude-integration/claude-picker-upload.test.ts
@lidge-jun

Copy link
Copy Markdown
Owner

Integration evidence (owner decision decision:b607a677, approved): independent gpt-6.1-sol review PASS at 41d5662; the current head ec56abd only adds a merge of origin/dev (after #7667 touched the transport inventory). Exact-head PR CI run 40170367942 and exact-head ci.yml lane=all run 40170390637 both succeeded.

Static merge simulation on current dev 4049f40 (this Mac): dev=4049f40c48b618d673571764ce292bef365ace8c base=6672cfb4a2b9a1dc72e26ed159c1d70368fe0ca7 pr_files= 8 overlap=[tests/fixtures/test-layout-expected.json ]; merge clean; install exit=0; gui install exit=0; tsc exit=0; tsc doctor exit=0; structure:check exit=0; privacy:scan exit=0; layout+ratchet guards exit=0 54 pass 0 fail ;

Merged by the triage manager as maintainer integration (MAINTAINERS.md, dev only) with green exact-head PR CI, an independent review PASS, and the owner's approval of this bundle.

@lidge-jun
lidge-jun merged commit 9b56622 into lidge-jun:dev Oct 25, 2026
64 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants