Issue Station
Sign In
Tools hereToolbox

openclaw - ✅(Solved) Fix sortToolsMessageItems uses toSorted() which is not stable [2 pull requests, 1 comments, 2 participants]

Official PRs (…)
ON THIS PAGE

Recommended Tools

×6

Utilities matched from this issue’s tags and category — try them while you read without losing context.

GitHub issue graph ai analysis

Paste a GitHub issue URL. We fetch that issue, discover linked issues from bodies/comments/timeline, collect linked pull requests, and produce a structured English report.

The report is written in English Markdown for sharing and archival.

Helpful · Quick feedback

Loading…
GitHub stats
openclaw/openclaw#61380•Fetched 2026-04-08 02:59:12
View on GitHub
Comments
1
Participants
2
Timeline
5
Reactions
1
Author
revinwang-gitrevinwang-git
Participants
GuneetGillGuneetGillrevinwang-gitrevinwang-git
Timeline (top)
cross-referenced ×2commented ×1mentioned ×1subscribed ×1

Fix Action

Fixed

PR fix notes

PR #61497: Fix/stable sort tools message

Description (problem / solution / changelog)

Summary

  • Problem: sortToolsMessageItems uses Array.toSorted(), which is not guaranteed to be stable across all runtimes
  • Why it matters: When tool order shifts between runs, every token after the tool list gets a different hash → full Prompt Cache miss for the system prompt, hurting latency and cost
  • What changed: Replaced .toSorted() with .slice().sort() to guarantee stable sort behavior, consistent with the pattern Claude Code already uses internally
  • What did NOT change: Sort comparator logic (localeCompare by name) is identical; no behavior change

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: Array.prototype.toSorted is not guaranteed stable in all environments targeted by OpenClaw (Node.js v22 + embedded bundle). Claude Code's own codebase explicitly avoids it for this reason.
  • Missing detection / guardrail: No lint rule preventing use of toSorted in cache-sensitive paths
  • Contributing context: ES2024 API adoption without checking cache invalidation implications

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: Unit test for sortToolsMessageItems asserting stable output across multiple calls with identical input
  • Scenario the test should lock in: Given the same tool list in varying insertion orders, output order must be identical and stable across repeated sorts
  • Why this is the smallest reliable guardrail: Pure function with deterministic input — no integration surface needed
  • Existing test that already covers this (if any): None found
  • If no new test is added, why not: N/A — test added

User-visible / Behavior Changes

None

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Any
  • Runtime/container: Node.js v22
  • Model/provider: Any
  • Integration/channel (if any): N/A
  • Relevant config (redacted): N/A

Steps

  1. Set up an agent with multiple tools registered
  2. Observe sortToolsMessageItems output across repeated calls with the same input
  3. Check prompt cache hit rate in logs

Expected

  • Tool list order is identical across runs → cache hits on system prompt tokens

Actual

  • Tool list order may vary → cache misses on all tokens following the tool list

Evidence

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

  • Risk: .slice() creates a shallow copy on every call — negligible overhead for typical tool list sizes
    • Mitigation: Tool lists are small (rarely >20 items); no meaningful performance impact

Changed files

PR #61512: fix: more predictable ordering for the “available tools” prompt text

Description (problem / solution / changelog)

Summary

  • Problem: Tool ordering in buildToolsMessage could vary with input array order and with locale-sensitive localeCompare behavior across environments. Same tools could still produce different prompt text.

  • Why it matters: The tools block sits inside a larger prompt. If those bytes drift, you get unnecessary variability and weaker prompt-cache behavior even when nothing “meaningful” about capabilities changed.

  • What changed:

    • Sort a shallow copy with [...items].toSorted(...) so we don’t mutate the caller’s array.
    • Use a locale-independent comparator (UTF-16 string order) inside that sort: 
      function comparePromptStrings(a: string, b: string): number {
  return a < b ? -1 : a > b ? 1 : 0;
}
    • Sort primarily by normalized tool id, with name as a tie-break if ids collide.

  • What did NOT change (scope boundary): No hook schema, gateway wiring, tool registry, or wording of the tools message beyond deterministic ordering of the same tools.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Related #61380 (original report focused on toSorted vs sort; this PR centers on deterministic, id-based ordering + tests.)
  • Related #
  • This PR fixes a bug or regression

Root Cause (if applicable)

  • Root cause: Ordering depended on input order and locale-sensitive string comparison for labels, so prompt text was not stable across environments.
  • Missing detection / guardrail: No unit test that reversed input order must yield identical buildToolsMessage output.
  • Contributing context (if known): N/A

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:

    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient

  • Target test or file: src/auto-reply/status.tools.test.ts

  • Scenario the test locks in:

    • Same two tools in opposite array order → full message string is identical (expect(textA).toEqual(textB)).
    • Compact listing still reflects id order (e.g. contains exec, web_search).

  • Why this is the smallest reliable guardrail: Pure formatting from structured input; asserting full output equality on permuted input directly pins determinism without heavier E2E.

  • New test added: produces deterministic ordering regardless of input order

User-visible / Behavior Changes

None for copy or which tools appear. Ordering of tools in the generated message is now defined by normalized id (stable tie-break on name), independent of how tools were listed in the incoming inventory.

Diagram (if applicable)

N/A

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: any
  • Runtime: Node 22+
  • Model/provider: N/A
  • Integration/channel: N/A
  • Config: N/A

Steps

  1. pnpm test -- src/auto-reply/status.tools.test.ts
  2. (Optional) Skim sortToolsMessageItems in src/auto-reply/status.ts: .sort on [...items], compare by id then name via comparePromptStrings.

Expected

  • All tests pass; determinism test passes (reversed input → same full string).

Evidence

  • Unit tests (status.tools.test.ts)
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Risks and Mitigations

None material. Ordering changes only if it previously followed a different locale or input order; behavior is intentionally standardized to id-based order.

Changed files

  • src/auto-reply/status.tools.test.ts (modified, +46/-0)
  • src/auto-reply/status.ts (modified, +8/-1)

Code Example

function sortToolsMessageItems(items) {
    return items.toSorted((a, b) => a.name.localeCompare(b.name));
}
RAW_BUFFERClick to expand / collapse

*Describe the bug In dist/pi-embedded-BYdcxQ5A.js, sortToolsMessageItems uses toSorted:

function sortToolsMessageItems(items) {
    return items.toSorted((a, b) => a.name.localeCompare(b.name));
}

toSorted (ES2024) is not guaranteed stable. Claude Code explicitly avoids it with this comment:

"Avoid Array.toSorted — toSorted() is unstable and any position change causes all subsequent tokens to have different hashes, invalidating the Prompt Cache."

Impact: When tool order changes between runs, every token after the tool list has a different hash → Prompt Cache miss for the entire system prompt.

Fix: Replace toSorted with .sort which the spec guarantees to be stable.

Environment: OpenClaw 2026.4.2, Node.js v22

Severity: Low — only cache hit rate, not functionality.

labels=bug,performance

extent analysis

TL;DR

Replace toSorted with the stable .sort method to ensure consistent sorting and prevent Prompt Cache misses.

Guidance

  • Identify all occurrences of toSorted in the codebase and replace them with .sort to maintain stability.
  • Verify the fix by checking the cache hit rate after replacing toSorted with .sort.
  • Test the sorting functionality to ensure it still works as expected after the replacement.
  • Consider adding a comment to explain why .sort is used instead of toSorted for future reference.

Example

function sortToolsMessageItems(items) {
    return items.sort((a, b) => a.name.localeCompare(b.name));
}

Notes

This fix assumes that the only issue is the instability of toSorted and that replacing it with .sort will resolve the Prompt Cache misses. If other issues arise, further investigation may be needed.

Recommendation

Apply workaround: Replace toSorted with .sort to ensure stable sorting and prevent cache misses, as the instability of toSorted is the root cause of the issue.

Vote matrix · Quick signals

Works
Did the solution work? Tap to confirm.
Easy Fix
Was it a quick fix?
Time Saver
Did it save you time?
Blocking
Was it severely blocking?
Common Issue
Are others likely hitting this too?
Flaky / Intermittent
Is it intermittent?
Verified / Reproducible
Can you reproduce it reliably?
Loading…
#retriever error#indexing error#inference speed#output truncation#response parsing

Still need to ship something?

×6

Another batch ranked right after the header list — different links, same matching logic.

Back to top recommendations

TRENDING

openclaw - ✅(Solved) Fix sortToolsMessageItems uses toSorted() which is not stable [2 pull requests, 1 comments, 2 participants]