feat(commands): introduce slash command registry and migrate existing commands #38
No reviewers
Labels
No labels
bug
commercial
documentation
duplicate
enhancement
feature
good first issue
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
jasoncouture/llama-shears!38
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/slash-command-registry"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Lifts slash-command metadata + dispatch out of
ChatSession's switch statement and into a DI-populated registry. New public abstractions package (LlamaShears.Core.Abstractions.Commands) so plugins can ship their own/whatevercommands once the package is on the feed. Migrates the four existing commands (/clear,/archive,/compact,/restart) onto the new shape.Public surface (new package)
src/public/LlamaShears.Core.Abstractions.Commands/:ISlashCommand—Name,Description,Parameters,Task<SlashCommandResult> ExecuteAsync(SlashCommandContext, CancellationToken).ISlashCommandRegistry— DI-populated catalogue,Find(name)(case-insensitive) +Commandsenumeration in registration order.SlashCommandParameter— declared parameter metadata (name, description, required).SlashCommandContext— per-invocation payload (agent id + positionalImmutableArray<string>).SlashCommandResult— execution outcome with post-execute hints. Field isContextChanged(general semantic — "the agent's context was modified, refresh the view") rather than something UI-specific likeResetBubblesso plugin authors aren't pinned to the chat surface.Per the existing
src/public/rules: README included, packs under PR 4, builds with the README enforcement from PR 1.Implementation (lives in Api.Web)
src/LlamaShears.Api.Web/Services/SlashCommands/:SlashCommandRegistry— backsISlashCommandRegistry. Frozen-dictionary lookup, ordering preserved.ClearCommand/ArchiveCommand/CompactCommand— call straight intoIAgentDirectoryexactly the same way the old switch did. Clear/Archive returnSlashCommandResult.ContextWasChanged.RestartCommand— callsIHostRestarter.RequestRestart(introduced in PR #37).SlashCommandsServiceCollectionExtensions.AddSlashCommands— wires the registry + the four shipped commands; called fromAddWebUi.ChatSession refactor
Gone:
ChatCommandenum,TryParseCommand, the per-command switch inExecuteCommandAsync. NewTryDispatchSlashCommandAsync:/.SlashCommandContext(agentId, args), awaitcommand.ExecuteAsync(...).result.ContextChanged, callResetBubbles.Constructor swaps
IHostRestarter(added in PR 7) forISlashCommandRegistry. The restart plumbing now flows throughRestartCommand.Execution model is intentionally unchanged
Each command still calls straight into the existing service synchronously — exactly the same calls the old switch made. The "should ride the bus like ChannelMessage" refactor (per the
project_command_requests_should_be_eventsmemory) is explicitly out of scope and left for later.Tests
5 new TUnit cases on
SlashCommandRegistry:FindReturnsRegisteredCommandFindIsCaseInsensitiveFindReturnsNullWhenAbsentFindReturnsNullForNullOrEmptyCommandsPreservesRegistrationOrderThe four migrated commands continue to be exercised by the existing chat-session integration coverage.
Test plan
dotnet test --solution LlamaShears.slnx— 389 passing (was 384).dotnet-test-on-source-changepre-commit check passed.docs-api-up-to-datepre-push check passed (the new package'sdocs/api/index regenerated cleanly).src/public/Directory.Build.targetsREADME enforcement (PR 1) green for the new package.Stacking note
Targets
feat/self-restart(PR #37). Will retargetmainautomatically as the chain merges.🤖 Generated with Claude Code
Pull request overview
Introduces a DI-backed slash-command registry and a new public abstractions package so hosts/plugins can define and dispatch
/commandswithoutChatSessionowning a hard-coded switch.Changes:
LlamaShears.Core.Abstractions.CommandsdefiningISlashCommand,ISlashCommandRegistry, and supporting context/result types./clear,/archive,/compact,/restartintoLlamaShears.Api.Webslash-command implementations and wired them intoAddWebUi.ChatSessionto dispatch via the registry and added unit tests + generated API docs for the new package.Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
ContextChanged)./restartviaIHostRestarter./compactviaIAgentDirectory.RequestCompactionAsync./clearviaIAgentDirectory.ClearAsync(archive:false)./archiveviaIAgentDirectory.ClearAsync(archive:true).SlashCommandResult.SlashCommandParameter.SlashCommandContext.ISlashCommandRegistry.ISlashCommand.💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
@ -481,25 +480,34 @@ public sealed class ChatSession :Changed?.Invoke();TryDispatchSlashCommandAsyncclaims (per PR description) to split on whitespace, but this implementation only splits on the literal space character. That means inputs separated by tabs (common when pasting) or other whitespace won’t parse as expected (e.g./cmd\targ). Consider splitting on all whitespace characters to match the intended behavior.@ -0,0 +1,20 @@using System.Collections.Immutable;ToFrozenDictionary(c => c.Name, OrdinalIgnoreCase)will throw if two commands share the same name (including differing only by case), but the exception won’t identify which registrations conflicted. Since this is meant to be plugin-extensible, consider detecting duplicates up-front and throwing an error that lists the conflicting command names and implementation types to make misconfiguration easy to diagnose.@ -0,0 +8,4 @@private readonly ImmutableDictionary<string, ISlashCommand> _byName;public SlashCommandRegistry(IEnumerable<ISlashCommand> commands){SlashCommandRegistrydoesn’t guard againstcommandsbeing null; if this is ever constructed outside DI (e.g., tests or custom wiring),[.. commands]will throw a less-informative exception. Consider an explicitArgumentNullException.ThrowIfNull(commands)to fail fast with a clearer error.@ -0,0 +1,21 @@using LlamaShears.Core.Abstractions.Commands;Registering
ISlashCommandRegistryas a singleton means it will resolve and capture allISlashCommandinstances at container build time. This effectively forces commands to be singleton/transient-at-startup and will fail if a plugin registers anISlashCommandas scoped (DI will reject scoped->singleton). Consider making the registry scoped (matchingChatSession) or changing the registry to resolve commands per-scope (and/or documenting that commands must be singleton-safe).@ -0,0 +40,4 @@public async Task FindReturnsNullForNullOrEmpty(){var registry = new SlashCommandRegistry([new StubCommand("/foo")]);Test name says it covers null or empty input, but it only asserts the empty-string case. Either add an explicit
nullcase (e.g.,registry.Find(null!)) or rename the test to only mention empty input so the intent matches the coverage.