refactor: agent lifecycle overhaul + session selector #95
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!95
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "undoing-claudes-dumbass-bullshit"
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
AgentManagerintoAgentHost,AgentLifecycleService,AgentInstanceRepository,AgentFactory; collapseIEventPublisherintoIEventBus; renameShearsPaths→ApplicationPathProvider.AgentStartRequest,AgentShutdownRequest,AgentDeath,ConfigurationChangedNotification); session path/data abstractions (SessionPath,IAgentData,AgentSessionPath).ChatHeader.razor,AgentList.razor); disable forced memory index on startup.LlamaShears.IntegrationTests.Fixturesproject; coverage forAgentInstanceRepositoryandAgentLifecycleService.Test plan
dotnet builddotnet testPull request overview
This PR performs a major refactor of the agent runtime lifecycle by replacing the old
AgentManager-centric design with a host/repository/lifecycle-service split, while also unifying event publishing intoIEventBusand introducing session-path/data-scope abstractions. It also updates the web UI to surface live sessions and adds a shared integration-test fixture project to host deterministic provider stubs.Changes:
AgentHost,AgentLifecycleService,AgentInstanceRepository,AgentHandle(cold-to-hot), new lifecycle/config events, and removesAgentManager/IAgentManager.IEventBusand renamesShearsPaths→ApplicationPathProvider.SessionPath,IAgentData) and updates UI/tests to support session selection + new integration test fixtures.Reviewed changes
Copilot reviewed 120 out of 125 changed files in this pull request and generated 10 comments.
Show a summary per file
ApplicationPathProvider.ApplicationPathProvider.IEventPublishertoIEventBus.IAgentManagerdependency.SessionIdtoSessionPath.IApplicationPathProvider.IEventBus.IEventBus.IEventBusand updates dispatcher signatures.IEventBus.AgentLifecycleService.IEventBus.IEventBus.AgentInstanceRepository.IEventBusand adds Subscribe passthrough.Microsoft.AspNetCore.Mvc.Testing.IProviderFactorystub for deterministic test hosting.ILanguageModelstub for integration tests.IAgentDatato seed model config into the per-turn scope.IApplicationPathProvider.IShearsPathsinterface toIApplicationPathProvider.IEventPublisher(publish folded intoIEventBus).IEventBus.PublishAsync.PublishAsynconto the bus interface.IEventPublishertoIEventBus.SessionId.AgentHandle.AgentLifecycleEvent.AgentDeathsingleton lifecycle payload.IAgentData.SessionPath).SessionPath/current/parent/root session ids from scope.IAgentDatainto a dictionary.SessionPath..And(...)chaining helpers to buildDisposableList.IAgentDatato seed config into per-turn scope.IApplicationPathProvider.IApplicationPathProvider.IEventBus.IEventBus.IApplicationPathProvider.IApplicationPathProvider.IApplicationPathProvider.ShearsPathsimplementation toApplicationPathProvider.IEventBus.IAgentFactoryin favor of abstractions surface.IApplicationPathProvider.IAgentManagerdependency).IEventBus.IEventBus.IAgentData, and capture execution context.IApplicationPathProvider.AgentShutdownRequest+ new session-path accessors +IEventBuspublish.IEventPublisher.IEventPublisherimplementation (bus only).IEventBus.IEventBus.💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The session selector uses a value attribute but no @bind/@onchange handler. In HTML/Blazor, value on doesn’t control the selected option by itself, and user changes won't be observed. Use
@bind(or@onchange+ selected option logic) and wire selection into the chat session state.@ -11,7 +11,7 @@ public static class EventingServiceCollectionExtensions{AddEventingFramework registers ForwardService<IEventBus, EventBus>() twice, which is redundant and may hide an intended forwarding for a different interface. Remove the duplicate registration (or forward the missing service if that was the intent).
@ -0,0 +36,4 @@var taskCompletionSource = new TaskCompletionSource();await using var cancellationTokenRegistration =stoppingToken.Register(s => ((TaskCompletionSource)s!).TrySetResult(), taskCompletionSource);stoppingToken.Register(...)returns a CancellationTokenRegistration (sync-disposable). Usingawait usinghere will not compile unless the type provides a DisposeAsync pattern. Use a regularusing var(or dispose it via your DisposableList) instead.@ -0,0 +153,4 @@handle.SessionPath.Root);var sessionShutdownTimeoutCancellationTokenSource = new CancellationTokenSource();var sessionShutdownTimeoutToken = sessionShutdownTimeoutCancellationTokenSource.Token;sessionShutdownTimeoutCancellationTokenSource.CancelAfter(TimeSpan.FromSeconds(15));_logger.BeginScope(...) returns an IDisposable that should be disposed to avoid leaking the logging scope. Capture the returned scope in a using/await using block around the shutdown work.
@ -0,0 +160,4 @@var eventType = Event.WellKnown.Command.AgentShutdown with { Id = handle.SessionPath.Current };var eventParameter = new AgentStopRequest(handle.SessionPath.Current);await _eventPublisher.PublishAsync(eventType, eventParameter, cancellationToken);LogShutdownRequestSentDisposingSessionId(handle.SessionPath.Current);ShutdownAgentAsync publishes an AgentShutdown event but sends an AgentStopRequest payload. Agents subscribe to AgentShutdownRequest, so this mismatch prevents agents from receiving the shutdown signal. Publish AgentShutdownRequest (and ensure the generic type matches) for Event.WellKnown.Command.AgentShutdown.
@ -0,0 +91,4 @@{Remove(descendent.SessionPath.Id);}}AgentInstanceRepository.RemoveDescendents ignores the includeParent parameter and never removes the parent, despite the interface/docs stating it can optionally include the parent. Either implement includeParent behavior (and/or rename) and consider using the Remove(id, out handle) path to preserve the child-safety contract.
@ -0,0 +59,4 @@await using var disposable = birthSubscription.And(deathSubscription).And(stopRegistration as IAsyncDisposable);AgentLifecycleService chains the CancellationTokenRegistration via
stopRegistration as IAsyncDisposable, which is always null (CancellationTokenRegistration only implements IDisposable). This means the registration isn't disposed. Chain it as IDisposable instead so it’s cleaned up when the service exits.@ -0,0 +20,4 @@/// <summary><see langword="true"/> when this notification represents an actual change (birth, tombstone, or hash-distinct update).</summary>public bool HasChanges => IsValid && (IsBirth || IsTombstone ||string.Equals(CurrentConfig!.Hash, UpdatedConfig!.Hash,StringComparison.OrdinalIgnoreCase));ConfigurationChangedNotification.HasChanges is currently true for updates when the old and new hashes are equal, and false when they differ. For update events, this should be the opposite (hash-distinct updates should be considered changes).
@ -13,0 +25,4 @@{throw new NotImplementedException();}}EventBusCapture is checked into the integration test file but is incomplete (empty ctor) and throws NotImplementedException for both IEventBus members, which will break compilation or runtime if referenced. Either remove this class from the test file or fully implement it and add assertions that use it.
@ -187,3 +186,4 @@private sealed class ThrowingPaths : IApplicationPathProvider{public string GetPath(PathKind kind, string? subpath = null, bool ensureExists = false)=> throw new InvalidOperationException("IShearsPaths.GetPath should not be reached when ModelsRoot is explicitly set.");The exception message in ThrowingPaths still references IShearsPaths even though the type was renamed to IApplicationPathProvider. Update the message so failures point to the correct API surface.