[Repo Assist] perf: single-pass Remove(key, out value) for pending-request dictionaries

[github-actions]

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Replace double-lookup TryGetValue + Remove with the single-pass Dictionary.Remove(TKey, out TValue) overload in two gateway-client hot-path methods.

Rationale

Both TakePendingRequestMethod and TakePendingChatSend are called for every incoming WebSocket response message — a high-frequency path under a lock. The previous implementation performed two separate hash-table lookups:

// Before — two lookups
if (!dict.TryGetValue(key, out var value)) return null;
dict.Remove(key);
return value;

Dictionary<K,V>.Remove(K, out V) (available since .NET Core 2.0) resolves the key once and removes the entry in the same operation:

// After — one lookup
return dict.Remove(key, out var value) ? value : null;

This also reduces the method body from 4–5 lines to 1 line under the lock, improving readability.

Scope

• TakePendingRequestMethod — called on every "res" type message (all gateway responses)
• TakePendingChatSend — called on every response, attempting to match a pending chat.send

No behaviour change; the ConcurrentDictionary-based _pendingWizardResponses already used TryRemove (equivalent single-pass) and is unchanged.

Test Status

• ✅ Shared tests: 2122 passed, 8 pre-existing failures unchanged
• ✅ Tray tests: 974 passed, 0 failed
• ⚠️ Build (./build.ps1): GitVersion MSBuild task requires GITHUB_ENV (pre-existing infrastructure limitation in agent environment)

Generated by 🌈 Repo Assist, see workflow run. Learn more.

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed June 8, 2026, 10:08 AM ET / 14:08 UTC.

Summary
This PR changes OpenClawGatewayClient to use Dictionary.Remove(key, out value) in TakePendingRequestMethod and TakePendingChatSend instead of a separate lookup plus removal.

Reproducibility: not applicable. this is a cleanup/performance PR, not a bug report with a failing runtime scenario. Source inspection confirms the old two-step lookup/remove pattern exists on current main.

Review metrics: 2 noteworthy metrics.

• Diff size: 1 file changed, +2/-10. The patch is a small production-only refactor with limited review surface.
• Hot-path methods: 2 private methods changed. Both changes affect gateway response handling under existing locks, so behavior preservation matters even though the diff is small.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Run ./build.ps1, shared tests, and tray tests in an environment where GITHUB_ENV and test assets are available, or record the maintainer-accepted validation exception.

Risk before merge

• [P1] Required validation is not cleanly established: the PR body reports shared tests with 8 pre-existing failures and ./build.ps1 blocked by GITHUB_ENV, so maintainers should confirm the repo-required build and tests in a suitable environment before merge.

Maintainer options:

1. Decide the mitigation before merge
   Land the narrow cleanup after repo-required validation is completed or accepted by maintainers for this automation PR.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• [P2] No repair lane is needed; this PR is already a narrow implementation candidate and only needs normal maintainer validation and merge handling.

Security
Cleared: The diff only changes private in-memory dictionary removal logic and does not touch credentials, dependencies, CI, scripts, or other security-sensitive surfaces.

Review details

Best possible solution:

Land the narrow cleanup after repo-required validation is completed or accepted by maintainers for this automation PR.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a cleanup/performance PR, not a bug report with a failing runtime scenario. Source inspection confirms the old two-step lookup/remove pattern exists on current main.

Is this the best way to solve the issue?

Yes: for net10.0, replacing TryGetValue plus Remove with Remove(key, out value) is the narrowest maintainable equivalent, and the PR keeps the existing locks and null handling intact.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 37b0ea672037.

Label changes

Label changes:

• add P3: This is a low-risk internal performance cleanup rather than an urgent user-facing regression.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a bot-generated Repo Assist PR, so the external contributor real-behavior proof gate is not applicable.

Label justifications:

• P3: This is a low-risk internal performance cleanup rather than an urgent user-facing regression.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This is a bot-generated Repo Assist PR, so the external contributor real-behavior proof gate is not applicable.

Evidence reviewed

What I checked:

• Repository policy read: AGENTS.md was read in full; its required validation guidance is relevant because the PR body reports incomplete required validation. (AGENTS.md:1, 37b0ea672037)
• Current main still has the old pattern: Current main still calls TryGetValue, then Remove, then returns the value in both pending-request methods, so the central cleanup is not already implemented on main. (src/OpenClaw.Shared/OpenClawGatewayClient.cs:1361, 37b0ea672037)
• PR patch is narrowly scoped: The PR commit changes one production file with 2 additions and 10 deletions, replacing only the two locked dictionary take/remove sequences. (src/OpenClaw.Shared/OpenClawGatewayClient.cs:1361, fda1e42f5c65)
• Target framework supports the overload: OpenClaw.Shared targets net10.0, so Dictionary<TKey,TValue>.Remove(TKey, out TValue) is available. (src/OpenClaw.Shared/OpenClaw.Shared.csproj:4, 37b0ea672037)
• Existing tests cover pending chat response behavior: Shared tests already exercise successful and failed pending chat.send response completion, which is the behavior preserved by the refactor. (tests/OpenClaw.Shared.Tests/OpenClawGatewayClientTests.cs:904, 37b0ea672037)
• Release check: The latest local release tag v0.6.3 still contains the double-lookup implementation, and no tag or local branch contains the PR commit, so the PR is not superseded by a shipped fix. (src/OpenClaw.Shared/OpenClawGatewayClient.cs:1356, 85445c78066b)

Likely related people:

• Christine Yan: git blame attributes the pending request dictionaries and both take/remove methods to the v0.6.3 snapshot commit; the shallow graft may hide earlier provenance, so this is the best local source-history anchor. (role: introduced behavior; confidence: medium; commits: 85445c78066b; files: src/OpenClaw.Shared/OpenClawGatewayClient.cs)
• Scott Hanselman: Recent commits touched OpenClawGatewayClient.cs and its tests shortly before this PR, including Slopwatch hardening and configured-model filtering work. (role: recent area contributor; confidence: medium; commits: d23f8ca50013, d1b136347e95; files: src/OpenClaw.Shared/OpenClawGatewayClient.cs, tests/OpenClaw.Shared.Tests/OpenClawGatewayClientTests.cs)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.