Skip to content

[Repo Assist] improve: FrozenDictionary command dispatch map in WindowsNodeClient#197

Closed
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-windowsnodeclient-command-dispatch-2026-04-22-c093b222bf8b2be5
Closed

[Repo Assist] improve: FrozenDictionary command dispatch map in WindowsNodeClient#197
github-actions[bot] wants to merge 1 commit intomasterfrom
repo-assist/improve-windowsnodeclient-command-dispatch-2026-04-22-c093b222bf8b2be5

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This is an automated pull request from Repo Assist.

Summary

Replace two O(n) _capabilities.FirstOrDefault(c => c.CanHandle(command)) linear scans in WindowsNodeClient with an O(1) FrozenDictionary<string, INodeCapability> command map.

Root Cause / Motivation

Both HandleNodeInvokeAsync and the node.invoke response handler dispatch commands by scanning the entire _capabilities list. For each incoming request, every capability's CanHandle() is called in sequence. With many registered capabilities (system, exec, screen capture, future additions), this scales as O(n×m) where n = capabilities and m = commands per capability.

FrozenDictionary is already the established pattern in this codebase for exactly this purpose — ChannelHealth, NotificationCategorizer, and OpenClawGatewayClient all use it for lookups.

Changes

  • WindowsNodeClient.cs

    • Add _commandMap field: FrozenDictionary<string, INodeCapability>.Empty
    • Add BuildCommandMap(): iterates _capabilities and calls TryAdd for each INodeCapability.Commands entry
    • RegisterCapability() now calls BuildCommandMap() after adding to _capabilities
    • Replace both FirstOrDefault dispatch calls with _commandMap.GetValueOrDefault(command)
  • WindowsNodeClientTests.cs

    • Add MockCapability helper class
    • Add 3 tests: routing to registered capability, unknown command (no invocation), first-registered-wins on command collision

Semantics Preserved

TryAdd semantics mean the first registered capability wins for any given command string — identical to the original FirstOrDefault behaviour.

Trade-offs

  • BuildCommandMap() is called once per RegisterCapability() call (all registrations happen at startup, before ConnectAsync, so rebuilding is negligible)
  • Reference assignment to _commandMap is atomic in .NET; no locking needed given the single-registration-phase pattern

Test Status

✅ Build passed
✅ Shared.Tests: 633 passed, 20 skipped (653 total) — 3 new tests added
✅ Tray.Tests: 122 passed

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

To install this agentic workflow, run

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

Replace O(n) linear capability scans with O(1) FrozenDictionary lookup.

- Add _commandMap field (FrozenDictionary<string, INodeCapability>)
- Add BuildCommandMap() to (re)build the map after each RegisterCapability call
- Replace both FirstOrDefault dispatch calls with _commandMap.GetValueOrDefault
- First-registered capability wins on command collision (preserves original semantics)
- Add 3 tests: routing, unknown command, first-registered-wins collision

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman
Copy link
Copy Markdown
Collaborator

Superseded by a new PR that includes the event-path dispatch test identified during rubber-duck review. The code change is identical; only test coverage was added.

@shanselman shanselman closed this Apr 23, 2026
shanselman added a commit that referenced this pull request Apr 23, 2026
…205)

* improve: FrozenDictionary command dispatch map in WindowsNodeClient

Replace O(n) linear capability scans with O(1) FrozenDictionary lookup.

- Add _commandMap field (FrozenDictionary<string, INodeCapability>)
- Add BuildCommandMap() to (re)build the map after each RegisterCapability call
- Replace both FirstOrDefault dispatch calls with _commandMap.GetValueOrDefault
- First-registered capability wins on command collision (preserves original semantics)
- Add 3 tests: routing, unknown command, first-registered-wins collision

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* improve: FrozenDictionary command dispatch map in WindowsNodeClient

Replace two O(n) FirstOrDefault(c => c.CanHandle(command)) scans
with O(1) FrozenDictionary lookup. TryAdd preserves first-registered-
wins semantics. Map rebuilt on each RegisterCapability() call
(startup only, before ConnectAsync).

Add event-path dispatch test to cover HandleNodeInvokeEventAsync
in addition to the request-path tests.

Based on Repo Assist PR #197, with additional test coverage.
Closes #197

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant