From ecd31642d7ed55773aedc0f04b45447b21cfafd1 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 27 Aug 2026 17:18:13 +0200 Subject: [PATCH] fix(coding-agent): remove unnecessary local service tokens --- .../src/experimental/client-tui.ts | 78 +++++-------------- .../src/experimental/services/README.md | 6 +- .../experimental/services/models-provider.ts | 49 +++++------- .../src/experimental/services/tui.ts | 14 ---- .../src/experimental/services/worker.ts | 19 +++-- .../test/experimental-client-tui.test.ts | 5 +- 6 files changed, 54 insertions(+), 117 deletions(-) delete mode 100644 packages/coding-agent/src/experimental/services/tui.ts diff --git a/packages/coding-agent/src/experimental/client-tui.ts b/packages/coding-agent/src/experimental/client-tui.ts index 9a2a6e492..3a5a98e2d 100644 --- a/packages/coding-agent/src/experimental/client-tui.ts +++ b/packages/coding-agent/src/experimental/client-tui.ts @@ -19,14 +19,13 @@ import { CustomEditor } from "../modes/interactive/components/custom-editor.ts"; import { getEditorTheme, initTheme, theme } from "../modes/interactive/theme/theme.ts"; import { type OpenClientRuntimeOptions, openClientRuntime } from "./client-runtime.ts"; import { ExperimentalChatView } from "./client-tui-chat.ts"; -import { combineFacetLoaders, createStaticFacetLoader, type FacetLoader, type LoadedFacets } from "./facet-loader.ts"; +import { combineFacetLoaders, type FacetLoader, type LoadedFacets } from "./facet-loader.ts"; import { createFacetHost, defineFacet, type FacetHost } from "./facets.ts"; import { type LaneReplica, type LaneWatchSource, openLaneReplica } from "./lane-replica.ts"; import { AgentController, type AgentOperationResponse, type AgentQueueResponse } from "./services/agent-controller.ts"; import type { ServerServiceConnection, SessionServiceConnection } from "./services/connection.ts"; import { Models } from "./services/models.ts"; import { SessionDirectory, SessionManagement } from "./services/sessions.ts"; -import { Tui } from "./services/tui.ts"; export interface RunClientTuiOptions extends OpenClientRuntimeOptions { readonly facetLoader?: FacetLoader; @@ -70,46 +69,6 @@ const selectTheme = { noMatch: (text: string) => chalk.yellow(text), }; -export const sessionsTuiFacet = defineFacet({ - id: "@pi/sessions", - setup(env) { - const tui = env.use(Tui); - const directory = env.use(SessionDirectory); - const management = env.use(SessionManagement); - env.onActivate(() => { - env.own(tui.registerSessions(directory, management)); - env.own(directory.state.subscribe(tui.refresh)); - }); - }, -}); - -export const modelSelectionTuiFacet = defineFacet({ - id: "@pi/model-selection", - setup(env) { - const tui = env.use(Tui); - const models = env.use(Models); - env.onActivate(() => { - env.own(tui.registerModelSelection(models)); - env.own(models.state.subscribe(tui.refresh)); - }); - }, -}); - -export const agentControllerTuiFacet = defineFacet({ - id: "@pi/agent-controller", - setup(env) { - const tui = env.use(Tui); - const controller = env.use(AgentController); - env.onActivate(() => env.own(tui.registerAgentController(controller))); - }, -}); - -const BUILTIN_TUI_FACET_LOADER = createStaticFacetLoader([ - sessionsTuiFacet, - modelSelectionTuiFacet, - agentControllerTuiFacet, -]); - /** Service-only presentation driven by a replicated main-lane snapshot. */ export class ExperimentalClientTui implements Component { readonly #ui: TUI; @@ -163,10 +122,9 @@ export class ExperimentalClientTui implements Component { finish(): void; }): Promise { initTheme(); - const loadedFacets = await combineFacetLoaders([ - BUILTIN_TUI_FACET_LOADER, - ...(options.facetLoader === undefined ? [] : [options.facetLoader]), - ]).load(); + const loadedFacets = await combineFacetLoaders( + options.facetLoader === undefined ? [] : [options.facetLoader], + ).load(); const component = new ExperimentalClientTui(options.ui, options.requestRender, options.finish, loadedFacets); try { await component.#start(options.servers); @@ -211,11 +169,15 @@ export class ExperimentalClientTui implements Component { async #start(servers: readonly ClientTuiServer[]): Promise { for (const server of servers) { - const tuiRuntimeFacet = defineFacet({ - id: "@pi/tui-runtime", + const presentationBridgeFacet = defineFacet({ + id: "@pi/presentation-bridge", setup: (env) => { - env.provide(Tui, { - registerSessions: (directory, management) => + const directory = env.use(SessionDirectory); + const management = env.use(SessionManagement); + const models = env.use(Models); + const controller = env.use(AgentController); + env.onActivate(() => { + env.own( this.#register(this.#sessions, "Sessions", { serverId: server.serverId, directory, @@ -223,20 +185,18 @@ export class ExperimentalClientTui implements Component { session: server.session, laneWatches: server.laneWatches, }), - registerModelSelection: (models) => + ); + env.own( this.#register(this.#modelSelections, "Model selection", { serverId: server.serverId, models }), - registerAgentController: (controller) => - this.#register(this.#agents, "Agent controller", { serverId: server.serverId, controller }), - refresh: () => this.#rebuild(), - setStatus: (status) => { - this.#status = status; - this.#rebuild(); - }, + ); + env.own(this.#register(this.#agents, "Agent controller", { serverId: server.serverId, controller })); + env.own(directory.state.subscribe(() => this.#rebuild())); + env.own(models.state.subscribe(() => this.#rebuild())); }); }, }); const facetHost = await createFacetHost({ - facets: [tuiRuntimeFacet, ...this.#loadedFacets.facets], + facets: [presentationBridgeFacet, ...this.#loadedFacets.facets], connections: [server.server, server.session], }); this.#facetHosts.push(facetHost); diff --git a/packages/coding-agent/src/experimental/services/README.md b/packages/coding-agent/src/experimental/services/README.md index f78f7502b..9d0afd948 100644 --- a/packages/coding-agent/src/experimental/services/README.md +++ b/packages/coding-agent/src/experimental/services/README.md @@ -15,6 +15,8 @@ Facet setup generates each host's RPC service catalogue from its provided RPC-ca With `PI_EXPERIMENTAL=1`, an interactive `pi client` creates and attaches a Session before opening the service-only chat TUI. `pi client -c` and `pi client -r` attach the newest existing Session instead, preserving its durable model and thinking configuration. Model selection is available on demand through `/model`; it is never a startup screen. The presentation drives the worker-owned main lane through `AgentController` and renders a coherent lane-watch replica with the Harness reducer while controller calls are pending. `Transcript` remains reserved for a later service-owned live-delta replacement of that compatibility watch. -The Session worker loads all built-in and configured Session facets through one `FacetLoader`, then creates one active `FacetHost`. The service-only TUI likewise loads all TUI facets once and creates one host that combines its local `Tui` service with connected server and selected-Session services. A host may reload selected facets when their declared service shape is unchanged; retained consumers keep the same local and RPC service facades while replacement implementations and singleton snapshots are installed behind them. Runtime facets provide host-created services such as `AgentController`, `ModelsRuntime`, and `Tui` through the same `env.provide()` operation used by every other facet. Synchronous setup-time `env.provide()`, `env.provideMany()`, `env.use()`, and `env.observe()` calls produce the internal dependency graph; setup does not repeat a declarative dependency list, and service handles remain disconnected until the complete graph validates. Providers activate before consumers, observations connect with their consuming facet, and facet replacement or host shutdown disposes affected lifecycles in reverse dependency order. Services are RPC-capable by default; process-local tokens such as `ModelsRuntime` and `Tui` declare `{ rpc: false }`. The host automatically publishes every provided RPC-capable service. +The Session worker loads all built-in and configured Session facets through one `FacetLoader`, then creates one active `FacetHost`. Host-created implementation dependencies such as `AgentLane`, `ModelRuntime`, and `SettingsManager` are passed directly to built-in facet factories; they are not exposed as services. The TUI loads configured presentation facets and adds one private bridge facet that consumes the connected server and selected-Session services for `ExperimentalClientTui`. There is no local TUI service until the presentation host has a real command, modal, and renderer contribution API worth exposing to independently loaded facets. -A facet always calls unqualified `env.use()` or `env.observe()`. The host resolves each token across facet-provided and connected services. Transport namespaces remain internal binding machinery rather than part of the facet environment. TUI facets own subscriptions to their replicated state; `ExperimentalClientTui` retains only terminal rendering, navigation, and action dispatch. The server provider remains directly assembled until its complete host environment exists. The question dialog, diff review, Git, indexing-job, and canvas examples in `packages/agent/docs/plugins.md` are extension patterns, not built-in coding-agent services. Private references, trace carriers, and flow control remain protocol/host infrastructure slices rather than presentation service tokens. +A host may reload selected facets when their declared service shape is unchanged; retained consumers keep the same RPC service facades while replacement implementations and singleton snapshots are installed behind them. Synchronous setup-time `env.provide()`, `env.provideMany()`, `env.use()`, and `env.observe()` calls produce the internal dependency graph; setup does not repeat a declarative dependency list, and service handles remain disconnected until the complete graph validates. Providers activate before consumers, observations connect with their consuming facet, and facet replacement or host shutdown disposes affected lifecycles in reverse dependency order. All currently implemented experimental service tokens are RPC-capable and automatically published by their providing host. + +A facet always calls unqualified `env.use()` or `env.observe()`. The host resolves each token across facet-provided and connected services. Transport namespaces remain internal binding machinery rather than part of the facet environment. `ExperimentalClientTui` currently owns terminal rendering, state subscriptions, navigation, and action dispatch directly. The server provider remains directly assembled until its complete host environment exists. The question dialog, diff review, Git, indexing-job, canvas, and rich local TUI service examples in `packages/agent/docs/plugins.md` are extension patterns, not built-in coding-agent services. Private references, trace carriers, and flow control remain protocol/host infrastructure slices rather than presentation service tokens. diff --git a/packages/coding-agent/src/experimental/services/models-provider.ts b/packages/coding-agent/src/experimental/services/models-provider.ts index 30ff8fcee..ff3ea061c 100644 --- a/packages/coding-agent/src/experimental/services/models-provider.ts +++ b/packages/coding-agent/src/experimental/services/models-provider.ts @@ -2,7 +2,6 @@ import { type AgentLane, BACKGROUND_CONTEXT, type Context, - defineService, type MutableReplicatedState, } from "@earendil-works/pi-agent-core"; import { getSupportedThinkingLevels } from "@earendil-works/pi-ai"; @@ -11,29 +10,6 @@ import type { SettingsManager } from "../../core/settings-manager.ts"; import { defineFacet, type Facet } from "../facets.ts"; import { Models, type Models as ModelsService, type ModelsState } from "./models.ts"; -export interface ModelsRuntime { - getAvailableSnapshot(): ReturnType; - refresh(options?: Parameters[0]): ReturnType | undefined; - getModel(...args: Parameters): ReturnType; - persistModel(provider: string, modelId: string): Promise; -} -export const ModelsRuntime = defineService("pi.local.models-runtime", { rpc: false }); - -export function createModelsRuntime( - runtime: ModelRuntime | undefined, - settingsManager?: SettingsManager, -): ModelsRuntime { - return { - getAvailableSnapshot: () => runtime?.getAvailableSnapshot() ?? [], - refresh: (options) => runtime?.refresh(options), - getModel: (...args) => runtime?.getModel(...args), - async persistModel(provider, modelId) { - settingsManager?.setDefaultModelAndProvider(provider, modelId); - await settingsManager?.flush(); - }, - }; -} - export interface ModelsServiceRuntime { readonly service: ModelsService; activate(context: Context): Promise; @@ -41,7 +17,8 @@ export interface ModelsServiceRuntime { export function createModelsService( lane: AgentLane, - modelRuntime: ModelsRuntime, + modelRuntime: ModelRuntime | undefined, + settingsManager: SettingsManager | undefined, createState: (initial: ModelsState) => MutableReplicatedState, ): ModelsServiceRuntime { let catalogRevision = 0; @@ -59,7 +36,7 @@ export function createModelsService( }; const readCatalog = async (context: Context): Promise => { const selected = await lane.getModel(context); - const available = modelRuntime.getAvailableSnapshot(); + const available = modelRuntime?.getAvailableSnapshot() ?? []; const catalog = selected === undefined || includesModel(available, selected) ? available : [...available, selected]; catalogRevision += 1; @@ -87,7 +64,7 @@ export function createModelsService( }, async refresh(context) { state.set({ ...state.value, refresh: { status: "refreshing" } }, context); - const refresh = modelRuntime.refresh({ signal: context.abortSignal }); + const refresh = modelRuntime?.refresh({ signal: context.abortSignal }); if (refresh === undefined) { state.set( { @@ -113,10 +90,11 @@ export function createModelsService( ); }, async select(model, context) { - const selected = modelRuntime.getModel(model.provider, model.modelId); + const selected = modelRuntime?.getModel(model.provider, model.modelId); if (selected === undefined) throw new Error(`Unknown model: ${model.provider}/${model.modelId}`); await lane.setModel({ provider: selected.provider, modelId: selected.id }, context); - await modelRuntime.persistModel(selected.provider, selected.id); + settingsManager?.setDefaultModelAndProvider(selected.provider, selected.id); + await settingsManager?.flush(); state.set({ ...state.value, configuration: await readConfiguration(context) }, context); }, }; @@ -129,11 +107,20 @@ export function createModelsService( }; } -export function createModelsServiceFacet(lane: AgentLane): Facet { +export function createModelsServiceFacet(options: { + readonly lane: AgentLane; + readonly modelRuntime: ModelRuntime | undefined; + readonly settingsManager?: SettingsManager; +}): Facet { return defineFacet({ id: "@pi/models", setup(env) { - const runtime = createModelsService(lane, env.use(ModelsRuntime), env.replicatedState); + const runtime = createModelsService( + options.lane, + options.modelRuntime, + options.settingsManager, + env.replicatedState, + ); env.provide(Models, runtime.service); env.onActivate(() => runtime.activate(BACKGROUND_CONTEXT)); }, diff --git a/packages/coding-agent/src/experimental/services/tui.ts b/packages/coding-agent/src/experimental/services/tui.ts deleted file mode 100644 index cfac168d4..000000000 --- a/packages/coding-agent/src/experimental/services/tui.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { defineService } from "@earendil-works/pi-agent-core"; -import type { AgentController } from "./agent-controller.ts"; -import type { Models } from "./models.ts"; -import type { SessionDirectory, SessionManagement } from "./sessions.ts"; - -export interface Tui { - registerSessions(directory: SessionDirectory, management: SessionManagement): () => void; - registerModelSelection(models: Models): () => void; - registerAgentController(controller: AgentController): () => void; - refresh(): void; - setStatus(status: string): void; -} - -export const Tui = defineService("pi.local.tui", { rpc: false }); diff --git a/packages/coding-agent/src/experimental/services/worker.ts b/packages/coding-agent/src/experimental/services/worker.ts index 66504d001..270857464 100644 --- a/packages/coding-agent/src/experimental/services/worker.ts +++ b/packages/coding-agent/src/experimental/services/worker.ts @@ -22,7 +22,7 @@ import { combineFacetLoaders, createStaticFacetLoader, type FacetLoader } from " import { createFacetHost, defineFacet, type Facet, type FacetHost } from "../facets.ts"; import { AgentController } from "./agent-controller.ts"; import { createAgentController } from "./agent-controller-provider.ts"; -import { createModelsRuntime, createModelsServiceFacet, ModelsRuntime } from "./models-provider.ts"; +import { createModelsServiceFacet } from "./models-provider.ts"; import { accountsServiceFacet, transcriptServiceFacet } from "./stubs-provider.ts"; export const ServiceOperationResultSchema = Type.Object( @@ -56,9 +56,13 @@ export interface SessionWorkerServices { dispose(): Promise; } -function createBuiltinSessionFacetLoader(lane: AgentLane): FacetLoader { +function createBuiltinSessionFacetLoader(options: { + readonly lane: AgentLane; + readonly modelRuntime: ModelRuntime | undefined; + readonly settingsManager?: SettingsManager; +}): FacetLoader { return createStaticFacetLoader([ - createModelsServiceFacet(lane), + createModelsServiceFacet(options), accountsServiceFacet, transcriptServiceFacet, ] satisfies readonly Facet[]); @@ -71,16 +75,15 @@ export async function createSessionWorkerServices(options: { readonly facetLoader?: FacetLoader; publish(scope: WorkerServiceScope, subscriptionId: string, update: ProtocolServiceProviderUpdate): Promise; }): Promise { - const sessionRuntimeFacet = defineFacet({ - id: "@pi/session-runtime", + const agentControllerRuntimeFacet = defineFacet({ + id: "@pi/agent-controller-runtime", setup(env) { env.provide(AgentController, createAgentController(options.lane)); - env.provide(ModelsRuntime, createModelsRuntime(options.modelRuntime, options.settingsManager)); }, }); const loader = combineFacetLoaders([ - createStaticFacetLoader([sessionRuntimeFacet]), - createBuiltinSessionFacetLoader(options.lane), + createStaticFacetLoader([agentControllerRuntimeFacet]), + createBuiltinSessionFacetLoader(options), ...(options.facetLoader === undefined ? [] : [options.facetLoader]), ]); const loaded = await loader.load(); diff --git a/packages/coding-agent/test/experimental-client-tui.test.ts b/packages/coding-agent/test/experimental-client-tui.test.ts index c1a934660..22d9a93ca 100644 --- a/packages/coding-agent/test/experimental-client-tui.test.ts +++ b/packages/coding-agent/test/experimental-client-tui.test.ts @@ -26,7 +26,6 @@ import { type SessionDirectoryState, SessionManagement, } from "../src/experimental/services/sessions.ts"; -import { Tui } from "../src/experimental/services/tui.ts"; const serverId = "00000000-0000-4000-8000-000000000001"; @@ -240,8 +239,8 @@ describe("experimental client TUI", () => { defineFacet({ id: "test-tui-facet", setup(env) { - const tui = env.use(Tui); - env.onActivate(() => tui.setStatus("Loaded test TUI facet")); + const models = env.use(Models); + env.onActivate(() => env.own(models.state.subscribe(() => {}))); }, }), ],