Refactor/session service#150
Draft
stippi wants to merge 13 commits into
Draft
Conversation
Move the user (global) skill scope from <config_dir>/skills to the cross-harness ~/.agents/skills location (the convention codex and Claude Code use), so user-authored skills can be shared across agent harnesses. Bundled system skills are unchanged: still extracted to <config_dir>/skills/.system (not into ~/.agents/skills). Adds config::user_skills_root() (overridable via CODE_ASSISTANT_USER_SKILLS_DIR) and system_skills_root() as the single source of truth, used by scope resolution, discovery, and bundled install.
Typed async methods replace the BackendEvent/BackendResponse protocol: each UI command gets its own reply or error, correlated by construction instead of by session_id over unpaired channels. Internally an actor — commands run strictly in order on the backend tokio runtime, so callers on other executors (GPUI) stay decoupled and session-mutation serialization is preserved. Also fixes a latent session-ID collision found by the new tests: generate_session_id had second resolution with a deterministic suffix, so sessions created within the same second overwrote each other. IDs now include a process-local counter. The BackendEvent dispatcher in backend.rs stays until the frontends are ported; it is deleted at the end of this branch.
The BackendEvent/BackendResponse channels and the response-translator task are gone. Commands go through a small Actions helper that calls the service in spawned tasks and applies each typed result to the app state, so the input loop never blocks. Startup session creation awaits its result directly instead of blocking on an uncorrelated response channel; a failing continue-session now falls back to a fresh session instead of silently keeping a broken id.
Commands are now typed methods in app/commands.rs, dispatched on the GPUI background executor; each applies its own result via the UI event queue or shared state. This deletes the BackendResponse translator (app/backend.rs) and the polling response task — command results no longer take the detour command-channel → response-channel → UiEvent. Components call the Gpui command methods directly instead of pushing command-flavored UiEvents; branch switching now reuses SetMessages + UpdatePlan instead of a dedicated BranchSwitched event. The wiring's startup path (initial task / connect latest session) also goes through the service instead of hand-rolling LLM client and agent start.
All frontends now go through SessionService, so the 1850-line backend.rs dispatcher and its two enums are gone. UiEvent loses the variants that were commands or RPC responses in disguise: SendUserMessage, QueueUserMessage, RequestPendingMessageEdit, StartMessageEdit, SwitchBranch, CancelSubAgent and BranchSwitched. What remains is a pure core→UI notification vocabulary (plus MessageEditReady, which the GPUI edit flow emits to hop its truncated transcript onto the UI thread). ui_acp is intentionally untouched: it never used the backend channels and keeps its direct SessionManager access — its protocol-adapter needs (deferred session creation with client ids, per-prompt agent starts) do not map onto the service API, and widening the service for one consumer would be new bloat.
SessionActivity is a shared handle owning the transition rules (terminal states persist, streaming lifecycle transitions, first visible output, rate limiting). ProxyUI now only reports lifecycle moments to it and broadcasts resulting changes — it is back to being a pure filter/buffer/forwarder. The agent task in SessionManager uses the same handle to set final states. Rules are covered by dedicated unit tests.
Session-tagged broadcast (tokio::sync::broadcast) as the future single downstream channel. Frontends subscribe and filter; a lagging subscriber observes StreamError::Lagged and resyncs via a session snapshot. Not wired up yet — the following commits move the publish side (ProxyUI, service, watcher) and the frontends onto it.
Dual-write transition step: ProxyUI, SessionService and the agent lifecycle now publish session-tagged events (and streaming fragments) to the EventStream in addition to the legacy single-UI push. The service exposes subscribe() and a new request_stop(session_id) — stop requests become a per-session core-side flag, which also makes cancelling a *background* session's agent possible (previously only the connected session could be cancelled). Frontends still consume the legacy push; they migrate one by one in the following commits, after which the legacy path and the ProxyUI gate/buffers are removed.
set_active_session now assembles a SessionSnapshot (transcript incl. the in-flight partial assistant message, tool results, plan, activity, metadata, pending message, model and sandbox state) instead of a burst of UiEvents. The service still derives the legacy connect-event sequence from the snapshot for frontends that haven't migrated to the stream yet (connect_events shim). The live tool-status map is no longer drained on connect — snapshots are non-destructive so any number of subscribers can resync; the map is cleared when a new agent run starts instead.
Both frontends now subscribe to the core→UI stream and filter by the viewed session instead of implementing UserInterface for the legacy push path (the service gets a NullUserInterface). Session loads apply the returned SessionSnapshot via the canonical connect-event sequence (SessionSnapshot::connect_events); a lagged subscriber resyncs with a fresh snapshot. Cancellation goes through service.request_stop — the core-side per-session flag — instead of relying on the UI being asked should_streaming_continue. The terminal derives its rate-limit spinner from RateLimited activity events (previously trait callbacks).
The per-session ProxyUI becomes a SessionEventPublisher: it publishes session-tagged events/fragments and records in-flight state for snapshots, nothing else. Gone are: - the is_ui_connected gate and set_ui_connected plumbing — 'connected' is now purely a frontend-side filter on the broadcast stream - the reconnect replay logic and its dedup rules — resync is snapshot + subsequent events, for any number of subscribers - the 1000-fragment ring-buffer cap that silently truncated long in-flight responses on reconnect (the buffer is bounded by one response and cleared on streaming start/stop/rollback) - the ui: Arc<dyn UserInterface> parameters on SessionService and SessionManager::start_agent_* — UserInterface remains only as the agent-side seam, implemented by the publisher - the dead as_any escape hatch on UserInterface ACP routes stream events to its per-prompt ACPUserUI via the existing active_uis registry; a short drain grace before deregistering covers the final events of a turn. Cancellation everywhere goes through the core-side per-session stop flag.
switch_model and change_sandbox_policy publish their change to the broadcast stream so every view of the session updates, not just the caller (the model-switch warning stays caller-only). GPUI drops its now-redundant local pushes. AGENTS.md describes the actual architecture again: SessionService commands upstream, one broadcast EventStream downstream, SessionEventPublisher at the agent seam.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.