diff --git a/ts/docs/architecture/lifecycle/agent-lifecycle.md b/ts/docs/architecture/lifecycle/agent-lifecycle.md index 564756ef2..214bc9c88 100644 --- a/ts/docs/architecture/lifecycle/agent-lifecycle.md +++ b/ts/docs/architecture/lifecycle/agent-lifecycle.md @@ -17,8 +17,8 @@ The set of agents a dispatcher can run is split by **how it changes**: `AppAgentProvider`s and never changes for the life of the session. - The **dynamic** set (installed agents) is owned by a host-side `AppAgentSource`. A dispatcher _connects_ to the source; `connect()` returns the - shared provider instances to register and hands the source a small callback - (`AppAgentHost`) it uses to register/unregister agents as the set changes. + shared provider instances to register and hands the source an + `AppAgentProviderSetController` it uses to change that session's live provider set. ```mermaid flowchart TB @@ -26,16 +26,17 @@ flowchart TB src["AppAgentSource
(record store + registry + lifecycle tracker)"] pkg["@package app agent"] end - d1["Dispatcher A"] -- connect(hostA) --> src - d2["Dispatcher B"] -- connect(hostB) --> src - src -. addProvider / removeProvider .-> d1 & d2 + d1["Dispatcher A"] -- connect(controllerA) --> src + d2["Dispatcher B"] -- connect(controllerB) --> src + src -. runExclusive(mutation) .-> d1 & d2 ``` -| Package | Role | -| -------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [`agent-dispatcher`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher) | Defines `AppAgentSource`, `AppAgentConnection`, `AppAgentHost`; implements the dispatcher-side host (the idle-gated applicator) and the low-level `AppAgentManager.addProvider`/`removeProvider`. | -| [`default-agent-provider`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider) | Implements `AppAgentSource`: record store, per-name tracker, fan-out registry, the `@package` app agent, and the update/uninstall barrier. | -| [`dispatcher-node-providers`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/nodeProviders) | Refcounted npm provider (`createNpmAppAgentProvider`) whose `isLoaded` the barrier reads to verify a version is fully released. | +| Package | Role | +| -------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [`dispatcher-types`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/types) | Defines the dependency-neutral `AppAgentProvider`, `AppAgentProviderSetController`, and callback-scoped `AppAgentProviderSetMutation` hosting contracts. | +| [`agent-dispatcher`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher) | Defines `AppAgentSource` and `AppAgentConnection`; implements the dispatcher-side controller and the low-level `AppAgentManager.addProvider`/`removeProvider`. | +| [`default-agent-provider`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider) | Implements `AppAgentSource`: record store, per-name tracker, fan-out registry, the `@package` app agent, and the update/uninstall barrier. | +| [`dispatcher-node-providers`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/nodeProviders) | Refcounted npm provider (`createNpmAppAgentProvider`) whose `isLoaded` the barrier reads to verify a version is fully released. | ## Design goals and non-goals @@ -70,7 +71,7 @@ The interfaces live in **`AppAgentSource`** is the dynamic set. Its dispatcher-facing interface is just: ```ts -connect(host: AppAgentHost): AppAgentConnection; +connect(controller: AppAgentProviderSetController): AppAgentConnection; ``` `connect()` returns a teardown handle plus a promise of the **shared** provider @@ -79,34 +80,33 @@ Providers are shared singletons: every `connect()` resolves with the same instance, so a loaded `AppAgent` is refcounted across all sessions rather than cloned per session. -**`AppAgentHost`** is the dispatcher-side callback the source uses to mutate one -session's live agent set. It is the _only_ interface the source touches — it never -reaches into grammars, collision detection, or the embedding cache: +**`AppAgentProviderSetController`** is the dispatcher-side capability the source uses to +mutate one session's live agent set. It exposes one operation: -- `addProvider(provider, notify?)` — register an agent into this session. -- `removeProvider(provider, notify?, dropConfig?)` — unload and drop an agent. -- `replaceProvider(oldProvider, resolveReplacement)` — the coordinated teardown/swap - primitive both `@update` and `@uninstall` fan out through (see - [Update coordination](#update-coordination)). +- `runExclusive(callback)` acquires the session's command lock and passes an + `AppAgentProviderSetMutation` to the callback. +- The mutation exposes `addProvider` and `removeProvider`. It is valid only while + that callback is active and rejects use after the callback returns. -Every op is applied through an **idle-gated FIFO applicator** and resolves when -the op is _applied_ — the ack the source's lifecycle tracker waits on. +The source composes install, uninstall, and replacement from these two mutation +primitives while the dispatcher holds the lock. Add/remove are not available on +the controller itself, so they cannot be called outside the exclusive section. ## The host ↔ source contract -The dispatcher and the source meet at exactly two interfaces — the dispatcher -implements `AppAgentHost`, the source implements `AppAgentSource` / +The dispatcher and the source meet at two interfaces: the dispatcher implements +`AppAgentProviderSetController`, and the source implements `AppAgentSource` / `AppAgentConnection` — and each side owes the other a small set of guarantees. A custom source (an embedder not using `default-agent-provider`) must uphold the right-hand column; the dispatcher upholds the left. -| Stage | The dispatcher (`AppAgentHost`) guarantees | The source (`AppAgentSource` / `AppAgentConnection`) must | -| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| **Connect** | calls `connect(host)` once per session, awaits the returned connection's `providers` under its held command lock, then registers them before serving | return **shared singleton** providers (the same instances on every connect) through `providers: Promise<...>` and an idempotent `dispose()`; record `host` for later fan-out | -| **Mutate** | applies `addProvider` / `removeProvider` / `replaceProvider` in **FIFO** order, gated on session idle, resolving each promise only once the op is **applied** | mutate a session **only** through its `AppAgentHost`; never reach into grammars, collision detection, or the dispatcher's context | -| **Coordinated swap** | runs `replaceProvider` as **one command-lock-held section** (remove → park on the replacement promise → add) so no request interleaves the swap | resolve `replaceProvider`'s replacement promise only after every host has quiesced **and** the old version's shared refcount is verified `0` (see [Update coordination](#update-coordination)) | -| **Leaf ops** | runs the teardown and startup legs under the held command lock | keep those legs **leaf ops** — process teardown/launch only, never dispatching a command or reacquiring the lock | -| **Teardown** | unregisters the providers from its own `AppAgentManager` and calls `dispose()` | stop fanning out to a host once its connection is disposed; a fan-out that raced `dispose()` must no-op | +| Stage | The dispatcher (`AppAgentProviderSetController`) guarantees | The source (`AppAgentSource` / `AppAgentConnection`) must | +| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **Connect** | calls `connect(controller)` once per session, awaits the returned connection's `providers` under its held command lock, then registers them before serving | return **shared singleton** providers (the same instances on every connect) through `providers: Promise<...>` and an idempotent `dispose()`; record `controller` for later fan-out | +| **Mutate** | exposes add/remove only through the temporary mutation passed to `runExclusive` and revokes it when the callback ends | mutate a session **only** inside `runExclusive`; never reach into grammars, collision detection, or the dispatcher's context | +| **Coordinated swap** | holds the command lock for the whole callback, so no request interleaves remove, barrier wait, and add | remove, quiesce, await the shared decision, and add the decided provider inside one callback; decide only after every controller quiesces and the old shared refcount is verified `0` | +| **Leaf ops** | runs the teardown and startup legs under the held command lock | keep those legs **leaf ops** — process teardown/launch only, never dispatching a command or reacquiring the lock | +| **Teardown** | unregisters the providers from its own `AppAgentManager` and closes the controller | stop fanning out to a controller once its connection is disposed; a fan-out that raced `dispose()` must no-op | The interface definitions in [`agentProvider.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts) @@ -125,7 +125,7 @@ owning agent's context: `@package …` resolves to the package agent, so its the dispatcher's `CommandHandlerContext`. A host handler therefore cannot cast its way into dispatcher internals. The one dispatcher access these handlers legitimately need, mutating the live session, arrives through the narrow -`AppAgentHost` on the package agent's own `agentContext`, which is per-dispatcher +`AppAgentProviderSetController` on the package agent's own `agentContext`, which is per-dispatcher by construction, so a handler automatically reaches _its_ session with no lookup. The package agent registers under the name `package`, is command-only (no @@ -190,7 +190,7 @@ entry so a fresh reinstall starts clean from the manifest default. Dispatchers are created and torn down dynamically (per conversation, with grace timers). The source must never fan out to a disposed dispatcher. -- **Connect** at context init: the dispatcher calls `source.connect(host)` for +- **Connect** at context init: the dispatcher calls `source.connect(controller)` for each injected `AppAgentSource`, awaits `connection.providers` under its held command lock, and registers the resolved providers so it neither loads a doomed version nor processes a command while an upgrading agent is mid-swap. @@ -215,7 +215,7 @@ by name and shared across dispatchers, **two versions of a name must never run a once** — an update is fundamentally a _restart of one shared process_, not an overlapping swap. The mechanism lives in the source barrier ([`defaultAgentProviders.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts)) -plus the `replaceProvider` primitive on `AppAgentHost`. +plus the scoped `AppAgentProviderSetController.runExclusive` primitive. ### What the swap must guarantee @@ -232,7 +232,7 @@ things true on every dispatcher at once: mismatch. Every update is therefore treated as potentially schema-changing and freezes all dispatchers together across the swap. -The slip happens if update is applied as two separate applicator operations: +The slip happens if update is applied as two separate lock acquisitions: `removeProvider(v1)` acquires and releases the command lock, then `addProvider(v2)` acquires it later after the shared process has drained. Any request that arrives in that released gap sees the name and schemas as absent. @@ -258,11 +258,10 @@ traffic. The command lock is the existing request boundary and keeps the routing seams (`isAppAgentName`, active schemas, and execution) consistent through one atomic local swap. -Structurally the held section is **`uninstall(v1)` immediately followed by -`install(v2)`** under one lock, so update reuses the uninstall/install primitives -rather than a custom state machine. Both `@update` and `@uninstall` fan out -through the one `replaceProvider(oldProvider, resolveReplacement)` primitive — -one op is one lock acquisition, so the whole freeze is a single awaitable unit. +Structurally the held section is **`removeProvider(v1)` followed by the barrier +wait and `addProvider(v2)`** inside one `runExclusive` callback. A rollback adds +v1 instead; a committed uninstall adds nothing. The source builds all three +outcomes from the same two mutation primitives. ### Sequence @@ -352,20 +351,25 @@ clean rollback. Because the swap spans a cross-session restart, `@update` and `@uninstall` cannot block the command lock waiting on it. The issuing session enqueues the op like a -sibling and returns _"update started"_; the terminal outcome is delivered later +sibling. Update immediately returns `unchanged` when the resolved package version +is already serving, or `started` with the old and new package versions when the +source provides them. A started swap's terminal outcome is delivered later through a one-shot `onOutcome(status)` callback (`updated`/`reverted` for update, `uninstalled`/`reverted` for uninstall) that the `@package` handler maps to a follow-up status line. This removes the inline "immediate" apply path; every -op on every dispatcher now flows through the one idle-gated queue. +swap on every dispatcher flows through the one idle-gated queue. ### Failure semantics and edge cases -- **Record write is the commit point.** Fan-out is best-effort notification after - it. -- **Issuing client** is awaited; failure is reported but the record is still - committed. -- **Sibling clients** each apply independently; a throw is caught and logged per - client. +- **Record write gates commit.** After every participating host removes v1 and + verify-0 passes, the source writes the committed record before publishing the + outcome. A write failure selects rollback, restores the v1 record, and makes + every host re-add v1. +- **Per-session config follows the net result.** Uninstall clears each session's + persisted enable preference only after its exclusive mutation ends as a net + removal. Rollback restores the same provider and leaves the preference intact. +- **Per-host apply failures** are caught and logged so a closing or broken host + cannot wedge the shared barrier. - **Leaf-op invariant:** teardown (`unload`/`close`) and startup (`load`/`init`) run under the held command lock and must be leaf ops — never dispatch a command or reacquire the lock, or the freeze would deadlock. @@ -383,16 +387,17 @@ op on every dispatcher now flows through the one idle-gated queue. ## Implementation map -| Concern | File | -| ------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `AppAgentHost` / `AppAgentSource` / `AppAgentConnection` interfaces | [`agentProvider.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts) | -| Idle-gated FIFO applicator | [`context/appAgentHost.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/context/appAgentHost.ts) | -| Applicator wiring, `commandLock`, connect/teardown | [`context/commandHandlerContext.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts) | -| `addProvider` / `removeProvider` / lazy load | [`context/appAgentManager.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts) | -| Refcounted npm provider (`isLoaded`, `close()`) | [`nodeProviders/.../npmAgentProvider.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/nodeProviders/src/agentProvider/npmAgentProvider.ts) | -| Per-name tracker + install/uninstall/update + barrier | [`defaultAgentProviders.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts) | -| `@package` app agent handlers | [`installSources/packageAgent.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/installSources/packageAgent.ts) | -| Installed-agent provider building + `agents.json` store | [`installSources/installedAgents.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/installSources/installedAgents.ts) | +| Concern | File | +| ---------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `AppAgentProvider` / `AppAgentProviderSetController` / `AppAgentProviderSetMutation` contracts | [`agentHosting.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/types/src/agentHosting.ts) | +| `AppAgentSource` / `AppAgentConnection` interfaces | [`agentProvider.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts) | +| Scoped controller implementation | [`context/appAgentSetController.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/context/appAgentSetController.ts) | +| Controller wiring, `commandLock`, connect/teardown | [`context/commandHandlerContext.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts) | +| `addProvider` / `removeProvider` / lazy load | [`context/appAgentManager.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/dispatcher/src/context/appAgentManager.ts) | +| Refcounted npm provider (`isLoaded`, `close()`) | [`nodeProviders/.../npmAgentProvider.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/dispatcher/nodeProviders/src/agentProvider/npmAgentProvider.ts) | +| Per-name tracker + install/uninstall/update + barrier | [`defaultAgentProviders.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts) | +| `@package` app agent handlers | [`installSources/packageAgent.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/installSources/packageAgent.ts) | +| Installed-agent provider building + `agents.json` store | [`installSources/installedAgents.ts`](https://github.com/microsoft/TypeAgent/blob/main/ts/packages/defaultAgentProvider/src/installSources/installedAgents.ts) | ## Related @@ -401,6 +406,6 @@ op on every dispatcher now flows through the one idle-gated queue. - [Embedding the dispatcher](../../guides/embedding-dispatcher.md) — wiring `appAgentProviders` and `appAgentSources` into a host. - [Dispatcher](../core/dispatcher.md) — command routing and the `AppAgentManager` - the applicator drives. + the controller drives. - [Agent-server conversations](../agents/agentServerConversations.md) — how one shared source serves many per-conversation dispatchers. diff --git a/ts/docs/guides/embedding-dispatcher.md b/ts/docs/guides/embedding-dispatcher.md index ce836d8ea..db20d312c 100644 --- a/ts/docs/guides/embedding-dispatcher.md +++ b/ts/docs/guides/embedding-dispatcher.md @@ -150,8 +150,9 @@ whatever table you return: import { createDispatcher, type AppAgentSource } from "agent-dispatcher"; const mySource: AppAgentSource = { - connect(host) { + connect(controller) { // return { providers: Promise.resolve([...]), dispose } — see AppAgentConnection + // Later changes use controller.runExclusive(mutation => ...). }, }; diff --git a/ts/packages/defaultAgentProvider/README.md b/ts/packages/defaultAgentProvider/README.md index 761e9104d..d2bc81ca6 100644 --- a/ts/packages/defaultAgentProvider/README.md +++ b/ts/packages/defaultAgentProvider/README.md @@ -68,7 +68,10 @@ move to the newest matching version (optionally constrained by a `` such as `^1.4`, `~2.0`, or `>=3 <4`), path agents refresh from their recorded path, and catalog agents are looked up again by short name. If the recorded source is no longer configured, the installed agent can still load at runtime, but update -fails until the source is added back or the agent is uninstalled. +fails until the source is added back or the agent is uninstalled. A feed update +reports the old and new package versions when its cross-session swap starts; if +the resolved version is already serving, it reports that immediately and skips +the swap. ### `@package uninstall ` diff --git a/ts/packages/defaultAgentProvider/package.json b/ts/packages/defaultAgentProvider/package.json index 5220be1c2..3dc7e6d2d 100644 --- a/ts/packages/defaultAgentProvider/package.json +++ b/ts/packages/defaultAgentProvider/package.json @@ -45,6 +45,7 @@ "@typeagent/aiclient": "workspace:*", "@typeagent/common-utils": "workspace:*", "@typeagent/config": "workspace:*", + "@typeagent/dispatcher-types": "workspace:*", "agent-cache": "workspace:*", "agent-dispatcher": "workspace:*", "browser-typeagent": "workspace:*", diff --git a/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts b/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts index 3c4dd022b..9a28de85a 100644 --- a/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts +++ b/ts/packages/defaultAgentProvider/src/defaultAgentProviders.ts @@ -5,8 +5,7 @@ import { AppAgentProvider, AppAgentSource, AppAgentConnection, - AppAgentHost, - InstalledAgentInfo, + AppAgentProviderSetController, IndexingServiceRegistry, DefaultIndexingServiceRegistry, DispatcherOptions, @@ -18,15 +17,17 @@ import { UpdateOutcomeStatus, UninstallOutcomeStatus, AGENT_INSTALL_ROOTS_SUBDIR, - AvailableInstallRow, - InstallMatchKind, InstallPreview, InstallPreviewMatch, InstallResult, + UpdateResult, deriveMatchKind, } from "./installSources/config.js"; import { createPackageAppAgentProvider, + AgentSourceGroup, + AvailableAgentInfo, + InstalledAgentInfo, InstalledAgentSourceApi, } from "./installSources/packageAgent.js"; @@ -288,7 +289,7 @@ type DynamicAgentEntry = /** * A source-coordinated teardown/swap barrier that either commits or, on a - * timeout or abort, rolls back. Every target host runs `replaceProvider`, tears + * timeout or abort, rolls back. Every target controller runs `runExclusive`, tears * down the shared old * (`v1`) version, and fills its slot via `quiesce`. Once every slot is filled and * verify-0 confirms the shared `v1` refcount is 0, the source commits — each @@ -314,7 +315,7 @@ type ReplaceBarrier = { readonly newProvider: AppAgentProvider | undefined; // Phase 1: hosts that have not yet quiesced (torn `v1` down). Empty ⇒ every // host removed `v1`. - readonly pending: Set; + readonly pending: Set; // Resolves (exactly once) when the barrier's outcome is decided (commit or // rollback), as a pure signal. A session that connects mid-`removing` is NOT // a participant (it never held `v1`, so it neither quiesces nor counts toward @@ -370,6 +371,8 @@ export function createDefaultInstalledAgentSource( options?: DefaultAppAgentSourceOptions, /** Host extension point for supplying alternate install sources. */ sourceFactory?: InstallSourceFactory, + /** @internal Test-only store writer for exercising commit failures. */ + storeWriter: typeof writeAgentsJson = writeAgentsJson, ): InstalledAgentSourceForTest { const instanceConfigs = getInstanceConfigProvider(instanceDir); const installDir = getInstallDir(instanceConfigs); @@ -382,6 +385,7 @@ export function createDefaultInstalledAgentSource( "Internal error: install directory could not be resolved (no instance directory).", ); } + const resolvedInstallDir = installDir; const sources = getResolvedInstallSources(instanceConfigs); // One shared limiter serializes the whole install op (resolve + materialize + // record write) and uninstall. @@ -443,7 +447,7 @@ export function createDefaultInstalledAgentSource( // // A draining name is never loaded on the normal path: throughout the // `removing` window every participant session holds its command lock (parked - // in `replaceProvider` awaiting `whenDecided`), and a session that connects + // in `runExclusive` awaiting `whenDecided`), and a session that connects // mid-`removing` parks on `whenDecided` before joining fan-out — so no // command, and therefore no `loadAppAgent`, runs against a draining name. function buildAgentProvider( @@ -451,9 +455,11 @@ export function createDefaultInstalledAgentSource( record: InstalledAgentRecord, ): AppAgentProvider { const loadRecord = registry.load(record); - // installDir is guaranteed resolved above (the source throws otherwise); - // the `!` bridges TS's lack of narrowing across this nested closure. - return createInstalledAppAgentProvider(name, loadRecord, installDir!); + return createInstalledAppAgentProvider( + name, + loadRecord, + resolvedInstallDir, + ); } // Build the shared provider for a freshly-resolved install/update record AND @@ -515,9 +521,9 @@ export function createDefaultInstalledAgentSource( return providers; } - // The client registry of connected AppAgentHosts, used for + // The client registry of connected app-agent-provider-set controllers, used for // cross-session fan-out. connect() adds; dispose() removes. - const clients = new Set(); + const clients = new Set(); // Per-name in-flight guard: per-name serialization lives // in the entry, not only in the global write limiter). A name is `busy` for @@ -528,19 +534,13 @@ export function createDefaultInstalledAgentSource( // drain of the same name. const busy = new Set(); - // Reject a mutating op on a name that is still draining - // name-reuse-during-removing): the name is off-limits until fully torn down. - function assertNotRemoving(name: string): void { + // Reject if the name is draining OR another op on it is in flight. + function assertNameFree(name: string): void { if (entries.get(name)?.status === "removing") { throw new Error( `Agent '${name}' is still being removed; retry shortly.`, ); } - } - - // Reject if the name is draining OR another op on it is in flight. - function assertNameFree(name: string): void { - assertNotRemoving(name); if (busy.has(name)) { throw new Error( `Agent '${name}' has an operation in progress; retry shortly.`, @@ -556,24 +556,6 @@ export function createDefaultInstalledAgentSource( return isLoadedProbe(barrier.oldProvider, barrier.name) !== true; } - // Cancel the live quiesce backstop timer. Idempotent. - function clearBarrierTimers(barrier: ReplaceBarrier): void { - if (barrier.quiesceTimer !== undefined) { - clearTimeout(barrier.quiesceTimer); - barrier.quiesceTimer = undefined; - } - } - - // The provider each host adds AFTER the barrier is decided. The - // source decides post-barrier: `v1` (the old provider) on a rollback so every - // session restores the exact version it had, `v2` (the new provider) on a - // committed update, or nothing (undefined) on a committed uninstall. - function decideAdd(barrier: ReplaceBarrier): AppAgentProvider | undefined { - return barrier.outcome === "rolledback" - ? barrier.oldProvider - : barrier.newProvider; - } - // Decide the barrier's outcome and unblock parked hosts / late joiners. // Runs exactly once (guarded by `outcome`): flips the entry (+ restores the // record on rollback), unblocks parked hosts / late joiners, then GCs the @@ -583,23 +565,32 @@ export function createDefaultInstalledAgentSource( if (barrier.outcome !== undefined) { return; } - barrier.outcome = outcome; - clearBarrierTimers(barrier); - // Flip source state BEFORE unblocking hosts (name active(v2)/absent on - // commit; active(v1) + record restored on rollback). A throw here (e.g. a - // synchronous agents.json write error during a rollback restore) must NOT - // skip `resolveDecided()` — the parked hosts would deadlock. They add the - // decided provider off `barrier.outcome` (via `decideAdd`), independent - // of the entry flip, so unblocking after a partial `onDecided` still - // restores the right version everywhere. + if (barrier.quiesceTimer !== undefined) { + clearTimeout(barrier.quiesceTimer); + barrier.quiesceTimer = undefined; + } + // Apply the source state and durable record before publishing the + // outcome. A commit write failure selects rollback, so hosts restore v1 + // instead of reporting success with runtime and agents.json disagreeing. try { barrier.onDecided(outcome); } catch (e) { debug( `barrier '${barrier.name}': onDecided(${outcome}) threw: ${e}`, ); + if (outcome === "committed") { + outcome = "rolledback"; + try { + barrier.onDecided(outcome); + } catch (rollbackError) { + debug( + `barrier '${barrier.name}': onDecided(${outcome}) threw: ${rollbackError}`, + ); + } + } } - // Unblock participant hosts parked in `replaceProvider` and late joiners + barrier.outcome = outcome; + // Unblock participant controllers parked in `runExclusive` and late joiners // parked in `connect()`. `onDecided` already flipped the entry, so late // joiners re-snapshot the decided version (`v2` commit / `v1` rollback / // absent on committed uninstall). @@ -634,10 +625,6 @@ export function createDefaultInstalledAgentSource( } } - function commit(barrier: ReplaceBarrier): void { - decide(barrier, "committed"); - } - // Roll back the swap: keep `v1`, discard `v2`. Triggered by the // quiesce timeout (a straggler that won't idle, or a `v1` that won't die). function rollback(barrier: ReplaceBarrier, reason: string): void { @@ -678,19 +665,22 @@ export function createDefaultInstalledAgentSource( // update / free the name on an uninstall). `v2`'s materialized manifest // was already structurally validated before the barrier was armed //, so there is nothing left to probe here. - commit(barrier); + decide(barrier, "committed"); } // Drop a host from a draining name's phase-1 barrier — a quiesce ACK, a // per-host teardown failure, or a disconnect. A disconnected // session has torn everything down, so it is treated exactly like a quiesce. // When the last slot fills, the barrier advances (verify-0 permitting). - function quiesce(name: string, host: AppAgentHost): void { + function quiesce( + name: string, + controller: AppAgentProviderSetController, + ): void { const entry = entries.get(name); if (entry?.status !== "removing") { return; } - if (!entry.barrier.pending.delete(host)) { + if (!entry.barrier.pending.delete(controller)) { return; } maybeAdvance(entry.barrier); @@ -699,9 +689,11 @@ export function createDefaultInstalledAgentSource( // The fan-out target set for a mutation: every connected session PLUS the // issuing one (which may not have formally connected). A fresh copy so a // concurrent connect/disconnect can't mutate it mid-iteration. - function fanOutTargets(issuingHost: AppAgentHost): Set { - const targets = new Set(clients); - targets.add(issuingHost); + function fanOutTargets( + issuingController: AppAgentProviderSetController, + ): Set { + const targets = new Set(clients); + targets.add(issuingController); return targets; } @@ -713,10 +705,7 @@ export function createDefaultInstalledAgentSource( excludeName: string, ): void { if (!isRootReferenced(instanceDir, root, excludeName)) { - // installDir is guaranteed resolved above (the source throws - // otherwise); the `!` bridges TS's lack of narrowing across this - // nested closure. - pruneAgentRoot(installDir!, root); + pruneAgentRoot(resolvedInstallDir, root); } } @@ -728,13 +717,13 @@ export function createDefaultInstalledAgentSource( ): void { const current = readAgentsJson(instanceDir) ?? { agents: {} }; mutate(current.agents); - writeAgentsJson(instanceDir, current); + storeWriter(instanceDir, current); } // Begin a coordinated teardown/swap across every connected session, - // time-bounded so a stall rolls it back. Every host - // — INCLUDING the issuing one — runs `replaceProvider` on its own idle-gated - // applicator: under a SINGLE held command lock it removes the old version, + // time-bounded so a stall rolls it back. Every controller, including the + // issuing one, runs one exclusive callback: under a SINGLE held command lock + // it removes the old version, // quiesces (fills its barrier slot), then awaits the shared `whenDecided` // before adding whatever the barrier decides — so no request interleaves the // swap on any session and no two versions coexist. Returns immediately once @@ -750,7 +739,7 @@ export function createDefaultInstalledAgentSource( function startReplace(params: { name: string; oldProvider: AppAgentProvider; - issuingHost: AppAgentHost; + issuingController: AppAgentProviderSetController; dropConfig: boolean; // The new version to add on commit; undefined for an uninstall. newProvider: AppAgentProvider | undefined; @@ -764,16 +753,16 @@ export function createDefaultInstalledAgentSource( const { name, oldProvider, - issuingHost, + issuingController, dropConfig, newProvider, onDecided, finalizeGc, onOutcome, } = params; - // The issuing host is always part of the barrier even if it never + // The issuing controller is always part of the barrier even if it never // formally connected (defensive); it is otherwise treated as a sibling. - const targets = fanOutTargets(issuingHost); + const targets = fanOutTargets(issuingController); let resolveDecided!: () => void; const whenDecided = new Promise((resolve) => { resolveDecided = resolve; @@ -803,51 +792,66 @@ export function createDefaultInstalledAgentSource( quiesceTimeoutMs, ); - for (const host of targets) { - host.replaceProvider( - oldProvider, - async () => { - quiesce(name, host); + async function replaceForController( + controller: AppAgentProviderSetController, + ): Promise { + try { + await controller.runExclusive(async (mutation) => { + await mutation.removeProvider(oldProvider, { + notify: true, + dropConfig, + }); + quiesce(name, controller); await whenDecided; - return decideAdd(barrier); - }, - true, - dropConfig, - ).then( - () => { - // A host that was already closed at enqueue time auto-acks - // without running the replacement resolver. Quiesce here too - // (idempotent — `pending.delete` guards it) so such a host - // fills its phase-1 slot from the success path and never - // wedges the barrier until the quiesce timeout. - quiesce(name, host); - }, - (e: unknown) => { - // A per-host teardown/add failure must not wedge the barrier: - // unblock phase 1 (if the remove leg threw). - debug(`replaceProvider failed for '${name}': ${e}`); - quiesce(name, host); - }, - ); + // Restore `v1` on rollback. On commit, add `v2` for an + // update or nothing for an uninstall. + const replacement = + barrier.outcome === "rolledback" + ? barrier.oldProvider + : barrier.newProvider; + if (replacement !== undefined) { + await mutation.addProvider(replacement, { + notify: true, + }); + } + }); + } catch (e) { + // A per-controller teardown/add failure must not wedge the + // barrier. The finally block fills phase 1 if remove failed. + debug(`exclusive replacement failed for '${name}': ${e}`); + } finally { + // A closed controller returns without running the callback. + // Quiesce here too. This is idempotent when the callback + // already quiesced after removing the old provider. + quiesce(name, controller); + } + } + + for (const controller of targets) { + void replaceForController(controller); } } // Fan out an add to every connected session: every - // host — INCLUDING the issuing one — enqueues the add on its own idle-gated - // applicator and is notified with a system message; none is applied inline + // controller - INCLUDING the issuing one - runs the add under its command + // lock and is notified with a system message; none is applied inline // under a held command lock. Each session derives the agent's enabled // state from its own config with the manifest default as fallback. - // A per-host throw is caught and logged, never failing the committed op. + // A per-controller throw is caught and logged, never failing the committed op. // Returns immediately; each add lands at that session's next idle. function fanOutAdd( provider: AppAgentProvider, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, ): void { - const targets = fanOutTargets(issuingHost); - for (const host of targets) { - host.addProvider(provider, true).catch((e) => { - debug(`addProvider failed: ${e}`); - }); + const targets = fanOutTargets(issuingController); + for (const controller of targets) { + controller + .runExclusive((mutation) => + mutation.addProvider(provider, { notify: true }), + ) + .catch((e) => { + debug(`addProvider failed: ${e}`); + }); } } @@ -856,11 +860,12 @@ export function createDefaultInstalledAgentSource( nameOrTarget: string, ref: string | undefined, sourceName: string | undefined, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, onStatus?: SourceStatus, abortSignal?: AbortSignal, ): Promise { const explicit = ref !== undefined; + let busyName: string | undefined; // Explicit (two-argument) mode knows the installed name up front, so // fail fast on a built-in / busy / draining name before resolving. if (explicit) { @@ -871,8 +876,8 @@ export function createDefaultInstalledAgentSource( } assertNameFree(nameOrTarget); busy.add(nameOrTarget); + busyName = nameOrTarget; } - let inferredBusy: string | undefined; try { // resolve + materialize is serialized by the registry's limiter. // In infer mode this derives the installed name from the resolved @@ -901,7 +906,7 @@ export function createDefaultInstalledAgentSource( } assertNameFree(name); busy.add(name); - inferredBusy = name; + busyName = name; } // Build the shared per-agent provider AND structurally validate // its freshly-materialized manifest BEFORE persisting: a @@ -929,7 +934,7 @@ export function createDefaultInstalledAgentSource( entries.set(name, { status: "active", provider }); // Fan out the add to every connected session — including the // issuing one — through each session's idle-gated applicator. - fanOutAdd(provider, issuingHost); + fanOutAdd(provider, issuingController); const result: InstallResult = { name, source: record.source, @@ -958,17 +963,14 @@ export function createDefaultInstalledAgentSource( } return result; } finally { - if (inferredBusy !== undefined) { - busy.delete(inferredBusy); - } - if (explicit) { - busy.delete(nameOrTarget); + if (busyName !== undefined) { + busy.delete(busyName); } } }, async uninstall( name: string, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, onOutcome?: (status: UninstallOutcomeStatus) => void, ): Promise { if (isBuiltin(name)) { @@ -1006,16 +1008,15 @@ export function createDefaultInstalledAgentSource( ); } const oldProvider = entry.provider; - // The barrier decision is the commit point. Now tear - // the live agent down across every connected session through the + // Tear the live agent down across every connected session through the // coordinated barrier (active → removing → absent): each - // session's `replaceProvider` (no new-version + // session's `runExclusive` callback (no new-version // thunk) unloads under one held command lock, then the name is // freed only once verify-0 confirms the shared process is down // everywhere. The name stays off-limits until the barrier - // completes. Uninstall drops each session's persisted enable - // preference (dropConfig=true) so a fresh reinstall starts from - // the manifest default. Once confirmed down, + // completes. A committed uninstall drops each session's + // persisted enable preference so a fresh reinstall starts from + // the manifest default; rollback preserves it. Once confirmed down, // the agent's version-scoped install root is pruned; the startup // orphan sweep is the backstop. If a straggler // never idles the barrier times out and ROLLS BACK: @@ -1024,7 +1025,7 @@ export function createDefaultInstalledAgentSource( startReplace({ name, oldProvider, - issuingHost, + issuingController, dropConfig: true, newProvider: undefined, onDecided: (outcome) => { @@ -1035,19 +1036,23 @@ export function createDefaultInstalledAgentSource( // dropped ONLY here (not before the barrier): while // the teardown is in flight the agent stays the // recorded-current install, so a crash mid-uninstall - // recovers to the still-installed agent and a rollback - // needs nothing restored. + // recovers to the still-installed agent. entries.delete(name); mutateAgentsJson((agents) => { delete agents[name]; }); } else { - // Rollback: the record was never dropped, so there - // is nothing to restore — just keep v1 live. + // Restore both source state and the durable v1 record. + // The write is normally idempotent, and also repairs a + // commit write that failed after partially replacing + // agents.json. entries.set(name, { status: "active", provider: oldProvider, }); + mutateAgentsJson((agents) => { + agents[name] = deletedRecord; + }); } }, finalizeGc: (outcome) => { @@ -1084,9 +1089,9 @@ export function createDefaultInstalledAgentSource( async update( name: string, range: string | undefined, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, onOutcome?: (status: UpdateOutcomeStatus) => void, - ): Promise { + ): Promise { if (isBuiltin(name)) { throw new Error( `Agent '${name}' is built-in and cannot be updated`, @@ -1104,6 +1109,12 @@ export function createDefaultInstalledAgentSource( if (existing === undefined) { throw new Error(`Agent '${name}' not found`); } + const oldEntry = entries.get(name); + if (oldEntry?.status !== "active") { + throw new Error( + `Agent '${name}' is recorded but not active in this source; the install store may have been modified out of band.`, + ); + } const updateResult = await registry.update(existing, { range, }); @@ -1133,16 +1144,11 @@ export function createDefaultInstalledAgentSource( // pick up an in-place manifest edit. if (updateResult.status === "no-op") { writeInstalledRecord(); - // Nothing swapped in any session, so the cross-session - // fan-out has nothing to announce; report the no-op to the - // issuing conversation directly so `@package update` on an - // already-current agent is not silent. - onOutcome?.("unchanged"); - return; + return { status: "unchanged" }; } // Coordinated update: tear the OLD // version down across every session and add the NEW one as ONE - // coordinated barrier — each session's `replaceProvider` removes + // coordinated barrier - each session's `runExclusive` callback removes // v1 then (after verify-0 confirms v1 is down everywhere) adds v2, // all under one held command lock, so no two versions of the name // ever coexist and no session observes it absent. No-coexistence @@ -1154,7 +1160,6 @@ export function createDefaultInstalledAgentSource( // is time-bounded: a straggler that won't idle or a v1 that won't // die ROLLS BACK to v1 — v2 is discarded and v1 stays // the recorded-current version, as if it never happened. - const oldEntry = entries.get(name); // Build v2's provider AND structurally validate its // freshly-materialized manifest while v1 is still live: a // corrupt/unresolvable v2 — from ANY source (feed, @@ -1177,76 +1182,74 @@ export function createDefaultInstalledAgentSource( // a no-op handled above and never reaches the barrier. const oldRoot = existing.installRoot; const newRoot = record.installRoot; - if (oldEntry?.status === "active") { - startReplace({ - name, - oldProvider: oldEntry.provider, - issuingHost, - dropConfig: false, - newProvider, - onDecided: (outcome) => { - if (outcome === "committed") { - // Flip in-memory FIRST (never throws) so the name - // is never stranded in `removing` (the tombstone - // would otherwise brick it), then persist v2 — the - // durable commit point. If that write - // throws, v1 stays the recorded-current version and - // the finalizeGc guard below keeps v1's root. - entries.set(name, { - status: "active", - provider: newProvider, - }); - writeInstalledRecord(); - } else { - // Rollback: v1 was never overwritten in the store, - // so there is nothing to restore — just keep v1 - // live and discard v2, as if the update never - // happened. - entries.set(name, { - status: "active", - provider: oldEntry.provider, - }); - } - }, - finalizeGc: (outcome) => { - // Prune the superseded root only after every host has - // swapped: v1 is never pruned before v2 is - // serving). Commit discards v1 — but only once v2 is - // actually the persisted record, so a commit-write that - // failed (leaving v1 recorded) keeps v1's root and - // orphans v2 for the startup sweep. Rollback discards - // v2 (v1 stays recorded + serving). - if (outcome === "committed") { - const committed = - readAgentsJson(instanceDir)?.agents[name]; - if ( - committed?.installRoot === newRoot && - oldRoot !== newRoot - ) { - pruneRootIfUnreferenced(oldRoot, name); - } - } else if (oldRoot !== newRoot) { - pruneRootIfUnreferenced(newRoot, name); + startReplace({ + name, + oldProvider: oldEntry.provider, + issuingController, + dropConfig: false, + newProvider, + onDecided: (outcome) => { + if (outcome === "committed") { + // Flip in-memory FIRST (never throws) so the name + // is never stranded in `removing` (the tombstone + // would otherwise brick it), then persist v2 — the + // durable commit point. If that write + // throws, v1 stays the recorded-current version and + // the finalizeGc guard below keeps v1's root. + entries.set(name, { + status: "active", + provider: newProvider, + }); + writeInstalledRecord(); + } else { + // Restore both source state and the durable v1 record. + // The write is normally idempotent, and also repairs a + // commit write that failed after partially replacing + // agents.json. + entries.set(name, { + status: "active", + provider: oldEntry.provider, + }); + mutateAgentsJson((agents) => { + agents[name] = existing; + }); + } + }, + finalizeGc: (outcome) => { + // Prune the superseded root only after every host has + // swapped: v1 is never pruned before v2 is + // serving). Commit discards v1 — but only once v2 is + // actually the persisted record, so a commit-write that + // failed (leaving v1 recorded) keeps v1's root and + // orphans v2 for the startup sweep. Rollback discards + // v2 (v1 stays recorded + serving). + if (outcome === "committed") { + const committed = + readAgentsJson(instanceDir)?.agents[name]; + if ( + committed?.installRoot === newRoot && + oldRoot !== newRoot + ) { + pruneRootIfUnreferenced(oldRoot, name); } - }, - onOutcome, - }); - } else { - // No live old version to tear down: commit v2 directly (write - // the record + add to every session); there is no barrier - // (nothing to coordinate), so report the final status - // directly. - writeInstalledRecord(); - entries.set(name, { - status: "active", - provider: newProvider, - }); - if (oldRoot !== record.installRoot) { - pruneRootIfUnreferenced(oldRoot, name); - } - fanOutAdd(newProvider, issuingHost); - onOutcome?.("updated"); - } + } else if (oldRoot !== newRoot) { + pruneRootIfUnreferenced(newRoot, name); + } + }, + onOutcome, + }); + return { + status: "started", + ...(updateResult.packageName !== undefined + ? { packageName: updateResult.packageName } + : {}), + ...(updateResult.oldVersion !== undefined + ? { oldVersion: updateResult.oldVersion } + : {}), + ...(updateResult.newVersion !== undefined + ? { newVersion: updateResult.newVersion } + : {}), + }; } finally { busy.delete(name); } @@ -1266,7 +1269,7 @@ export function createDefaultInstalledAgentSource( }, }); }, - listInstalled(): InstalledAgentInfo[] { + listInstalled(): AgentSourceGroup[] { // The source owns only mutable install records (`agents.json`). // Bundled agents are provided separately by the bundled provider and // are intentionally excluded from these install summaries. A record @@ -1274,18 +1277,43 @@ export function createDefaultInstalledAgentSource( // name that is currently `removing` (draining) is hidden — it is not // an installed agent anymore. const agents = readAgentsJson(instanceDir)?.agents ?? {}; - return Object.values(agents) - .filter( - (record) => entries.get(record.name)?.status !== "removing", - ) - .map((record) => { - const ref = record.ref ?? record.module ?? record.path; - return { - name: record.name, - source: record.source, - ...(ref !== undefined ? { ref } : {}), - }; + const agentsBySource = new Map(); + for (const record of Object.values(agents)) { + if (entries.get(record.name)?.status === "removing") { + continue; + } + const ref = record.ref ?? record.module ?? record.path; + const info = { + name: record.name, + ...(ref !== undefined ? { ref } : {}), + }; + const group = agentsBySource.get(record.source); + if (group === undefined) { + agentsBySource.set(record.source, [info]); + } else { + group.push(info); + } + } + + const groups: AgentSourceGroup[] = []; + for (const info of registry.list()) { + const sourceAgents = agentsBySource.get(info.name); + if (sourceAgents === undefined) { + continue; + } + groups.push({ + source: info.name, + sourceKind: info.kind, + agents: sourceAgents, }); + agentsBySource.delete(info.name); + } + // Keep records from removed source configurations visible after all + // currently configured sources. + for (const [sourceName, sourceAgents] of agentsBySource) { + groups.push({ source: sourceName, agents: sourceAgents }); + } + return groups; }, listSources(): string[] { // Source names in resolution order, for `@package install --source` @@ -1294,10 +1322,10 @@ export function createDefaultInstalledAgentSource( }, async listAvailableAgents(opts?: { sourceName?: string; - }): Promise { - // Source-aware install rows for `@package available` and filtered - // completion in `@package install`. - const rows: AvailableInstallRow[] = []; + }): Promise[]> { + // Source groups for `@package available` and filtered completion in + // `@package install`. + const groups: AgentSourceGroup[] = []; for (const info of registry.list()) { if ( opts?.sourceName !== undefined && @@ -1310,12 +1338,29 @@ export function createDefaultInstalledAgentSource( continue; } try { - rows.push(...(await src.listAgents())); + const sourceAgents = (await src.listAgents()).map( + ({ ref, defaultAgentName, packageName }) => ({ + ref, + ...(defaultAgentName !== undefined + ? { defaultAgentName } + : {}), + ...(packageName !== undefined + ? { packageName } + : {}), + }), + ); + if (sourceAgents.length > 0) { + groups.push({ + source: info.name, + sourceKind: info.kind, + agents: sourceAgents, + }); + } } catch (e) { debug(`listAgents failed for source '${info.name}': ${e}`); } } - return rows; + return groups; }, async preview( nameOrTarget: string, @@ -1338,36 +1383,34 @@ export function createDefaultInstalledAgentSource( const toMatch = (m: PreviewMatch): InstallPreviewMatch => { // The registry only commits to name-vs-ref; the finer label is // derived here from the resolved candidate's own fields. - const matchKind: InstallMatchKind = deriveMatchKind({ + const matchKind = deriveMatchKind({ matchedByName: m.matchedByName, path: m.candidate.path, }); - const im: { - source: string; - sourceKind?: string; - matchKind: InstallMatchKind; - name: string; - packageName?: string; - path?: string; - ref?: string; - } = { source: m.source, matchKind, name: m.name }; + const match: { + -readonly [K in keyof InstallPreviewMatch]: InstallPreviewMatch[K]; + } = { + source: m.source, + matchKind, + name: m.name, + }; const sourceKind = registry.get(m.source)?.kind; if (sourceKind !== undefined) { - im.sourceKind = sourceKind; + match.sourceKind = sourceKind; } if (m.candidate.packageName !== undefined) { - im.packageName = m.candidate.packageName; + match.packageName = m.candidate.packageName; } if (m.candidate.path !== undefined) { - im.path = m.candidate.path; + match.path = m.candidate.path; } if ( m.candidate.module !== undefined && m.candidate.ref !== undefined ) { - im.ref = m.candidate.ref; + match.ref = m.candidate.ref; } - return im; + return match; }; return { winner: toMatch(result.winner), @@ -1382,16 +1425,15 @@ export function createDefaultInstalledAgentSource( }; // The dispatcher-facing AppAgentSource API is connect(); the write API is - // captured by the per-session `@package` agent below. The - // concrete object keeps an unadvertised test handle for focused unit tests, - // but the exported constructor returns only AppAgentSource. + // captured by the per-session `@package` agent below. The concrete object + // also exposes a handle for focused unit tests. const appAgentSource: InstalledAgentSourceForTest = { testApi: source, - connect(host: AppAgentHost): AppAgentConnection { + connect(controller: AppAgentProviderSetController): AppAgentConnection { // The package agent is per-connection (its agentContext carries this - // session's AppAgentHost); the installed providers are shared. + // session's app-agent-provider-set controller); the installed providers are shared. const packageProvider = createPackageAppAgentProvider({ - appAgentHost: host, + appAgentProviderSetController: controller, source, }); // Torn down before the initial set resolved: a connection disposed @@ -1432,7 +1474,7 @@ export function createDefaultInstalledAgentSource( // that installs an agent while we parked lands it in this // snapshot; one that installs after we join gets it via // fan-out — exactly once either way. - clients.add(host); + clients.add(controller); return [packageProvider, ...activeProviders()]; } await Promise.all( @@ -1451,7 +1493,7 @@ export function createDefaultInstalledAgentSource( // it never finished joining). Does NOT tear down the shared // providers — other sessions still hold them; the dispatcher // unregisters them from its own manager at teardown. - clients.delete(host); + clients.delete(controller); // Disconnect while a teardown/swap is in flight: a gone // session has removed everything, so drop it // from every barrier's pending set (which may complete one). @@ -1460,7 +1502,7 @@ export function createDefaultInstalledAgentSource( // started); a barrier it merely parked on never listed it in // `pending`, so the `quiesce` there is a no-op. for (const name of [...entries.keys()]) { - quiesce(name, host); + quiesce(name, controller); // Re-poll verify-0 even when this host had already left // `pending`. The dispatcher tears this session's agents // down — dropping the shared `v1` refcount — BEFORE it diff --git a/ts/packages/defaultAgentProvider/src/installSources/config.ts b/ts/packages/defaultAgentProvider/src/installSources/config.ts index 47df21026..7d8065a64 100644 --- a/ts/packages/defaultAgentProvider/src/installSources/config.ts +++ b/ts/packages/defaultAgentProvider/src/installSources/config.ts @@ -5,7 +5,7 @@ // `InstallSourceInfo`, and the install-record shapes (`ResolvedCandidate` / // `InstalledAgentRecord`). These live in the host (default-agent-provider), not // the dispatcher core: the core knows nothing about how sources are configured, -// listed, resolved, or recorded - it only exposes the per-session `AppAgentHost` +// listed, resolved, or recorded - it only exposes the per-session controller // the host uses to register and tear down agents. The host contributes the whole // `@package` command table (`@package install`/`uninstall`/`update`/`list`, with // the source table nested as `@package source`) via @@ -148,6 +148,21 @@ export interface InstallResult { warnings?: string[]; } +/** + * The immediate disposition of an `@package update` request. `unchanged` + * means the resolved package version is already serving. `started` means the + * cross-session swap was armed and will settle asynchronously; package version + * details are present when the source can determine them. + */ +export type UpdateResult = + | { status: "unchanged" } + | { + status: "started"; + packageName?: string; + oldVersion?: string; + newVersion?: string; + }; + /** * The subdirectory under `installDir` that holds the per-agent, version-scoped * install roots. Each feed install materializes into its own @@ -214,7 +229,13 @@ export interface ResolveResult { * concrete version). */ export type InstallSourceUpdateResult = - | { status: "updated"; record: MaterializedInstallRecord } + | { + status: "updated"; + record: MaterializedInstallRecord; + packageName?: string; + oldVersion?: string; + newVersion?: string; + } | { status: "no-op"; record: MaterializedInstallRecord }; /** @@ -238,16 +259,16 @@ export type SourceStatus = (message: string) => void; /** * The terminal outcome the issuing conversation is told about after a coordinated - * `@package update` settles asynchronously. `updated` = the swap + * `@package update` swap settles asynchronously. `updated` = the swap * committed to `v2`; `reverted` = a phase timeout (a straggler that would not - * idle, or a `v2` that would not start) rolled back to `v1`, leaving `v1` serving - * in every session; `unchanged` = the requested version was already the installed, - * serving version, so nothing was swapped (a no-op the fan-out cannot express). - * A committed `updated` is announced by the source's cross-session fan-out (like - * an install's add); `reverted` and `unchanged` change no live session state, so - * the fan-out is silent and only the issuing conversation is told. + * idle, or a `v2` that would not start) rolled back to `v1`, leaving `v1` + * serving in every session. An unchanged request is returned immediately as an + * {@link UpdateResult} and never enters the barrier. A committed `updated` is + * announced by the source's cross-session fan-out (like an install's add); + * `reverted` changes no live session state, so only the issuing conversation is + * told. */ -export type UpdateOutcomeStatus = "updated" | "reverted" | "unchanged"; +export type UpdateOutcomeStatus = "updated" | "reverted"; /** * The terminal outcome the issuing conversation is told about after a coordinated diff --git a/ts/packages/defaultAgentProvider/src/installSources/feedSource.ts b/ts/packages/defaultAgentProvider/src/installSources/feedSource.ts index b85d8b9fe..e3e20b643 100644 --- a/ts/packages/defaultAgentProvider/src/installSources/feedSource.ts +++ b/ts/packages/defaultAgentProvider/src/installSources/feedSource.ts @@ -132,6 +132,18 @@ function installRootFor(moduleName: string, version: string): string { return `${sanitizeLabel(moduleName)}@${version}`; } +function versionFromInstallRoot( + moduleName: string, + installRoot: string | undefined, +): string | undefined { + const prefix = `${sanitizeLabel(moduleName)}@`; + if (installRoot === undefined || !installRoot.startsWith(prefix)) { + return undefined; + } + const version = installRoot.slice(prefix.length); + return isConcreteVersion(version) ? version : undefined; +} + // A short, unique, filesystem-safe install-id. Used to // name the TEMPORARY install root (`.tmp-`) a slow-path materialize installs // into before atomically adopting it as the content-addressed `module@version` @@ -873,9 +885,18 @@ export function createFeedSource( record: noOpRecord, }; } + const oldVersion = versionFromInstallRoot( + moduleName, + record.installRoot, + ); return { status: "updated" as const, record: await this.materialize(candidate), + packageName: moduleName, + ...(oldVersion !== undefined ? { oldVersion } : {}), + ...(candidate.version !== undefined + ? { newVersion: candidate.version } + : {}), }; }, async materialize( diff --git a/ts/packages/defaultAgentProvider/src/installSources/packageAgent.ts b/ts/packages/defaultAgentProvider/src/installSources/packageAgent.ts index 314482b81..e7c5ede0c 100644 --- a/ts/packages/defaultAgentProvider/src/installSources/packageAgent.ts +++ b/ts/packages/defaultAgentProvider/src/installSources/packageAgent.ts @@ -22,13 +22,11 @@ import { displayWarn, } from "@typeagent/agent-sdk/helpers/display"; import { - AppAgentHost, AppAgentProvider, - InstalledAgentInfo, + AppAgentProviderSetController, } from "agent-dispatcher"; import chalk from "chalk"; import { - AvailableInstallRow, InstallMatchKind, InstallPreview, InstallResult, @@ -36,17 +34,39 @@ import { SourceStatus, UninstallOutcomeStatus, UpdateOutcomeStatus, + UpdateResult, } from "./config.js"; // A legal dispatcher agent identifier (matches existing agent names such as // "github-cli", "osNotifications"). const AGENT_NAME_RE = /^[A-Za-z][A-Za-z0-9_-]*$/; +/** A host-rendered summary of one installed agent for `@package list`. */ +export interface InstalledAgentInfo { + name: string; + // Feed specifier, package name, or path. Omitted when none is recorded. + ref?: string; +} + +/** One install target advertised by a source. */ +export interface AvailableAgentInfo { + readonly ref: string; + readonly defaultAgentName?: string | undefined; + readonly packageName?: string | undefined; +} + +/** Agent information grouped under one configured install source. */ +export interface AgentSourceGroup { + readonly source: string; + readonly sourceKind?: string | undefined; + readonly agents: T[]; +} + /** * The record-store / registry API the `@package` handlers use, supplied by * the host's `AppAgentSource`. All `agents.json` access, source resolution, and * the cross-session fan-out live behind this, so the handlers never touch the - * dispatcher's internals. Each mutating op takes the `issuingHost` (the session + * dispatcher's internals. Each mutating op takes the `issuingController` (the session * that ran the command, reached off the package agent's own `agentContext`) so * the source can register/tear down the agent in the issuing session (awaited) * while fanning out to the other sessions best-effort as a follow-up. @@ -68,7 +88,7 @@ export interface InstalledAgentSourceApi { nameOrTarget: string, ref: string | undefined, sourceName: string | undefined, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, onStatus?: SourceStatus, abortSignal?: AbortSignal, ): Promise; @@ -96,7 +116,7 @@ export interface InstalledAgentSourceApi { // the unload lands at each session's next idle. uninstall( name: string, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, onOutcome?: (status: UninstallOutcomeStatus) => void, ): Promise; // Re-materialize against the recorded source (fails fast on error), write the @@ -109,24 +129,24 @@ export interface InstalledAgentSourceApi { // issuing one — so this returns as soon as the record is committed. A // COMMITTED swap is announced by the cross-session fan-out ("Agent 'x' was // updated."), exactly as install announces an add, so callers need not echo - // it. `onOutcome` reports the final status: `updated` (committed), `reverted` - // (a rollback to v1 on timeout/failed-start), or `unchanged` (the requested - // version was already serving — a no-op the fan-out cannot express). + // it. Returns the immediate disposition: `unchanged`, or `started` with + // package version details when available. `onOutcome` reports the later + // barrier status: `updated` (committed) or `reverted` (rolled back). update( name: string, range: string | undefined, - issuingHost: AppAgentHost, + issuingController: AppAgentProviderSetController, onOutcome?: (status: UpdateOutcomeStatus) => void, - ): Promise; - // Host-rendered summaries of installed agents, backing `@package list`. - listInstalled(): InstalledAgentInfo[]; + ): Promise; + // Host-rendered summaries of installed agents, grouped in source order. + listInstalled(): AgentSourceGroup[]; // Source names in resolution order (for `@package install --source`). listSources(): string[]; - // Enumerable install rows (default agent name + package name) with source - // names. Optional source filter narrows results to one source. + // Enumerable install targets grouped in source order. Optional source + // filter narrows results to one source. listAvailableAgents(opts?: { sourceName?: string; - }): Promise; + }): Promise[]>; // The host-owned source command table, nested under `@package source`. sourceCommands(): CommandHandlerTable; } @@ -134,12 +154,12 @@ export interface InstalledAgentSourceApi { /** * The host-owned `agentContext` of the `@package` app agent. It is not the * dispatcher's `CommandHandlerContext`: the only dispatcher access it exposes - * is the narrow {@link AppAgentHost} (to register/tear down agents in + * is the narrow {@link AppAgentProviderSetController} (to register/tear down agents in * the issuing session), plus the host's own {@link InstalledAgentSourceApi} * closures. So a handler can never reach back into dispatcher internals. */ export interface PackageAgentContext { - readonly appAgentHost: AppAgentHost; + readonly appAgentProviderSetController: AppAgentProviderSetController; readonly source: InstalledAgentSourceApi; } @@ -148,7 +168,46 @@ type PackageSessionContext = SessionContext; // Names of agents that can be uninstalled/updated. function managedAgentNames(context: PackageSessionContext): string[] { - return context.agentContext.source.listInstalled().map((info) => info.name); + return context.agentContext.source + .listInstalled() + .flatMap((group) => group.agents.map((info) => info.name)); +} + +function displaySourceTables( + context: PackageActionContext, + groups: AgentSourceGroup[], + columns: string[], + compareRows: (a: T, b: T) => number, + formatRow: (row: T) => string[], +): void { + groups.forEach((group, index) => { + const sourceKind = group.sourceKind; + const sourceHeading = + sourceKind !== undefined + ? `${group.source} (${sourceKind})` + : group.source; + context.actionIO.appendDisplay( + { + type: "text", + content: chalk.yellow( + `${index === 0 ? "" : "\n"}${sourceHeading}\n`, + ), + }, + "block", + ); + const text: string[][] = [columns]; + group.agents.sort(compareRows); + for (const row of group.agents) { + text.push(formatRow(row)); + } + context.actionIO.appendDisplay( + { + type: "text", + content: text, + }, + "block", + ); + }); } class ListInstalledCommandHandler implements CommandHandler { @@ -160,54 +219,36 @@ class ListInstalledCommandHandler implements CommandHandler { ) { const { source } = context.sessionContext.agentContext; // `@package list` shows mutable installed records only. - const records = source.listInstalled(); - if (records.length === 0) { + const groups = source.listInstalled(); + if (groups.length === 0) { displayResult("No installed agents found.", context); return; } - // Group records by source so each source renders as its own table. - const groups = new Map(); - for (const record of records) { - const group = groups.get(record.source); - if (group === undefined) { - groups.set(record.source, [record]); - } else { - group.push(record); - } - } + // Preserve source order, matching `@package available`. Sort agents + // within each source and render each heading and table as a block. + displaySourceTables( + context, + groups, + ["Name", "Reference"], + (a, b) => a.name.localeCompare(b.name), + (record) => [ + chalk.cyanBright(record.name), + record.ref !== undefined + ? chalk.gray(record.ref) + : chalk.gray("—"), + ], + ); - // Sources listed alphabetically; one table per source, with a heading. - const sortedSources = [...groups.keys()].sort(); - sortedSources.forEach((source, index) => { - const groupRecords = groups - .get(source)! - .sort((a, b) => a.name.localeCompare(b.name)); - context.actionIO.appendDisplay({ - type: "text", - content: chalk.yellow(`${index === 0 ? "" : "\n"}${source}\n`), - }); - const text: string[][] = [["Agent", "Reference"]]; - for (const record of groupRecords) { - text.push([ - chalk.cyanBright(record.name), - record.ref !== undefined - ? chalk.gray(record.ref) - : chalk.gray("—"), - ]); - } - context.actionIO.appendDisplay({ + context.actionIO.appendDisplay( + { type: "text", - content: text, - }); - }); - - context.actionIO.appendDisplay({ - type: "text", - content: chalk.gray( - "\nShowing installable installed agents only. Use '@config agent' to see all available agents and their status.", - ), - }); + content: chalk.gray( + "\nShowing installable installed agents only. Use '@config agent' to see all available agents and their status.", + ), + }, + "block", + ); } } @@ -241,36 +282,30 @@ class ListAvailableCommandHandler implements CommandHandler { displayStatus("Refreshing source metadata...", context); await source.refresh(sourceName); } - const rows = ( - await source.listAvailableAgents( - sourceName !== undefined ? { sourceName } : undefined, - ) - ).sort( - (a, b) => - (a.defaultAgentName ?? a.packageName ?? "").localeCompare( - b.defaultAgentName ?? b.packageName ?? "", - ) || a.source.localeCompare(b.source), + const groups = await source.listAvailableAgents( + sourceName !== undefined ? { sourceName } : undefined, ); - if (rows.length === 0) { + if (groups.length === 0) { displayResult("No installable agents found.", context); return; } // Show only what can be typed into `@package install`: the default agent // name and the package name. The internal catalog key / durable ref is - // never displayed. - const text: string[][] = [["Name", "Package", "Source"]]; - for (const row of rows) { - text.push([ + // never displayed. Keep unrelated catalogs and feeds in separate tables. + displaySourceTables( + context, + groups, + ["Name", "Package"], + (a, b) => + (a.defaultAgentName ?? a.packageName ?? "").localeCompare( + b.defaultAgentName ?? b.packageName ?? "", + ), + (row) => [ chalk.cyanBright(row.defaultAgentName ?? "—"), row.packageName ? chalk.gray(row.packageName) : chalk.gray("—"), - chalk.gray(row.source), - ]); - } - context.actionIO.appendDisplay({ - type: "text", - content: text, - }); + ], + ); } public async getCompletion( @@ -364,7 +399,10 @@ class InstallCommandHandler implements CommandHandler { context: PackageActionContext, params: ParsedCommandParams, ) { - const { appAgentHost, source } = context.sessionContext.agentContext; + const { + appAgentProviderSetController: appAgentProviderSetController, + source, + } = context.sessionContext.agentContext; const { args, flags } = params; const { target, name } = args; const sourceName = flags.source ?? undefined; @@ -434,7 +472,7 @@ class InstallCommandHandler implements CommandHandler { nameOrTarget, ref, sourceName, - appAgentHost, + appAgentProviderSetController, (message) => displayStatus(message, context), context.abortSignal, ); @@ -485,16 +523,18 @@ class InstallCommandHandler implements CommandHandler { // Complete default agent names and package names. The second // argument (explicit installed name) is not completed. const sourceName = params.flags?.source as string | undefined; - const rows = await source.listAvailableAgents( + const groups = await source.listAvailableAgents( sourceName !== undefined ? { sourceName } : undefined, ); const values = new Set(); - for (const row of rows) { - if (row.defaultAgentName !== undefined) { - values.add(row.defaultAgentName); - } - if (row.packageName !== undefined) { - values.add(row.packageName); + for (const group of groups) { + for (const agent of group.agents) { + if (agent.defaultAgentName !== undefined) { + values.add(agent.defaultAgentName); + } + if (agent.packageName !== undefined) { + values.add(agent.packageName); + } } } completions.push({ name, completions: [...values] }); @@ -523,7 +563,10 @@ class UninstallCommandHandler implements CommandHandler { context: PackageActionContext, params: ParsedCommandParams, ) { - const { appAgentHost, source } = context.sessionContext.agentContext; + const { + appAgentProviderSetController: appAgentProviderSetController, + source, + } = context.sessionContext.agentContext; const name = params.args.name; // Start the coordinated teardown and fan out the removal to every // session — including this one — through its idle-gated applicator, each @@ -537,30 +580,25 @@ class UninstallCommandHandler implements CommandHandler { // ROLLBACK — a phase timeout that leaves the agent installed and changes // nothing, so the fan-out is silent — is surfaced here, through the // session's notification channel (which survives command completion). - let settledSynchronously = false; - await source.uninstall(name, appAgentHost, (outcome) => { - settledSynchronously = true; - if (outcome === "reverted") { - context.sessionContext.notify( - AppAgentEvent.Inline, - `Agent '${name}' uninstall reverted; the agent is still installed.`, - ); - } - }); - // The barrier teardown settles asynchronously, so print the "started" - // acknowledgement. The `settledSynchronously` guard mirrors the update - // handler and stays defensive: if a source ever reported its outcome - // synchronously (before this await resolved), the "was removed" fan-out - // would already be the whole story and a "will unload shortly" line - // would be misleading. Today's uninstall path never settles that way (a - // recorded name is torn down through the barrier; a non-active recorded - // name throws before reaching here), so this branch normally prints. - if (!settledSynchronously) { - displayResult( - `Agent '${name}' uninstall started; it will unload from each session shortly.`, - context, - ); - } + await source.uninstall( + name, + appAgentProviderSetController, + (outcome) => { + if (outcome === "reverted") { + context.sessionContext.notify( + AppAgentEvent.Inline, + `Agent '${name}' uninstall reverted; the agent is still installed.`, + ); + } + }, + ); + // A successful return means the asynchronous barrier was armed. Its + // terminal outcome cannot arrive in the issuing session until this + // command releases that session's command lock. + displayResult( + `Agent '${name}' uninstall started; it will unload from each session shortly.`, + context, + ); } public async getCompletion( @@ -601,7 +639,10 @@ class UpdateCommandHandler implements CommandHandler { context: PackageActionContext, params: ParsedCommandParams, ) { - const { appAgentHost, source } = context.sessionContext.agentContext; + const { + appAgentProviderSetController: appAgentProviderSetController, + source, + } = context.sessionContext.agentContext; const { name, range } = params.args; // The source materializes the new version first and only rewrites the @@ -614,36 +655,36 @@ class UpdateCommandHandler implements CommandHandler { // A COMMITTED swap is announced by the source's cross-session fan-out // ("Agent 'x' was updated."), delivered uniformly to every session // exactly as install announces an add; the command adds no echo of its - // own. Only the outcomes the fan-out cannot express are surfaced here, - // through the session's notification channel (which survives command - // completion): a ROLLBACK (v1 restored, nothing changed) and an - // UNCHANGED no-op (the requested version was already serving). - let settledSynchronously = false; - await source.update(name, range, appAgentHost, (outcome) => { - settledSynchronously = true; - if (outcome === "reverted") { - context.sessionContext.notify( - AppAgentEvent.Inline, - `Agent '${name}' update failed; reverted to the previous version.`, - ); - } else if (outcome === "unchanged") { - context.sessionContext.notify( - AppAgentEvent.Inline, - `Agent '${name}' is already up to date.`, - ); - } - }); - // The barrier swap settles asynchronously, so print the "started" - // acknowledgement — unless the source already settled inline (an - // already-current no-op, or a non-active agent added with no barrier), - // in which case the inline outcome / fan-out is the whole story and a - // "will reload shortly" line would be misleading. - if (!settledSynchronously) { - displayResult( - `Agent '${name}' update started; it will reload in each session shortly.`, - context, - ); + // own. A rollback is surfaced through the session's notification + // channel, which survives command completion. An unchanged no-op is + // returned immediately and displayed as part of this command. + const result = await source.update( + name, + range, + appAgentProviderSetController, + (outcome) => { + if (outcome === "reverted") { + context.sessionContext.notify( + AppAgentEvent.Inline, + `Agent '${name}' update failed; reverted to the previous version.`, + ); + } + }, + ); + if (result.status === "unchanged") { + displayResult(`Agent '${name}' is already up to date.`, context); + return; } + const versionChange = + result.packageName !== undefined && + result.oldVersion !== undefined && + result.newVersion !== undefined + ? ` for package '${result.packageName}' (${result.oldVersion} -> ${result.newVersion})` + : ""; + displayResult( + `Agent '${name}' update${versionChange} started; it will reload in each session shortly.`, + context, + ); } public async getCompletion( @@ -708,7 +749,7 @@ export function buildPackageCommandTable( * Build an in-memory {@link AppAgentProvider} that vends the single command-only * `@package` agent bound to the given host-owned context. One is * created per connected dispatcher (its `agentContext` carries that session's - * {@link AppAgentHost}). + * {@link AppAgentProviderSetController}). */ export function createPackageAppAgentProvider( ctx: PackageAgentContext, diff --git a/ts/packages/defaultAgentProvider/src/installSources/sourceCommands.ts b/ts/packages/defaultAgentProvider/src/installSources/sourceCommands.ts index 433024799..4af18da57 100644 --- a/ts/packages/defaultAgentProvider/src/installSources/sourceCommands.ts +++ b/ts/packages/defaultAgentProvider/src/installSources/sourceCommands.ts @@ -22,7 +22,7 @@ import { getAddSourceCommandHandlers } from "./addSource.js"; // The host owns the entire `@package source` command surface. The dispatcher // core has no install-source registry interface: it exposes only the -// per-session `AppAgentHost` mutation surface, and merges this whole table in +// per-session app-agent-provider-set controller, and merges this whole table in // under `@package` as `source` (via `InstalledAgentSourceApi.sourceCommands()`). // All knowledge of the kind taxonomy, listing/ordering, resolution preview, and // the add grammar lives here. diff --git a/ts/packages/defaultAgentProvider/test/installSourcesFeed.spec.ts b/ts/packages/defaultAgentProvider/test/installSourcesFeed.spec.ts index 92779ae49..d717b03b2 100644 --- a/ts/packages/defaultAgentProvider/test/installSourcesFeed.spec.ts +++ b/ts/packages/defaultAgentProvider/test/installSourcesFeed.spec.ts @@ -386,6 +386,11 @@ describe("feedSource.update", () => { expect(result.record.module).toBe("@typeagent/foo-agent"); expect(result.record.ref).toBe("@typeagent/foo-agent@^1.0.0"); expect(result.record.installRoot).toBe("_typeagent_foo-agent@1.4.0"); + if (result.status === "updated") { + expect(result.packageName).toBe("@typeagent/foo-agent"); + expect(result.oldVersion).toBe("1.0.0"); + expect(result.newVersion).toBe("1.4.0"); + } }); it("applies a version range to the module", async () => { @@ -404,6 +409,10 @@ describe("feedSource.update", () => { expect(result.status).toBe("updated"); expect(result.record.ref).toBe("@typeagent/foo-agent@^2.0.0"); expect(result.record.installRoot).toBe("_typeagent_foo-agent@2.0.0"); + if (result.status === "updated") { + expect(result.oldVersion).toBe("1.0.0"); + expect(result.newVersion).toBe("2.0.0"); + } }); it("rejects a shell-injection range before it reaches npm", async () => { diff --git a/ts/packages/defaultAgentProvider/test/installSourcesInstalledProvider.spec.ts b/ts/packages/defaultAgentProvider/test/installSourcesInstalledProvider.spec.ts index 797af6e5c..a83b7caa9 100644 --- a/ts/packages/defaultAgentProvider/test/installSourcesInstalledProvider.spec.ts +++ b/ts/packages/defaultAgentProvider/test/installSourcesInstalledProvider.spec.ts @@ -22,41 +22,59 @@ import { InstallSourceConfig, InstalledAgentRecord, } from "../src/installSources/config.js"; -import { AppAgentProvider, AppAgentHost } from "agent-dispatcher"; +import { + AppAgentProvider, + AppAgentProviderSetController, + AppAgentProviderSetMutation, +} from "agent-dispatcher"; import { createLimiter } from "@typeagent/common-utils"; +import type { + AgentSourceGroup, + InstalledAgentInfo, +} from "../src/installSources/packageAgent.js"; + +function installedNames(source: { + listInstalled(): AgentSourceGroup[]; +}): string[] { + return source + .listInstalled() + .flatMap((group) => group.agents.map((agent) => agent.name)); +} + +type TestMutationFns = { + addProvider(provider: AppAgentProvider, notify?: boolean): Promise; + removeProvider( + provider: AppAgentProvider, + notify?: boolean, + dropConfig?: boolean, + ): Promise; +}; -// Compose a faithful `replaceProvider` for a test host from its own add/remove, -// modelling the applicator's single-lock section (5.7): remove the old version, -// then call the async thunk that quiesces, awaits the shared barrier, and -// decides what to add. This preserves both the recorded remove-then-add op order -// AND the barrier gating (a host that blocks its removeProvider keeps the -// barrier pending until released). -function withReplace( - host: Pick, -): AppAgentHost { +// Adapt test add/remove recorders to the callback-scoped controller contract. +// The callback itself models the one command-lock-held section. +function withReplace(host: TestMutationFns): AppAgentProviderSetController { return { - addProvider: host.addProvider, - removeProvider: host.removeProvider, - replaceProvider: async ( - oldProvider, - resolveReplacement, - notify = false, - dropConfig = false, - ) => { - await host.removeProvider(oldProvider, notify, dropConfig); - // The source decides post-barrier what to add: v2 (commit update), - // v1 (rollback), or nothing (commit uninstall). - const newProvider = await resolveReplacement(); - if (newProvider !== undefined) { - await host.addProvider(newProvider, notify); - } + async runExclusive( + callback: (mutation: AppAgentProviderSetMutation) => Promise | T, + ) { + const value = await callback({ + addProvider: (provider, options) => + host.addProvider(provider, options?.notify), + removeProvider: (provider, options) => + host.removeProvider( + provider, + options?.notify, + options?.dropConfig, + ), + }); + return { status: "completed", value }; }, }; } // A no-op issuing host used by tests that only exercise the record store or the // vended provider set (fan-out behavior is covered by its own describe). -const noopHost: AppAgentHost = withReplace({ +const noopHost: AppAgentProviderSetController = withReplace({ addProvider: async () => {}, removeProvider: async () => {}, }); @@ -188,11 +206,13 @@ function createTestUpdateableSource( function createTestUpdateableInstalledAgentSource( instanceDir: string, options?: Parameters[1], + storeWriter?: Parameters[3], ) { return createDefaultInstalledAgentSource( instanceDir, options, createTestUpdateableSource, + storeWriter, ); } @@ -486,8 +506,14 @@ describe("getDefaultAppAgentSource", () => { ); await expect(built.testApi.listAvailableAgents()).resolves.toEqual([ - { source: "healthy", ref: "foo", packageName: "foo" }, - { source: "healthy", ref: "bar", packageName: "bar" }, + { + source: "healthy", + sourceKind: "feed", + agents: [ + { ref: "foo", packageName: "foo" }, + { ref: "bar", packageName: "bar" }, + ], + }, ]); }); @@ -616,7 +642,7 @@ describe("AppAgentSource fan-out (4, )", () => { | { op: "remove"; name: string; notify: boolean }; function recordingHost(onAdd?: () => void): { - host: AppAgentHost; + host: AppAgentProviderSetController; calls: HostCall[]; } { const calls: HostCall[] = []; @@ -766,6 +792,31 @@ describe("AppAgentSource fan-out (4, )", () => { expect(readAgentsJson(instanceDir)?.agents.foo).toBeDefined(); }); + it("update of a name recorded but not active in this source errors (out-of-band inconsistency)", async () => { + const instanceDir = updateableTestInstanceDir(); + const stale = createTestUpdateableInstalledAgentSource(instanceDir); + const installer = createTestUpdateableInstalledAgentSource(instanceDir); + await installer.testApi.install( + "foo", + makePathAgentDir(), + undefined, + noopHost, + ); + + const issuing = recordingHost(); + const sibling = recordingHost(); + stale.connect(issuing.host); + stale.connect(sibling.host); + + await expect( + stale.testApi.update("foo", undefined, issuing.host), + ).rejects.toThrow(/not active in this source/i); + + expect(issuing.calls).toEqual([]); + expect(sibling.calls).toEqual([]); + expect(readAgentsJson(instanceDir)?.agents.foo).toBeDefined(); + }); + it("threads dropConfig=true for uninstall and false for update to every remove leg (Model B)", async () => { // Capture the dropConfig each remove leg receives (the shared // recordingHost drops the third arg; this dedicated host keeps it). @@ -913,7 +964,7 @@ describe("AppAgentSource fan-out (4, )", () => { // apply lands only once the command releases it. const commandLock = createLimiter(1); const applied: HostCall[] = []; - const issuing: AppAgentHost = withReplace({ + const issuing: AppAgentProviderSetController = withReplace({ addProvider: (p, notify) => commandLock(async () => { applied.push({ @@ -989,7 +1040,7 @@ describe("AppAgentSource fan-out (4, )", () => { describe("AppAgentSource lifecycle tracker (7)", () => { // An issuing host whose ops resolve immediately. - function fastHost(): AppAgentHost { + function fastHost(): AppAgentProviderSetController { return withReplace({ addProvider: async () => {}, removeProvider: async () => {}, @@ -1037,7 +1088,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { async function installFoo( built: ReturnType, - issuing: AppAgentHost, + issuing: AppAgentProviderSetController, ) { await built.testApi.install( "foo", @@ -1131,7 +1182,8 @@ describe("AppAgentSource lifecycle tracker (7)", () => { () => !built.testApi .listInstalled() - .map((i) => i.name) + .flatMap((group) => group.agents) + .map((agent) => agent.name) .includes("foo"), "foo to enter the draining (removing) state", ); @@ -1211,8 +1263,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { ); await flush(); - const listed = () => - new Set(built.testApi.listInstalled().map((a) => a.name)); + const listed = () => new Set(installedNames(built.testApi)); // Start foo's drain; the sibling holds it `removing`. const updatingFoo = built.testApi.update("foo", undefined, issuing); @@ -1268,7 +1319,8 @@ describe("AppAgentSource lifecycle tracker (7)", () => { () => !built.testApi .listInstalled() - .map((i) => i.name) + .flatMap((group) => group.agents) + .map((agent) => agent.name) .includes("foo"), "foo to enter the draining (removing) state", ); @@ -1357,21 +1409,18 @@ describe("AppAgentSource lifecycle tracker (7)", () => { () => !built.testApi .listInstalled() - .map((i) => i.name) + .flatMap((group) => group.agents) + .map((agent) => agent.name) .includes("foo"), "foo to enter the draining (removing) state", ); - expect(built.testApi.listInstalled().map((i) => i.name)).not.toContain( - "foo", - ); + expect(installedNames(built.testApi)).not.toContain("foo"); gated.release(); await updating; await flush(); // After the drain + re-add, it is listed again. - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); }); it("update adds the new version only after the old drains everywhere (no coexistence)", async () => { @@ -1413,7 +1462,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { // A sibling whose removeProvider always rejects: its barrier slot must // still be filled (the per-host catch → quiesce, 5.7) so the // barrier completes and the re-add fires — exactly once, never twice. - const throwingSibling: AppAgentHost = withReplace({ + const throwingSibling: AppAgentProviderSetController = withReplace({ addProvider: async () => {}, removeProvider: async () => { throw new Error("sibling remove boom"); @@ -1437,9 +1486,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { // exactly one remove followed by exactly one add on the issuing session. expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); // The name is free + active again (listed, and reusable). - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); }); it("update re-adds exactly once even when a sibling throws in its ADD leg (post-quiesce)", async () => { @@ -1450,7 +1497,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { // but whose ADD leg then throws. The source's per-host catch calls // `quiesce` a SECOND time; the barrier is already settled, so the double // quiesce must be a harmless no-op (no double onComplete / re-add). - const throwingAddSibling: AppAgentHost = withReplace({ + const throwingAddSibling: AppAgentProviderSetController = withReplace({ addProvider: async () => { throw new Error("sibling add boom"); }, @@ -1473,9 +1520,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { // The post-quiesce add failure did not corrupt the barrier: the issuing // session swapped exactly once and the name is active + listed. expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); }); it("update adds v2 on every session only after the LAST session quiesces (staggered)", async () => { @@ -1585,7 +1630,7 @@ describe("AppAgentSource lifecycle tracker (7)", () => { const instanceDir = pathOnlyInstanceDir(); const built = createDefaultInstalledAgentSource(instanceDir); const issuing = fastHost(); - const throwingSibling: AppAgentHost = withReplace({ + const throwingSibling: AppAgentProviderSetController = withReplace({ addProvider: async () => {}, removeProvider: async () => { throw new Error("sibling remove boom"); @@ -1664,7 +1709,7 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { async function installFooV1( built: ReturnType, instanceDir: string, - issuing: AppAgentHost, + issuing: AppAgentProviderSetController, ): Promise { await built.testApi.install( "foo", @@ -1705,9 +1750,13 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { straggler.calls.length = 0; const outcomes: string[] = []; - await built.testApi.update("foo", undefined, issuing.host, (o) => - outcomes.push(o), + const result = await built.testApi.update( + "foo", + undefined, + issuing.host, + (o) => outcomes.push(o), ); + expect(result).toEqual({ status: "started" }); // The straggler never quiesces → the phase-1 backstop fires → rollback. await settle(); @@ -1716,9 +1765,7 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { expect(readAgentsJson(instanceDir)!.agents.foo.installRoot).toBe( v1Root, ); - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); // The issuing session removed v1 then re-added v1 (rolled back, no v2). expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); @@ -1777,14 +1824,12 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { }, }); const issuing = recordingHost(); - // A session whose `replaceProvider` auto-acks WITHOUT running its thunk + // A session whose controller reports closed without running its callback // (models its barrier op queued-not-started at close): it fills its // phase-1 slot from the success continuation, which can empty `pending` // BEFORE its teardown has dropped the shared v1 ref. - const closingHost: AppAgentHost = { - addProvider: async () => {}, - removeProvider: async () => {}, - replaceProvider: async () => {}, + const closingHost: AppAgentProviderSetController = { + runExclusive: async () => ({ status: "closed" }), }; built.connect(issuing.host); const closingConn = built.connect(closingHost); @@ -1842,6 +1887,78 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); }); + it("rolls an update back when the commit record write fails", async () => { + const instanceDir = updateableTestInstanceDir(); + let failNextWrite = false; + const built = createTestUpdateableInstalledAgentSource( + instanceDir, + undefined, + (dir, data) => { + if (failNextWrite) { + failNextWrite = false; + throw new Error("commit write failed"); + } + writeAgentsJson(dir, data); + }, + ); + const issuing = recordingHost(); + built.connect(issuing.host); + await installFooV1(built, instanceDir, issuing.host); + const oldRecord = readAgentsJson(instanceDir)!.agents.foo; + issuing.calls.length = 0; + failNextWrite = true; + + const outcomes: string[] = []; + await built.testApi.update("foo", undefined, issuing.host, (outcome) => + outcomes.push(outcome), + ); + await settle(); + + expect(outcomes).toEqual(["reverted"]); + expect(readAgentsJson(instanceDir)!.agents.foo).toEqual(oldRecord); + expect(installedNames(built.testApi)).toContain("foo"); + expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); + }); + + it("rolls an uninstall back when the commit record write fails", async () => { + const instanceDir = pathOnlyInstanceDir(); + let failNextWrite = false; + const built = createDefaultInstalledAgentSource( + instanceDir, + undefined, + undefined, + (dir, data) => { + if (failNextWrite) { + failNextWrite = false; + throw new Error("commit write failed"); + } + writeAgentsJson(dir, data); + }, + ); + const issuing = recordingHost(); + built.connect(issuing.host); + await built.testApi.install( + "foo", + makePathAgentDir(), + undefined, + issuing.host, + ); + const oldRecord = readAgentsJson(instanceDir)!.agents.foo; + issuing.calls.length = 0; + failNextWrite = true; + + const outcomes: string[] = []; + await built.testApi.uninstall("foo", issuing.host, (outcome) => + outcomes.push(outcome), + ); + await settle(); + + expect(outcomes).toEqual(["reverted"]); + expect(readAgentsJson(instanceDir)!.agents.foo).toEqual(oldRecord); + expect(installedNames(built.testApi)).toContain("foo"); + expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); + }); + it("a disconnect during a rollback is safe (name stays active on v1) ()", async () => { const instanceDir = updateableTestInstanceDir(); const built = createTestUpdateableInstalledAgentSource(instanceDir, { @@ -1874,9 +1991,7 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { expect(readAgentsJson(instanceDir)!.agents.foo.installRoot).toBe( v1Root, ); - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); }); it("a rolled-back update leaves v1 durable for an immediately-following op (record never overwritten)", async () => { @@ -1908,9 +2023,7 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { expect(readAgentsJson(instanceDir)!.agents.foo.installRoot).toBe( v1Root, ); - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); // The rollback GC prunes the v2 root, NEVER v1's: v1's version-scoped // install-root directory must survive so v1 stays loadable (a regression // that pruned `oldRoot` on rollback would delete the restored version). @@ -1943,14 +2056,11 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { }, }); const issuing = recordingHost(); - // A closed/disposed host: its `replaceProvider` auto-acks immediately - // WITHOUT ever running its thunk or awaiting `whenDecided` (models an - // applicator closed at enqueue time). The barrier must still fill its + // A closed/disposed controller reports closed without running its callback + // or awaiting `whenDecided`. The barrier must still fill its // phase-1 slot for it via the success continuation. - const closedHost: AppAgentHost = { - addProvider: async () => {}, - removeProvider: async () => {}, - replaceProvider: async () => {}, + const closedHost: AppAgentProviderSetController = { + runExclusive: async () => ({ status: "closed" }), }; built.connect(issuing.host); built.connect(closedHost); @@ -1967,9 +2077,7 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { expect(outcomes).toEqual(["updated"]); expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); }); it("an uninstall straggler that won't idle rolls back (record + agent restored, name reusable)", async () => { @@ -2000,9 +2108,7 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { path.join(instanceDir, "installedAgents", "agents", v1Root), ), ).toBe(true); - expect(built.testApi.listInstalled().map((i) => i.name)).toContain( - "foo", - ); + expect(installedNames(built.testApi)).toContain("foo"); expect(issuing.calls).toEqual([{ op: "remove" }, { op: "add" }]); // The name was freed from `removing`: it is mutable again (a re-install @@ -2032,10 +2138,8 @@ describe("Update Coordination — timeout & rollback (5.3)", () => { const gate = new Promise((r) => (releaseLive = r)); const liveStraggler = recordingHost(gate); // Closed host: auto-acks + fills its slot from the success path. - const closedHost: AppAgentHost = { - addProvider: async () => {}, - removeProvider: async () => {}, - replaceProvider: async () => {}, + const closedHost: AppAgentProviderSetController = { + runExclusive: async () => ({ status: "closed" }), }; built.connect(issuing.host); built.connect(liveStraggler.host); @@ -2074,7 +2178,7 @@ describe("installed agent source api (install/uninstall/update)", () => { // A no-op issuing host: the record-store logic is independent of the // fan-out, so these tests pass a host whose add/remove do nothing. Fan-out / // enable / notification behavior is covered by the "fan-out" describe below. - const host: AppAgentHost = withReplace({ + const host: AppAgentProviderSetController = withReplace({ addProvider: async () => {}, removeProvider: async () => {}, }); @@ -2547,7 +2651,7 @@ describe("structural manifest check on install/update (5.3)", () => { ).rejects.toThrow(); // Nothing persisted: a broken agent is never recorded. expect(readAgentsJson(instanceDir)?.agents.x).toBeUndefined(); - expect(built.listInstalled().map((i) => i.name)).not.toContain("x"); + expect(installedNames(built)).not.toContain("x"); }); it("install of a catalog source succeeds when the manifest reads", async () => { @@ -2592,7 +2696,7 @@ describe("structural manifest check on install/update (5.3)", () => { expect(readAgentsJson(instanceDir)!.agents.x.path).toBe( path.join(instanceDir, "catalog-package"), ); - expect(built.listInstalled().map((i) => i.name)).toContain("x"); + expect(installedNames(built)).toContain("x"); }); it("a `path` install is validated too — a manifest-less directory fails and records nothing", async () => { @@ -2607,6 +2711,6 @@ describe("structural manifest check on install/update (5.3)", () => { built.install("p", bareDir, undefined, noopHost), ).rejects.toThrow(); expect(readAgentsJson(instanceDir)?.agents.p).toBeUndefined(); - expect(built.listInstalled().map((i) => i.name)).not.toContain("p"); + expect(installedNames(built)).not.toContain("p"); }); }); diff --git a/ts/packages/defaultAgentProvider/test/packageAgent.spec.ts b/ts/packages/defaultAgentProvider/test/packageAgent.spec.ts index f92f0d27f..2f162e688 100644 --- a/ts/packages/defaultAgentProvider/test/packageAgent.spec.ts +++ b/ts/packages/defaultAgentProvider/test/packageAgent.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { AppAgentHost } from "agent-dispatcher"; +import { AppAgentProviderSetController } from "agent-dispatcher"; import { buildPackageCommandTable, createPackageAppAgentProvider, @@ -11,10 +11,14 @@ import { } from "../src/installSources/packageAgent.js"; import { CommandHandler } from "@typeagent/agent-sdk/helpers/command"; -const noopHost: AppAgentHost = { - addProvider: async () => {}, - removeProvider: async () => {}, - replaceProvider: async () => {}, +const noopHost: AppAgentProviderSetController = { + runExclusive: async (callback) => { + const value = await callback({ + addProvider: async () => {}, + removeProvider: async () => {}, + }); + return { status: "completed", value }; + }, }; type SourceCall = @@ -50,6 +54,7 @@ function makeSource(overrides: Partial = {}): { }, update: async (name, range) => { calls.push({ op: "update", name, range }); + return { status: "started" }; }, listInstalled: () => [], listSources: () => [], @@ -126,12 +131,14 @@ function capturingActionContext(agentContext: PackageAgentContext) { function tightlyCapturingActionContext(agentContext: PackageAgentContext) { const captured: string[] = []; + const modes: Array = []; const stripAnsi = (text: string) => text.replace(/\x1B\[[0-?]*[ -/]*[@-~]/g, ""); const context = { sessionContext: { agentContext }, actionIO: { - appendDisplay: (content: any) => { + appendDisplay: (content: any, mode?: string) => { + modes.push(mode); const text = typeof content === "string" ? content @@ -148,7 +155,7 @@ function tightlyCapturingActionContext(agentContext: PackageAgentContext) { takeAction: () => {}, }, } as any; - return { context, output: () => captured.join("") }; + return { context, output: () => captured.join(""), modes }; } function getHandler( @@ -163,7 +170,7 @@ describe("@package agent", () => { it("vends a command-only agent named 'package' with the host-owned context", async () => { const { api } = makeSource(); const ctx: PackageAgentContext = { - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }; const provider = createPackageAppAgentProvider(ctx); @@ -185,7 +192,10 @@ describe("@package agent", () => { const { api, calls } = makeSource(); const handler = getHandler(api, "install"); await handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { target: "/some/path", name: "foo" }, flags: { source: "path" }, @@ -206,7 +216,10 @@ describe("@package agent", () => { const handler = getHandler(api, "install"); await expect( handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { target: "/x", name: "bad name!" }, flags: {}, @@ -220,7 +233,10 @@ describe("@package agent", () => { const { api, calls } = makeSource(); const handler = getHandler(api, "uninstall"); await handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { name: "foo" } } as any, ); expect(calls).toEqual([{ op: "uninstall", name: "foo" }]); @@ -230,13 +246,16 @@ describe("@package agent", () => { const { api, calls } = makeSource(); const handler = getHandler(api, "update"); await handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { name: "foo", range: "^1.0" } } as any, ); expect(calls).toEqual([{ op: "update", name: "foo", range: "^1.0" }]); }); - it("uninstall does not echo a committed teardown (the source fan-out owns it)", async () => { + it("uninstall acknowledges start without echoing a committed teardown", async () => { // A committed uninstall is announced by the source's cross-session // fan-out ("was removed"), like install's "was added" — the command // adds no echo of its own. @@ -246,12 +265,15 @@ describe("@package agent", () => { }, }); const handler = getHandler(api, "uninstall"); - const { context, notifications } = notifyCapturingActionContext({ - appAgentHost: noopHost, + const { context, output } = capturingActionContext({ + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: { name: "foo" } } as any); - expect(notifications).toEqual([]); + expect(output()).toContain( + "Agent 'foo' uninstall started; it will unload from each session shortly.", + ); + expect(output()).not.toContain("was removed"); }); it("uninstall reports started when teardown outcome is asynchronous", async () => { @@ -263,7 +285,7 @@ describe("@package agent", () => { }); const handler = getHandler(api, "uninstall"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: { name: "foo" } } as any); @@ -280,7 +302,7 @@ describe("@package agent", () => { }); const handler = getHandler(api, "uninstall"); const { context, notifications } = notifyCapturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: { name: "foo" } } as any); @@ -293,11 +315,12 @@ describe("@package agent", () => { const { api } = makeSource({ update: async (_name, _range, _host, onOutcome) => { onOutcome?.("updated"); + return { status: "started" }; }, }); const handler = getHandler(api, "update"); const { context, notifications } = notifyCapturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -310,11 +333,12 @@ describe("@package agent", () => { const { api } = makeSource({ update: async (_name, _range, _host, onOutcome) => { onOutcome?.("reverted"); + return { status: "started" }; }, }); const handler = getHandler(api, "update"); const { context, notifications } = notifyCapturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -327,31 +351,26 @@ describe("@package agent", () => { it("update surfaces an already-current no-op", async () => { const { api } = makeSource({ - update: async (_name, _range, _host, onOutcome) => { - onOutcome?.("unchanged"); - }, + update: async () => ({ status: "unchanged" }), }); const handler = getHandler(api, "update"); - const { context, notifications } = notifyCapturingActionContext({ - appAgentHost: noopHost, + const { context, output } = capturingActionContext({ + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: { name: "foo", range: undefined }, } as any); - expect(notifications).toEqual(["Agent 'foo' is already up to date."]); + expect(output()).toContain("Agent 'foo' is already up to date."); }); it("update reports started when swap outcome is asynchronous", async () => { const { api } = makeSource({ - update: async () => { - // Intentionally no immediate outcome callback: this models the - // real barrier swap path, where completion settles later. - }, + update: async () => ({ status: "started" }), }); const handler = getHandler(api, "update"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -361,6 +380,28 @@ describe("@package agent", () => { "Agent 'foo' update started; it will reload in each session shortly.", ); }); + + it("update reports the package version change when available", async () => { + const { api } = makeSource({ + update: async () => ({ + status: "started", + packageName: "@typeagent/foo-agent", + oldVersion: "1.0.0", + newVersion: "1.4.0", + }), + }); + const handler = getHandler(api, "update"); + const { context, output } = capturingActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }); + await handler.run(context, { + args: { name: "foo", range: undefined }, + } as any); + expect(output()).toContain( + "Agent 'foo' update for package '@typeagent/foo-agent' (1.0.0 -> 1.4.0) started; it will reload in each session shortly.", + ); + }); }); describe("@package command table", () => { @@ -394,7 +435,10 @@ describe("@package handler error handling", () => { const handler = getHandler(api, "install"); await expect( handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { target: "bad" }, flags: {} } as any, ), ).rejects.toThrow(/resolution failed/); @@ -409,7 +453,10 @@ describe("@package handler error handling", () => { const handler = getHandler(api, "uninstall"); await expect( handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { name: "missing" } } as any, ), ).rejects.toThrow(/not found/); @@ -424,7 +471,10 @@ describe("@package handler error handling", () => { const handler = getHandler(api, "update"); await expect( handler.run( - fakeActionContext({ appAgentHost: noopHost, source: api }), + fakeActionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { args: { name: "foo" } } as any, ), ).rejects.toThrow(/no longer configured/); @@ -437,22 +487,33 @@ describe("@package handler completions", () => { listAvailableAgents: async () => [ { source: "catalog", - ref: "k1", - defaultAgentName: "catalog-agent", - packageName: "@x/catalog-agent", + agents: [ + { + ref: "k1", + defaultAgentName: "catalog-agent", + packageName: "@x/catalog-agent", + }, + ], }, { source: "feed", - ref: "@x/feed-agent", - defaultAgentName: "feed-agent", - packageName: "@x/feed-agent", + agents: [ + { + ref: "@x/feed-agent", + defaultAgentName: "feed-agent", + packageName: "@x/feed-agent", + }, + ], }, ], listSources: () => ["catalog", "feed"], }); const handler = getHandler(api, "install"); const result = await handler.getCompletion!( - fakeSessionContext({ appAgentHost: noopHost, source: api }), + fakeSessionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), {} as any, ["target", "--source"], ); @@ -475,24 +536,36 @@ describe("@package handler completions", () => { return [ { source: "catalog", - ref: "k1", - defaultAgentName: "catalog-agent", - packageName: "@x/catalog-agent", + agents: [ + { + ref: "k1", + defaultAgentName: "catalog-agent", + packageName: "@x/catalog-agent", + }, + ], }, ]; } return [ { source: "catalog", - ref: "k1", - defaultAgentName: "catalog-agent", - packageName: "@x/catalog-agent", + agents: [ + { + ref: "k1", + defaultAgentName: "catalog-agent", + packageName: "@x/catalog-agent", + }, + ], }, { source: "feed", - ref: "@x/feed-agent", - defaultAgentName: "feed-agent", - packageName: "@x/feed-agent", + agents: [ + { + ref: "@x/feed-agent", + defaultAgentName: "feed-agent", + packageName: "@x/feed-agent", + }, + ], }, ]; }, @@ -500,7 +573,10 @@ describe("@package handler completions", () => { }); const handler = getHandler(api, "install"); const result = await handler.getCompletion!( - fakeSessionContext({ appAgentHost: noopHost, source: api }), + fakeSessionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), { flags: { source: "catalog" } } as any, ["target"], ); @@ -516,14 +592,17 @@ describe("@package handler completions", () => { it("uninstall/update complete the managed agent names", async () => { const { api } = makeSource({ listInstalled: () => [ - { name: "a", source: "path" }, - { name: "b", source: "feed" }, + { source: "path", agents: [{ name: "a" }] }, + { source: "feed", agents: [{ name: "b" }] }, ], }); for (const which of ["uninstall", "update"] as const) { const handler = getHandler(api, which); const result = await handler.getCompletion!( - fakeSessionContext({ appAgentHost: noopHost, source: api }), + fakeSessionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), {} as any, ["name"], ); @@ -536,42 +615,54 @@ describe("@package handler completions", () => { }); describe("@package available", () => { - it("renders available agents (name, package, source) in sorted order", async () => { + it("renders one sorted table per source in source order", async () => { const { api } = makeSource({ listAvailableAgents: async () => [ { source: "feed", - ref: "@x/zeta", - defaultAgentName: "zeta", - packageName: "@x/zeta", + sourceKind: "feed", + agents: [ + { + ref: "@x/zeta", + defaultAgentName: "zeta", + packageName: "@x/zeta", + }, + { + ref: "@x/beta", + defaultAgentName: "beta", + packageName: "@x/beta", + }, + ], }, { source: "catalog", - ref: "k-alpha", - defaultAgentName: "alpha", - packageName: "alpha-pkg", - }, - { - source: "feed", - ref: "@x/beta", - defaultAgentName: "beta", - packageName: "@x/beta", + sourceKind: "catalog", + agents: [ + { + ref: "k-alpha", + defaultAgentName: "alpha", + packageName: "alpha-pkg", + }, + ], }, ], }); const handler = getHandler(api, "available"); - const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + const { context, output, modes } = tightlyCapturingActionContext({ + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: {}, flags: {} } as any); const text = output(); - expect(text).toContain("Name Package Source"); - expect(text).toContain("alpha alpha-pkg catalog"); - expect(text).toContain("beta @x/beta feed"); - expect(text).toContain("zeta @x/zeta feed"); - expect(text.indexOf("alpha")).toBeLessThan(text.indexOf("beta")); + expect(text).toContain( + "feed (feed)\nName Package\nbeta @x/beta\nzeta @x/zeta", + ); + expect(text).toContain( + "\ncatalog (catalog)\nName Package\nalpha alpha-pkg", + ); + expect(text.indexOf("feed")).toBeLessThan(text.indexOf("catalog")); expect(text.indexOf("beta")).toBeLessThan(text.indexOf("zeta")); + expect(modes).toEqual(["block", "block", "block", "block"]); }); it("reports empty state when no agents are available", async () => { @@ -580,7 +671,7 @@ describe("@package available", () => { }); const handler = getHandler(api, "available"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: {}, flags: {} } as any); @@ -594,31 +685,43 @@ describe("@package available", () => { return [ { source: "catalog", - ref: "k-alpha", - defaultAgentName: "alpha", - packageName: "alpha-pkg", + agents: [ + { + ref: "k-alpha", + defaultAgentName: "alpha", + packageName: "alpha-pkg", + }, + ], }, ]; } return [ { source: "catalog", - ref: "k-alpha", - defaultAgentName: "alpha", - packageName: "alpha-pkg", + agents: [ + { + ref: "k-alpha", + defaultAgentName: "alpha", + packageName: "alpha-pkg", + }, + ], }, { source: "feed", - ref: "@x/beta", - defaultAgentName: "beta", - packageName: "@x/beta", + agents: [ + { + ref: "@x/beta", + defaultAgentName: "beta", + packageName: "@x/beta", + }, + ], }, ]; }, }); const handler = getHandler(api, "available"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -626,8 +729,10 @@ describe("@package available", () => { flags: { source: "catalog" }, } as any); const text = output(); - expect(text).toContain("alpha alpha-pkg catalog"); - expect(text).not.toContain("beta @x/beta feed"); + expect(text).toContain("catalog"); + expect(text).toContain("alpha alpha-pkg"); + expect(text).not.toContain("feed"); + expect(text).not.toContain("beta @x/beta"); }); it("completes --source from listSources", async () => { @@ -636,7 +741,10 @@ describe("@package available", () => { }); const handler = getHandler(api, "available"); const result = await handler.getCompletion!( - fakeSessionContext({ appAgentHost: noopHost, source: api }), + fakeSessionContext({ + appAgentProviderSetController: noopHost, + source: api, + }), {} as any, ["--source"], ); @@ -648,24 +756,33 @@ describe("@package available", () => { }); describe("@package list", () => { - it("renders headings, tables, and footer with explicit line breaks", async () => { + it("renders source headings, tables, and footer in block mode", async () => { const { api } = makeSource({ listInstalled: () => [ - { name: "beta", source: "feed", ref: "pkg-beta" }, - { name: "alpha", source: "catalog", ref: "pkg-alpha" }, + { + source: "feed-source", + sourceKind: "feed", + agents: [{ name: "beta", ref: "pkg-beta" }], + }, + { + source: "catalog-source", + sourceKind: "catalog", + agents: [{ name: "alpha", ref: "pkg-alpha" }], + }, ], }); const handler = getHandler(api, "list"); - const { context, output } = tightlyCapturingActionContext({ - appAgentHost: noopHost, + const { context, output, modes } = tightlyCapturingActionContext({ + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { args: {} } as any); expect(output()).toContain( - "catalog\nAgent Reference\nalpha pkg-alpha\nfeed\nAgent Reference\nbeta pkg-beta\nShowing installable installed agents only.", + "feed-source (feed)\nName Reference\nbeta pkg-beta\ncatalog-source (catalog)\nName Reference\nalpha pkg-alpha\nShowing installable installed agents only.", ); + expect(modes).toEqual(["block", "block", "block", "block", "block"]); }); }); @@ -685,7 +802,7 @@ describe("@package install one-argument, dry-run, and refresh", () => { }); const handler = getHandler(api, "install"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -721,7 +838,7 @@ describe("@package install one-argument, dry-run, and refresh", () => { }); const handler = getHandler(api, "install"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -771,7 +888,7 @@ describe("@package install one-argument, dry-run, and refresh", () => { }); const handler = getHandler(api, "install"); const { context, output } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { @@ -801,7 +918,7 @@ describe("@package install one-argument, dry-run, and refresh", () => { }); const handler = getHandler(api, "install"); const { context } = capturingActionContext({ - appAgentHost: noopHost, + appAgentProviderSetController: noopHost, source: api, }); await handler.run(context, { diff --git a/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts b/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts index afe6521b5..c5d3a9ad2 100644 --- a/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts +++ b/ts/packages/dispatcher/dispatcher/src/agentProvider/agentProvider.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import { AppAgent, AppAgentManifest } from "@typeagent/agent-sdk"; +import type { AppAgent, AppAgentManifest } from "@typeagent/agent-sdk"; /** * A read-only view over a set of app agents: enumerate names, fetch manifests, @@ -10,10 +10,10 @@ import { AppAgent, AppAgentManifest } from "@typeagent/agent-sdk"; * * Implementor requirements: * - **Read-only.** Never mutate dispatcher state or reach into grammars, - * collision detection, or the embedding cache — this is purely a source of + * collision detection, or the embedding cache. This is purely a source of * agents to load. * - **Stable names.** `getAppAgentNames()` must return the same names for the - * life of the provider. A provider handed to {@link AppAgentHost.addProvider} + * life of the provider. A provider handed to an app-agent-provider-set mutation * must expose exactly one name (the host asserts this). * - **Balanced, refcount-safe load/unload.** When an instance is shared across * sessions, N `loadAppAgent` calls require N `unloadAppAgent` calls before the @@ -29,161 +29,100 @@ export interface AppAgentProvider { loadAppAgent(appAgentName: string): Promise; unloadAppAgent(appAgentName: string): Promise; setTraceNamespaces?(namespaces: string): void; - // Optional: providers that start slowly can return a stub manifest from - // getAppAgentManifest and call the registered callback with the real - // manifest once the agent is ready. onSchemaReady?: ( callback: (agentName: string, manifest: AppAgentManifest) => void, ) => void; - // Optional: returns the names of agents currently loading asynchronously. - // Only these agents should show ⏳ in the UI. If omitted, no agents are - // treated as loading. getLoadingAgentNames?(): string[]; - // Optional: whether the agent currently has a loaded (refcounted) instance — - // i.e. some holder has `loadAppAgent`-ed it without a matching - // `unloadAppAgent`. The installed-agent source's coordinated - // teardown/swap reads this to VERIFY a torn-down version's shared process is - // fully released (not loaded anywhere) before it starts the new version or - // frees the name, rather than inferring it from teardown ACKs. Providers that - // do not refcount omit it (treated as released). + /** + * Return whether an agent has a loaded instance. A shared provider must + * report its actual refcount state rather than one dispatcher's local state. + */ isLoaded?(appAgentName: string): boolean; } -/** - * The dispatcher-side client callback an {@link AppAgentSource} uses to mutate a - * single connected session's live agent set. Implemented by the - * dispatcher (one per `CommandHandlerContext`); the source holds one per - * connected session and calls it to fan install/uninstall/update out to that - * session. - * - * It is the *only* surface the source uses to mutate live dispatcher state; the - * source never reaches into grammars, collision detection, or the embedding - * cache. Both operations are applied through an idle-gated FIFO applicator and - * resolve when the op is **applied** (the ack the source's lifecycle tracker - * waits on). - * - * Implementor (the dispatcher) must guarantee: - * - **FIFO, idle-gated apply.** Ops are applied in call order and deferred until - * the session is idle; each returned promise resolves only once the op has - * been applied (the source's lifecycle tracker treats that resolution as the - * ack). - * - **Single-agent registration.** `addProvider` asserts the provider exposes - * exactly one name. - * - **Identity-based removal.** `removeProvider`/`replaceProvider` match the - * provider by identity, not by name. - * - **Command-lock-held swap.** `replaceProvider` runs its remove → park → add - * as one command-lock-held section so no request interleaves the swap. - * - **Disposal safety.** After the session's {@link AppAgentConnection.dispose}, - * a late op must no-op, and a barrier the host is mid-parking on auto-acks. - */ -export interface AppAgentHost { +export type AddAppAgentProviderOptions = { + /** Show the dispatcher-local notification for this change. */ + notify?: boolean; /** - * Register a provider's agent into this dispatcher's live state. The initial - * enabled state is derived from session config with the agent's manifest - * default as the fallback: an installed agent honors its - * manifest default just like a bundled agent, and a user's per-session - * `@config agent` override still wins. Asserts the single-agent invariant - * (`provider.getAppAgentNames().length === 1`). Resolves when APPLIED — may - * be deferred until the session is idle. - * - * `notify`: when true, the dispatcher shows a system message - * naming the agent and its resulting state. Because every op — including the - * issuing session's — now applies asynchronously through the idle-gated - * queue (the inline path was removed), the issuing session is notified - * like a sibling so the terminal outcome is reported when the op settles. + * Record the agent as already known to this session. Initial source + * attachment uses false so load-time reconciliation can detect changes + * that occurred while the session was offline. */ - addProvider(provider: AppAgentProvider, notify?: boolean): Promise; + recordAsKnown?: boolean; +}; +export type RemoveAppAgentProviderOptions = { + /** Show the dispatcher-local notification for this change. */ + notify?: boolean; /** - * Remove a previously-added provider from this dispatcher by provider - * IDENTITY: unload its agent, drop schemas/grammars/embeddings, close any - * live `SessionContext`, and drop the provider's records. Internally derives - * the name(s) via `getAppAgentNames()` and calls the name-based - * `removeAgent` per name. Resolves when APPLIED. - * - * `notify`: when true, the dispatcher shows a system message - * that the agent was uninstalled/updated. As with {@link addProvider}, the - * issuing session is notified like a sibling now that every op applies - * through the idle-gated queue. - * - * `dropConfig`: when true (default — an explicit - * `@package uninstall`), the agent's persisted enable preference (its - * schema/action/command overrides) is cleared so a fresh reinstall starts - * from the manifest default. An `@package update` passes `false` so the remove leg - * of its remove-then-add swap preserves the user's per-session preference - * across a version bump. + * Clear the persisted enable preference. Uninstall uses true; update uses + * false so the replacement keeps the user's preference. */ + dropConfig?: boolean; +}; + +/** + * Temporary write access to one dispatcher's live app-agent set. + * + * The controller creates this capability for one `runExclusive` callback. The + * capability stops accepting actions when that callback returns. Both methods + * are leaf operations: they do not acquire the dispatcher command lock. + */ +export interface AppAgentProviderSetMutation { + addProvider( + provider: AppAgentProvider, + options?: AddAppAgentProviderOptions, + ): Promise; removeProvider( provider: AppAgentProvider, - notify?: boolean, - dropConfig?: boolean, + options?: RemoveAppAgentProviderOptions, ): Promise; +} - /** - * Coordinated teardown/swap of an installed agent as ONE command-lock-held - * critical section — the primitive both `@package update` and - * `@package uninstall` fan out through, replacing a separate remove-then-add. On this - * dispatcher it runs, under a SINGLE command-lock acquisition (no request can - * interleave, closing the update request-slip): - * - * 1. remove `oldProvider` (unload its agent — decrement the SHARED - * refcount — and drop its routing artifacts), exactly like - * {@link removeProvider}; - * 2. call `resolveReplacement` so the source can fill this host's barrier - * slot, park on its coordinated release promise, and decide what to add; - * 3. if `resolveReplacement` returns a provider, add it exactly like - * {@link addProvider}. The source decides post-barrier what to add: the NEW version on a - * committed `@package update`, the OLD version on a cancelled/timed-out update - * that ROLLS BACK (`v1` restored), or `undefined` (no add) on a - * committed `@package uninstall` (`old → ∅`). `undefined` means no add. - * - * No two versions of the name ever coexist, and no session observes the name - * absent across the swap. **Leaf-op invariant:** the teardown and - * startup legs run under the held command lock and must be leaf ops — process - * teardown/launch only, never dispatching a command or reacquiring the lock. - * - * `notify`/`dropConfig` are forwarded to the remove/add legs exactly as for - * {@link removeProvider}/{@link addProvider} (an update passes - * `dropConfig=false` to preserve the enable preference across the bump; an - * uninstall passes `true`). On {@link dispose} mid-op the host is dropped - * from the barrier and the op auto-acks. - */ - replaceProvider( - oldProvider: AppAgentProvider, - resolveReplacement: () => Promise, - notify?: boolean, - dropConfig?: boolean, - ): Promise; +export type AppAgentProviderSetRunResult = + | { status: "completed"; value: T } + | { status: "closed" }; + +/** + * Host-facing control surface for one dispatcher's live app-agent set. + * + * The controller owns the dispatcher command lock. The host can mutate the set + * only through the callback-scoped capability passed to `runExclusive`. + */ +export interface AppAgentProviderSetController { + runExclusive( + callback: (mutation: AppAgentProviderSetMutation) => Promise | T, + ): Promise>; } /** * The dispatcher-facing surface of the dynamic (installed) agent set. * Injected as `appAgentSources` alongside the static `appAgentProviders`. - * The concrete host object also carries the write/command surface + * The concrete source object also carries the write/command surface * (`install`/`uninstall`/`update`/`packageCommands`), but the dispatcher is * handed only the narrow `connect` view, so it can never drive an install. * * Implementor requirements (a custom source, e.g. an embedder not using * `default-agent-provider`): * - **One `connect` per dispatcher.** Return the SHARED singleton provider - * instances (the same instances on every call) and record `host` so later + * instances (the same instances on every call) and record `controller` so later * install/uninstall/update can be fanned out to that session. - * - **Mutate only through `host`.** Reach live sessions only via the given - * {@link AppAgentHost}; never touch dispatcher internals directly. - * - **Respect disposal.** Stop fanning out to a host once its + * - **Mutate only through the controller.** Reach live sessions only via + * {@link AppAgentProviderSetController.runExclusive}; never touch dispatcher internals. + * - **Respect disposal.** Stop fanning out to a controller once its * {@link AppAgentConnection.dispose} has been called; a fan-out that raced * disposal must no-op. - * - **Honor the swap barrier.** When driving {@link AppAgentHost.replaceProvider}, - * its async new-provider thunk should not resolve until every host has - * quiesced and the old version's shared refcount is verified 0. + * - **Honor the swap barrier.** For replacement, remove the old provider, + * quiesce, await the shared decision, and add the decided provider inside one + * `runExclusive` callback. */ export interface AppAgentSource { /** * Called once per dispatcher at context init. Returns the provider(s) this * source contributes to THIS session plus a teardown handle. The source - * records `host` for fan-out. + * records `controller` for fan-out. */ - connect(host: AppAgentHost): AppAgentConnection; + connect(controller: AppAgentProviderSetController): AppAgentConnection; } /** @@ -197,7 +136,7 @@ export interface AppAgentSource { * nothing is in flight, or, when a teardown/swap is in flight, once every such * barrier has settled and the source can snapshot a quiet active set — and * must not outlive those barriers. - * - `dispose()` must be idempotent and must only deregister THIS host from + * - `dispose()` must be idempotent and must only deregister THIS controller from * fan-out; it must never tear down the shared providers other sessions hold. */ export interface AppAgentConnection { @@ -233,20 +172,6 @@ export interface AppAgentConnection { dispose(): void; } -/** - * A host-rendered one-line summary of a single installed agent. The host maps - * its full `agents.json` record down to this for `@package list`. (The dispatcher - * core no longer reads the record store; this type is shared with the host's - * `AppAgentSource` implementation in `default-agent-provider`.) - */ -export interface InstalledAgentInfo { - name: string; // dispatcher agent name - source: string; // provenance (name of the source it was installed from) - // The reference that identifies the install (feed specifier / package name / - // path), whichever the record carries. Omitted if none. - ref?: string; -} - export interface ConstructionProvider { getBuiltinConstructionConfig( explainerName: string, diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentHost.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentHost.ts deleted file mode 100644 index 7446b18a9..000000000 --- a/ts/packages/dispatcher/dispatcher/src/context/appAgentHost.ts +++ /dev/null @@ -1,269 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { Limiter } from "@typeagent/common-utils"; -import registerDebug from "debug"; -import { - AppAgentHost, - AppAgentProvider, -} from "../agentProvider/agentProvider.js"; - -const debug = registerDebug("typeagent:dispatcher:agentHost"); - -type QueuedOp = { - readonly kind: "add" | "remove" | "replace"; - readonly run: () => Promise; - resolve: () => void; - reject: (reason?: unknown) => void; - // Settled once the ack has resolved/rejected (applied, abandoned, or errored). - settled: boolean; - // True once `run()` has actually started (past the closed-check inside the - // command lock). A running op is left to finish; dispose only auto-acks ops - // that have not started. - running: boolean; -}; - -/** - * The two dispatcher-side operations the applicator drives. Injected so the - * applicator is decoupled from `CommandHandlerContext` and unit-testable in - * isolation (queue ordering, ack timing, idle-gating, dispose auto-ack). - */ -export type AppAgentHostApplyFns = { - // Register a provider's single agent; its enabled state is derived from - // session config, falling back to the agent's manifest default. - // `notify` requests a sibling fan-out system message. - applyAdd: (provider: AppAgentProvider, notify: boolean) => Promise; - // Unload a previously-added provider by identity. `notify` requests a - // sibling fan-out system message. `dropConfig` clears the - // agent's persisted enable preference (true for uninstall, false for the - // remove leg of an update, which preserves it across a version bump). - applyRemove: ( - provider: AppAgentProvider, - notify: boolean, - dropConfig: boolean, - ) => Promise; - // Emit a SINGLE consolidated sibling fan-out system message for a - // `replaceProvider` swap's NET effect, so an update reads as one "updated" - // line instead of the raw remove-then-add pair. `op` is: - // - "update": a new version replaced the old (report "updated"), - // - "remove": the old version was uninstalled with no replacement - // (report "removed"), - // - undefined: a rollback restored the old version — nothing changed, so - // stay silent. - // Only invoked when the replace requested `notify`. Optional so unit tests - // that exercise the applicator in isolation need not supply it. - notifyReplace?: ( - op: "update" | "remove" | undefined, - oldProvider: AppAgentProvider, - newProvider: AppAgentProvider | undefined, - ) => void; -}; - -/** - * The dispatcher-side {@link AppAgentHost} implementation: an idle-gated FIFO - * applicator. Each `addProvider` / `removeProvider` is - * enqueued and applied at the session's next idle (gated through the session's - * command lock, so it runs between user commands and never interleaves with an - * in-flight command), in FIFO order (so an update's remove-then-add lands in - * order). The returned Promise resolves when the op is **applied** — the ack the - * source's lifecycle tracker waits on. - * - * On {@link dispose}, queued ops (including one waiting on the command lock) are - * abandoned and their acks resolved (auto-ack: a gone session has removed - * everything), and any later op is a no-op — so a fan-out that lands after close - * cannot touch a torn-down session. - */ -export class AppAgentHostApplicator implements AppAgentHost { - private closed = false; - // All enqueued-but-not-settled ops (including the one currently waiting on - // or holding the command lock), so dispose can auto-ack them. - private readonly pending = new Set(); - - constructor( - // The session's single-slot command lock; gating each op on it defers - // application until the session is idle. - private readonly commandLock: Limiter, - private readonly apply: AppAgentHostApplyFns, - ) {} - - public addProvider( - provider: AppAgentProvider, - notify: boolean = false, - ): Promise { - // Assert the single-agent invariant at the add boundary: - // source-vended providers are single-agent, so a facade regression - // fails loudly rather than desyncing the source's per-name lifecycle - // tracker. - const names = provider.getAppAgentNames(); - if (names.length !== 1) { - return Promise.reject( - new Error( - `AppAgentHost.addProvider requires a single-agent provider; got ${names.length} name(s): [${names.join(", ")}]`, - ), - ); - } - const run = () => this.apply.applyAdd(provider, notify); - return this.enqueue("add", run); - } - - public removeProvider( - provider: AppAgentProvider, - notify: boolean = false, - dropConfig: boolean = true, - ): Promise { - const run = () => this.apply.applyRemove(provider, notify, dropConfig); - return this.enqueue("remove", run); - } - - public replaceProvider( - oldProvider: AppAgentProvider, - resolveReplacement: () => Promise, - notify: boolean = false, - dropConfig: boolean = false, - ): Promise { - // Assert the single-agent invariant on the old provider at the boundary; - // the new provider is asserted below when it is built. - const oldNames = oldProvider.getAppAgentNames(); - if (oldNames.length !== 1) { - return Promise.reject( - new Error( - `AppAgentHost.replaceProvider requires a single-agent old provider; got ${oldNames.length} name(s): [${oldNames.join(", ")}]`, - ), - ); - } - // Default to preserving the enable preference: a bare - // `replaceProvider` is a swap, not a removal, so unlike `removeProvider` - // (which defaults dropConfig=true for uninstall) it keeps config by - // default. The barrier always passes `dropConfig` explicitly regardless. - // The whole teardown → quiesce → wait → (add) sequence is ONE command - // lock job: no user command interleaves between the remove and the add. - // Teardown/startup are leaf ops — they never reacquire the command lock - // or dispatch a command. - const run = async () => { - // 1. Teardown leg: unload `old` + drop routing (decrement the shared - // refcount), exactly like a remove. Its own sibling notification - // is SUPPRESSED (notify=false): a replace reports its NET effect - // once, below, so an update never surfaces as a bare "removed". - await this.apply.applyRemove(oldProvider, false, dropConfig); - // 2. Let the source fill this host's barrier slot, park (still - // holding the command lock), and decide what should be added. - const newProvider = await resolveReplacement(); - // The session may have been torn down while parked (dispose leaves a - // running op to finish): skip the startup leg rather than add the - // new version into a closing session. The source already dropped this - // host from the barrier on disconnect, so this is a clean no-op. - if (this.closed) { - return; - } - // 3. Startup leg: add whatever the source returned. The source - // decides post-barrier: the NEW version on a committed update, the OLD version on a - // cancelled/timed-out update that ROLLS BACK (v1 restored), or - // `undefined` (no add) on a committed uninstall (`old → ∅`). Its - // own notification is likewise SUPPRESSED; the consolidated one - // below carries the net effect. - if (newProvider !== undefined) { - const newNames = newProvider.getAppAgentNames(); - if (newNames.length !== 1) { - throw new Error( - `AppAgentHost.replaceProvider requires a single-agent new provider; got ${newNames.length} name(s): [${newNames.join(", ")}]`, - ); - } - await this.apply.applyAdd(newProvider, false); - } - // Consolidated sibling notification: ONE message for the swap's net - // effect. Distinguished by identity of the decided provider: - // - undefined -> committed uninstall -> "remove" - // - === oldProvider -> rollback (v1 restored) -> silent - // - a new provider -> committed update -> "update" - if (notify) { - const op = - newProvider === undefined - ? "remove" - : newProvider === oldProvider - ? undefined - : "update"; - this.apply.notifyReplace?.(op, oldProvider, newProvider); - } - }; - return this.enqueue("replace", run); - } - - /** True once {@link dispose} has been called. */ - public get isClosed(): boolean { - return this.closed; - } - - private enqueue( - kind: "add" | "remove" | "replace", - run: () => Promise, - ): Promise { - if (this.closed) { - // Late op (fan-out that lands after dispose): no-op. - debug(`Skipping ${kind} on closed host`); - return Promise.resolve(); - } - let resolve!: () => void; - let reject!: (reason?: unknown) => void; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - const op: QueuedOp = { - kind, - run, - resolve, - reject, - settled: false, - running: false, - }; - this.pending.add(op); - void this.commandLock(async () => { - if (op.settled) { - return; - } - if (this.closed) { - this.settle(op, op.resolve); - return; - } - op.running = true; - try { - await op.run(); - this.settle(op, op.resolve); - } catch (e) { - this.settle(op, () => op.reject(e)); - } - }).catch((e) => { - // The command lock itself failed (not op.run) — settle the op so - // its ack never hangs. - this.settle(op, () => op.reject(e)); - }); - return promise; - } - - private settle(op: QueuedOp, complete: () => void): void { - if (op.settled) { - return; - } - op.settled = true; - this.pending.delete(op); - complete(); - } - - /** - * Abandon queued ops and auto-ack them. An op already running - * `run()` is left to finish; every not-yet-started op (queued, or waiting on - * the command lock) is auto-acked. Idempotent. After this, - * {@link addProvider}/{@link removeProvider} are no-ops. - */ - public dispose(): void { - if (this.closed) { - return; - } - this.closed = true; - for (const op of [...this.pending]) { - if (!op.running) { - this.settle(op, op.resolve); - } - } - } -} diff --git a/ts/packages/dispatcher/dispatcher/src/context/appAgentSetController.ts b/ts/packages/dispatcher/dispatcher/src/context/appAgentSetController.ts new file mode 100644 index 000000000..72a7f2bc0 --- /dev/null +++ b/ts/packages/dispatcher/dispatcher/src/context/appAgentSetController.ts @@ -0,0 +1,308 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Limiter } from "@typeagent/common-utils"; +import { + AppAgentProvider, + AppAgentProviderSetController, + AppAgentProviderSetMutation, + AppAgentProviderSetRunResult, +} from "../agentProvider/agentProvider.js"; +import { AsyncLocalStorage } from "node:async_hooks"; + +type PendingExclusiveRun = { + settled: boolean; + running: boolean; + close: () => void; +}; + +type AppliedMutation = { + kind: "add" | "remove"; + provider: AppAgentProvider; + notify: boolean; + removeResult?: AppAgentProviderSetRemoveResult | undefined; +}; + +type NetChange = { + oldProvider: AppAgentProvider | undefined; + newProvider: AppAgentProvider | undefined; + notify: boolean; + removeResult: AppAgentProviderSetRemoveResult | undefined; +}; + +export type AppAgentProviderSetRemoveResult = { + agentNames: readonly string[]; + schemaNames: readonly string[]; +}; + +const exclusiveControllerContext = + new AsyncLocalStorage(); + +/** Dispatcher-local leaf operations used by the scoped controller. */ +export type AppAgentProviderSetApplyFns = { + applyAdd: ( + provider: AppAgentProvider, + recordAsKnown: boolean, + ) => Promise; + applyRemove: ( + provider: AppAgentProvider, + dropConfig: boolean, + ) => Promise; + finalizeRemove?: (result: AppAgentProviderSetRemoveResult) => void; + notifyChange?: ( + kind: "add" | "remove" | "update", + oldProvider: AppAgentProvider | undefined, + newProvider: AppAgentProvider | undefined, + ) => void; +}; + +/** + * Controls one dispatcher's live app-agent provider set. Every mutation runs under the + * dispatcher command lock and is available only through the callback-scoped + * capability passed to `runExclusive`. + */ +export class AppAgentProviderSetControllerImpl + implements AppAgentProviderSetController +{ + private closed = false; + private readonly pending = new Set(); + private activeMutation: { revoke: () => void } | undefined; + + public constructor( + private readonly commandLock: Limiter, + private readonly apply: AppAgentProviderSetApplyFns, + ) {} + + public runExclusive( + callback: (mutation: AppAgentProviderSetMutation) => Promise | T, + ): Promise> { + if (exclusiveControllerContext.getStore() === this) { + return Promise.reject( + new Error( + "runExclusive cannot be called recursively for the same app-agent-provider-set controller.", + ), + ); + } + if (this.closed) { + return Promise.resolve({ status: "closed" }); + } + + return new Promise>( + (resolve, reject) => { + let close = () => resolve({ status: "closed" }); + const run: PendingExclusiveRun = { + settled: false, + running: false, + close: () => close(), + }; + this.pending.add(run); + + void this.commandLock(async () => { + if (run.settled) { + return; + } + if (this.closed) { + this.settle(run, () => resolve({ status: "closed" })); + return; + } + run.running = true; + + let active = true; + let tail = Promise.resolve(); + const accepted: Promise[] = []; + const applied: AppliedMutation[] = []; + const revoke = () => { + active = false; + }; + this.activeMutation = { revoke }; + + const accept = ( + action: () => Promise, + ): Promise => { + if (!active) { + return Promise.reject( + new Error( + this.closed + ? "The app-agent-provider-set controller is closed." + : "The app-agent-provider-set mutation capability is no longer active. Use it only inside its runExclusive callback.", + ), + ); + } + const acceptedAction = tail.then(action); + tail = acceptedAction.catch(() => {}); + accepted.push(acceptedAction); + return acceptedAction; + }; + + const mutation: AppAgentProviderSetMutation = { + addProvider: (provider, options) => + accept(async () => { + this.assertSingleAgent(provider, "addProvider"); + await this.apply.applyAdd( + provider, + options?.recordAsKnown ?? true, + ); + applied.push({ + kind: "add", + provider, + notify: options?.notify ?? false, + }); + }), + removeProvider: (provider, options) => + accept(async () => { + this.assertSingleAgent( + provider, + "removeProvider", + ); + const removeResult = + await this.apply.applyRemove( + provider, + options?.dropConfig ?? true, + ); + applied.push({ + kind: "remove", + provider, + notify: options?.notify ?? false, + removeResult: removeResult ?? undefined, + }); + }), + }; + + try { + const value = await exclusiveControllerContext.run( + this, + () => callback(mutation), + ); + revoke(); + await Promise.all(accepted); + if (!this.closed) { + const changes = this.collectNetChanges(applied); + this.finalizeNetRemovals(changes); + this.notifyNetChanges(changes); + } + this.settle(run, () => + resolve({ status: "completed", value }), + ); + } catch (error) { + revoke(); + await Promise.allSettled(accepted); + this.settle(run, () => reject(error)); + } finally { + if (this.activeMutation?.revoke === revoke) { + this.activeMutation = undefined; + } + } + }).catch((error) => { + this.settle(run, () => reject(error)); + }); + + close = () => + this.settle(run, () => resolve({ status: "closed" })); + }, + ); + } + + public get isClosed(): boolean { + return this.closed; + } + + public dispose(): void { + if (this.closed) { + return; + } + this.closed = true; + this.activeMutation?.revoke(); + for (const run of [...this.pending]) { + if (!run.running) { + run.close(); + } + } + } + + private assertSingleAgent( + provider: AppAgentProvider, + operation: "addProvider" | "removeProvider", + ): void { + const names = provider.getAppAgentNames(); + if (names.length !== 1) { + throw new Error( + `AppAgentProviderSetMutation.${operation} requires a single-agent provider; got ${names.length} name(s): [${names.join(", ")}]`, + ); + } + } + + private collectNetChanges(applied: AppliedMutation[]): NetChange[] { + const changes = new Map(); + for (const mutation of applied) { + const name = mutation.provider.getAppAgentNames()[0]; + let change = changes.get(name); + if (change === undefined) { + change = { + oldProvider: undefined, + newProvider: undefined, + notify: false, + removeResult: undefined, + }; + changes.set(name, change); + } + change.notify ||= mutation.notify; + if (mutation.kind === "remove") { + if (change.newProvider === mutation.provider) { + // A remove that cancels an earlier add in the same run has + // no baseline old provider to report. + change.newProvider = undefined; + } else { + change.oldProvider ??= mutation.provider; + change.removeResult ??= mutation.removeResult; + } + } else { + change.newProvider = mutation.provider; + } + } + return [...changes.values()]; + } + + private finalizeNetRemovals(changes: NetChange[]): void { + for (const change of changes) { + if ( + change.oldProvider !== undefined && + change.newProvider === undefined + ) { + if (change.removeResult !== undefined) { + this.apply.finalizeRemove?.(change.removeResult); + } + } + } + } + + private notifyNetChanges(changes: NetChange[]): void { + if (this.apply.notifyChange === undefined) { + return; + } + for (const change of changes) { + if (!change.notify || change.oldProvider === change.newProvider) { + continue; + } + const kind = + change.oldProvider === undefined + ? "add" + : change.newProvider === undefined + ? "remove" + : "update"; + this.apply.notifyChange( + kind, + change.oldProvider, + change.newProvider, + ); + } + } + + private settle(run: PendingExclusiveRun, complete: () => void): void { + if (run.settled) { + return; + } + run.settled = true; + this.pending.delete(run); + complete(); + } +} diff --git a/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts b/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts index 387675447..630c13efe 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/commandHandlerContext.ts @@ -69,15 +69,15 @@ import { import { IPortRegistrar, PortRegistrar } from "./portRegistrar.js"; import { AppAgentProvider, - AppAgentHost, AppAgentSource, AppAgentConnection, ConstructionProvider, } from "../agentProvider/agentProvider.js"; import { - AppAgentHostApplicator, - AppAgentHostApplyFns, -} from "./appAgentHost.js"; + AppAgentProviderSetApplyFns, + AppAgentProviderSetControllerImpl, + AppAgentProviderSetRemoveResult, +} from "./appAgentSetController.js"; import { getSchemaNamePrefix } from "../execute/actionHandlers.js"; import { RequestMetricsManager } from "../utils/metrics.js"; import { displayError } from "@typeagent/agent-sdk/helpers/display"; @@ -192,11 +192,10 @@ export type CopilotImporter = ( export type CommandHandlerContext = { readonly agents: AppAgentManager; readonly portRegistrar: IPortRegistrar; - // The per-dispatcher AppAgentHost applicator: an - // idle-gated FIFO add/remove surface connected AppAgentSources use to mutate - // this session's live agent set. This instance is placed into the + // The scoped controller connected AppAgentSources use to mutate this + // session's live agent set. This instance is placed into the // host-owned `@package` agent's own agentContext. - appAgentHost: AppAgentHostApplicator; + appAgentProviderSetController: AppAgentProviderSetControllerImpl; // Live connections to the injected AppAgentSources. Disposed at context // teardown, which deregisters this host from each source's registry without // tearing down the shared provider instances. @@ -869,7 +868,7 @@ export function reconcileKnownAgents(context: CommandHandlerContext): void { } /** - * The {@link AppAgentHost.addProvider} body: register + * Register a provider under an app-agent-provider-set mutation. * the provider (deriving its enabled state from session config with the manifest * default as fallback, via {@link installAppProvider}), record it in the known * set, and — on a sibling fan-out (`notify`) — show a system message naming @@ -878,45 +877,33 @@ export function reconcileKnownAgents(context: CommandHandlerContext): void { async function hostAddProvider( context: CommandHandlerContext, provider: AppAgentProvider, - notify: boolean, + recordAsKnown: boolean, ) { await installAppProvider(context, provider); - // Record the newly-added agent(s) so a later load reconciles accurately. - addKnownAgents(context, provider.getAppAgentNames()); - - // Sibling fan-out notification: show a system message naming - // the agent and its resulting (config/manifest-derived) state. - if (notify) { - const name = provider.getAppAgentNames()[0]; - emitAgentChangeNotification( - context.clientIO, - "add", - provider, - isAgentEnabled(context, name), - ); + if (recordAsKnown) { + addKnownAgents(context, provider.getAppAgentNames()); } } /** - * The {@link AppAgentHost.removeProvider} body: tear down a + * Tear down a provider under an app-agent-provider-set mutation. * previously-added provider by identity via the {@link AppAgentManager} * removeProvider primitive, and forget it from the known set. Runs through the * idle-gated applicator. On a sibling fan-out (`notify`), surfaces * a system message. * - * `dropConfig`: when true (explicit `@package uninstall`), also - * clears the agent's persisted enable preference so a fresh reinstall starts - * from the manifest default. An `@package update` passes `false` so the remove leg of - * its remove-then-add swap leaves the user's per-session preference intact - * across a version bump. + * `dropConfig`: when true (explicit `@package uninstall`), returns the captured + * agent and schema names needed to clear the persisted enable preference. The + * controller passes that data to `finalizeRemove` only for a net removal. A + * rollback that restores the provider keeps the preference. An `@package update` + * passes `false` so its remove-then-add swap preserves the preference. */ async function hostRemoveProvider( context: CommandHandlerContext, provider: AppAgentProvider, - notify: boolean, dropConfig: boolean, -) { +): Promise { const names = provider.getAppAgentNames(); // Capture the agent's schema names before removal so we can clear their // persisted config entries afterward. @@ -929,43 +916,24 @@ async function hostRemoveProvider( context.agentCache.grammarStore, ); - if (dropConfig) { - dropAgentConfig(context, names, schemaNames); - } removeKnownAgents(context, names); - - if (notify) { - emitAgentChangeNotification( - context.clientIO, - "remove", - provider, - false, - ); - } + return dropConfig ? { agentNames: names, schemaNames } : undefined; } /** - * The consolidated sibling notification for a {@link AppAgentHost.replaceProvider} - * swap: one message for the swap's net effect instead of the raw remove-then-add - * pair. `op` is "update" (a new version replaced the old — report "updated"), - * "remove" (the old version was uninstalled with no replacement — report - * "removed"), or undefined (a rollback restored the old version — nothing - * changed, stay silent). The enabled state is read from the session's own config - * so each session's message reflects its local preference (preserved across a - * version bump). + * Report the net effect after an exclusive mutation completes. A remove/add + * pair is one update notification, and restoring the same provider is silent. */ -function hostNotifyReplace( +function hostNotifyChange( context: CommandHandlerContext, - op: "update" | "remove" | undefined, - oldProvider: AppAgentProvider, + op: "add" | "update" | "remove", + oldProvider: AppAgentProvider | undefined, newProvider: AppAgentProvider | undefined, ) { - if (op === undefined) { - // Rollback: the old version was restored, so nothing changed for this - // session — no message. - return; - } if (op === "remove") { + if (oldProvider === undefined) { + return; + } emitAgentChangeNotification( context.clientIO, "remove", @@ -974,15 +942,13 @@ function hostNotifyReplace( ); return; } - // Update: report the agent as updated, carrying this session's own - // enabled state (the preference is preserved across the version bump). if (newProvider === undefined) { return; } const name = newProvider.getAppAgentNames()[0]; emitAgentChangeNotification( context.clientIO, - "update", + op, newProvider, isAgentEnabled(context, name), ); @@ -1077,7 +1043,8 @@ export async function initializeCommandHandlerContext( portRegistrar, // Assigned just below once `context` exists (the apply closures need // it); mirrors how `requestQueue` is wired. - appAgentHost: undefined as unknown as AppAgentHostApplicator, + appAgentProviderSetController: + undefined as unknown as AppAgentProviderSetControllerImpl, appAgentConnections: [], session, persistDir, @@ -1233,30 +1200,35 @@ export async function initializeCommandHandlerContext( await initializeMemory(context, sessionDirPath); - // Build the per-dispatcher AppAgentHost applicator. + // Build the per-dispatcher app-agent-provider-set controller. // Its apply closures reach the fully-built `context`. - const hostApplyFns: AppAgentHostApplyFns = { - applyAdd: (provider, notify) => - hostAddProvider(context, provider, notify), - applyRemove: (provider, notify, dropConfig) => - hostRemoveProvider(context, provider, notify, dropConfig), - notifyReplace: (op, oldProvider, newProvider) => - hostNotifyReplace(context, op, oldProvider, newProvider), + const applyFns: AppAgentProviderSetApplyFns = { + applyAdd: (provider, recordAsKnown) => + hostAddProvider(context, provider, recordAsKnown), + applyRemove: (provider, dropConfig) => + hostRemoveProvider(context, provider, dropConfig), + finalizeRemove: ({ agentNames, schemaNames }) => + dropAgentConfig(context, agentNames, schemaNames), + notifyChange: (op, oldProvider, newProvider) => + hostNotifyChange(context, op, oldProvider, newProvider), }; - context.appAgentHost = new AppAgentHostApplicator( - context.commandLock, - hostApplyFns, - ); + context.appAgentProviderSetController = + new AppAgentProviderSetControllerImpl( + context.commandLock, + applyFns, + ); await addAppAgentProviders(context, options?.appAgentProviders); // Connect the injected dynamic agent sources. The // initial set comes from the vended `connection.providers` registered // through the normal path; subsequent add/remove deltas arrive via the - // `AppAgentHost` fan-out. + // scoped controller fan-out. if (options?.appAgentSources) { for (const source of options.appAgentSources) { - const connection = source.connect(context.appAgentHost); + const connection = source.connect( + context.appAgentProviderSetController, + ); context.appAgentConnections.push(connection); // Register the vended providers under a SINGLE held command // lock, acquired synchronously in the same tick as the @@ -1286,12 +1258,21 @@ export async function initializeCommandHandlerContext( // Uses `installAppProvider` directly (not the add-known-agents // path) so `reconcileKnownAgents` below still sees the true // persisted-vs-available diff. - const { providers } = connection; - await context.commandLock(async () => { - for (const provider of await providers) { - await installAppProvider(context, provider); - } - }); + const result = + await context.appAgentProviderSetController.runExclusive( + async (mutation) => { + for (const provider of await connection.providers) { + await mutation.addProvider(provider, { + recordAsKnown: false, + }); + } + }, + ); + if (result.status === "closed") { + throw new Error( + "App-agent-provider-set controller closed during initialization.", + ); + } } } @@ -1320,7 +1301,7 @@ export async function initializeCommandHandlerContext( return context; } catch (e) { if (contextForCleanup !== undefined) { - contextForCleanup.appAgentHost?.dispose(); + contextForCleanup.appAgentProviderSetController?.dispose(); try { await contextForCleanup.requestQueue?.drainAndStop(); } catch { @@ -1705,9 +1686,8 @@ function processSetAppAgentStateResult( export async function closeCommandHandlerContext( context: CommandHandlerContext, ) { - // Stop accepting fan-out ops into this (closing) session: abandon queued - // add/remove and make any later fan-out a no-op. - context.appAgentHost.dispose(); + // Stop accepting exclusive mutations in this closing session. + context.appAgentProviderSetController.dispose(); // Drain in-flight/queued entries before tearing down agents. try { await context.requestQueue.drainAndStop(); @@ -1722,7 +1702,7 @@ export async function closeCommandHandlerContext( // down — other sessions still hold them. await context.agents.close(); // Only after this session's agent instances are unloaded do we disconnect - // from the dynamic sources: deregister this host from each source's client + // from the dynamic sources: deregister this controller from each source's client // registry so any in-flight barrier stops waiting on it. for (const connection of context.appAgentConnections) { try { diff --git a/ts/packages/dispatcher/dispatcher/src/index.ts b/ts/packages/dispatcher/dispatcher/src/index.ts index 4c3d5ff24..abfc3b9a4 100644 --- a/ts/packages/dispatcher/dispatcher/src/index.ts +++ b/ts/packages/dispatcher/dispatcher/src/index.ts @@ -15,10 +15,13 @@ export type { } from "./context/portRegistrar.js"; export type { AppAgentProvider, - AppAgentHost, + AddAppAgentProviderOptions, + RemoveAppAgentProviderOptions, + AppAgentProviderSetMutation, + AppAgentProviderSetRunResult, + AppAgentProviderSetController, AppAgentSource, AppAgentConnection, - InstalledAgentInfo, ConstructionProvider, } from "./agentProvider/agentProvider.js"; export type { diff --git a/ts/packages/dispatcher/dispatcher/test/appAgentHost.spec.ts b/ts/packages/dispatcher/dispatcher/test/appAgentHost.spec.ts deleted file mode 100644 index 7f0915938..000000000 --- a/ts/packages/dispatcher/dispatcher/test/appAgentHost.spec.ts +++ /dev/null @@ -1,1045 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { describe, it, expect } from "@jest/globals"; -import { AppAgentEvent } from "@typeagent/agent-sdk"; -import { createLimiter } from "@typeagent/common-utils"; -import { AppAgentProvider } from "../src/agentProvider/agentProvider.js"; -import { - AppAgentHostApplicator, - AppAgentHostApplyFns, -} from "../src/context/appAgentHost.js"; -import { AppAgentManager } from "../src/context/appAgentManager.js"; -import { PortRegistrar } from "../src/context/portRegistrar.js"; -import { - emitAgentChangeNotification, - reconcileKnownAgents, -} from "../src/context/commandHandlerContext.js"; - -// A single-agent provider stub (the shape the source vends). -function fakeProvider(name: string): AppAgentProvider { - return { - getAppAgentNames: () => [name], - getAppAgentManifest: async () => ({}) as any, - loadAppAgent: async () => ({}) as any, - unloadAppAgent: async () => {}, - }; -} - -// A multi-agent provider stub (violates the single-agent invariant). -function fakeMultiProvider(...names: string[]): AppAgentProvider { - return { - getAppAgentNames: () => names, - getAppAgentManifest: async () => ({}) as any, - loadAppAgent: async () => ({}) as any, - unloadAppAgent: async () => {}, - }; -} - -// A deferred promise helper for gating apply functions / occupying the lock. -function deferred() { - let resolve!: (value: T) => void; - let reject!: (reason?: unknown) => void; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - return { promise, resolve, reject }; -} - -const tick = () => new Promise((res) => setTimeout(res, 0)); - -describe("AppAgentHostApplicator", () => { - it("applies ops in FIFO order (update remove-then-add lands in order)", async () => { - const order: string[] = []; - const apply: AppAgentHostApplyFns = { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }; - const host = new AppAgentHostApplicator(createLimiter(1), apply); - const p = fakeProvider("foo"); - - // update = remove-then-add, enqueued back-to-back. - const removeP = host.removeProvider(p); - const addP = host.addProvider(p); - await Promise.all([removeP, addP]); - - expect(order).toEqual(["remove:foo", "add:foo"]); - }); - - it("threads notify through to the apply functions", async () => { - const seen: { - addNotify?: boolean; - removeNotify?: boolean; - } = {}; - const apply: AppAgentHostApplyFns = { - applyAdd: async (_p, notify) => { - seen.addNotify = notify; - }, - applyRemove: async (_p, notify) => { - seen.removeNotify = notify; - }, - }; - const host = new AppAgentHostApplicator(createLimiter(1), apply); - await host.addProvider(fakeProvider("foo"), true); - await host.removeProvider(fakeProvider("foo"), true); - expect(seen).toEqual({ - addNotify: true, - removeNotify: true, - }); - }); - - it("resolves the ack only when the op is applied", async () => { - const gate = deferred(); - let applied = false; - const apply: AppAgentHostApplyFns = { - applyAdd: async () => { - await gate.promise; - applied = true; - }, - applyRemove: async () => {}, - }; - const host = new AppAgentHostApplicator(createLimiter(1), apply); - - let ackResolved = false; - const ack = host.addProvider(fakeProvider("foo")).then(() => { - ackResolved = true; - }); - - await tick(); - // The apply function is blocked on the gate, so the ack must not resolve. - expect(applied).toBe(false); - expect(ackResolved).toBe(false); - - gate.resolve(); - await ack; - expect(applied).toBe(true); - expect(ackResolved).toBe(true); - }); - - it("idle-gates: defers application while the session is busy", async () => { - const commandLock = createLimiter(1); - let added = false; - const apply: AppAgentHostApplyFns = { - applyAdd: async () => { - added = true; - }, - applyRemove: async () => {}, - }; - const host = new AppAgentHostApplicator(commandLock, apply); - - // Occupy the single command-lock slot to simulate an in-flight command. - const busy = deferred(); - const busyDone = commandLock(async () => { - await busy.promise; - }); - - const ack = host.addProvider(fakeProvider("foo")); - await tick(); - // The session is busy, so the op must not have applied yet. - expect(added).toBe(false); - - // Release the session; the op now applies at idle. - busy.resolve(); - await busyDone; - await ack; - expect(added).toBe(true); - }); - - it("threads notify/dropConfig through and no-ops after dispose", async () => { - const seen: { - addNotify?: boolean; - removeNotify?: boolean; - dropConfig?: boolean; - } = {}; - let calls = 0; - const apply: AppAgentHostApplyFns = { - applyAdd: async (_p, notify) => { - calls++; - seen.addNotify = notify; - }, - applyRemove: async (_p, notify, dropConfig) => { - calls++; - seen.removeNotify = notify; - seen.dropConfig = dropConfig; - }, - }; - const host = new AppAgentHostApplicator(createLimiter(1), apply); - // No command is holding the lock, so each enqueued op applies at idle. - await host.addProvider(fakeProvider("foo"), true); - await host.removeProvider(fakeProvider("foo"), true, false); - expect(seen).toEqual({ - addNotify: true, - removeNotify: true, - dropConfig: false, - }); - - host.dispose(); - // A late op after teardown is a no-op (6). - await expect( - host.addProvider(fakeProvider("foo"), false), - ).resolves.toBeUndefined(); - expect(calls).toBe(2); - }); - - it("dispose auto-acks pending removals and abandons queued ops", async () => { - const commandLock = createLimiter(1); - let removeCalls = 0; - const apply: AppAgentHostApplyFns = { - applyAdd: async () => {}, - applyRemove: async () => { - removeCalls++; - }, - }; - const host = new AppAgentHostApplicator(commandLock, apply); - - // Keep the session busy so the removal stays queued. - const busy = deferred(); - const busyDone = commandLock(async () => { - await busy.promise; - }); - - const removeAck = host.removeProvider(fakeProvider("foo")); - await tick(); - - // Dispose while the removal is still queued: it must auto-ack without - // ever running applyRemove. - host.dispose(); - await expect(removeAck).resolves.toBeUndefined(); - expect(removeCalls).toBe(0); - - // Drain the (now unrelated) busy task. - busy.resolve(); - await busyDone; - expect(removeCalls).toBe(0); - }); - - it("makes ops after dispose a no-op (fan-out that lands after close)", async () => { - let calls = 0; - const apply: AppAgentHostApplyFns = { - applyAdd: async () => { - calls++; - }, - applyRemove: async () => { - calls++; - }, - }; - const host = new AppAgentHostApplicator(createLimiter(1), apply); - host.dispose(); - - await expect( - host.addProvider(fakeProvider("foo")), - ).resolves.toBeUndefined(); - await expect( - host.removeProvider(fakeProvider("foo")), - ).resolves.toBeUndefined(); - expect(calls).toBe(0); - expect(host.isClosed).toBe(true); - }); - - it("dispose is idempotent", async () => { - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => {}, - applyRemove: async () => {}, - }); - host.dispose(); - expect(() => host.dispose()).not.toThrow(); - expect(host.isClosed).toBe(true); - }); - - it("double dispose is safe and a late op after both disposes no-ops", async () => { - let calls = 0; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => { - calls++; - }, - applyRemove: async () => { - calls++; - }, - }); - host.dispose(); - host.dispose(); - await expect( - host.addProvider(fakeProvider("late")), - ).resolves.toBeUndefined(); - expect(calls).toBe(0); - expect(host.isClosed).toBe(true); - }); - - it("rejects a multi-agent provider (single-agent invariant)", async () => { - let addCalls = 0; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => { - addCalls++; - }, - applyRemove: async () => {}, - }); - - await expect( - host.addProvider(fakeMultiProvider("foo", "bar")), - ).rejects.toThrow(/single-agent provider/i); - // The invariant fails before the op is ever applied. - expect(addCalls).toBe(0); - }); - - it("propagates apply errors to the ack (issuing session awaited failure)", async () => { - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => { - throw new Error("collision"); - }, - applyRemove: async () => {}, - }); - await expect(host.addProvider(fakeProvider("foo"))).rejects.toThrow( - /collision/, - ); - }); - - it("propagates applyRemove errors to the ack (symmetric to add)", async () => { - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => {}, - applyRemove: async () => { - throw new Error("unload failed"); - }, - }); - await expect(host.removeProvider(fakeProvider("foo"))).rejects.toThrow( - /unload failed/, - ); - }); - - it("leaves an actively-running op to finish on dispose (only queued ops auto-ack)", async () => { - const commandLock = createLimiter(1); - const gate = deferred(); - let addFinished = false; - let queuedRan = false; - const host = new AppAgentHostApplicator(commandLock, { - applyAdd: async () => { - // First op: block mid-run until the gate opens. - await gate.promise; - addFinished = true; - }, - applyRemove: async () => { - queuedRan = true; - }, - }); - - // Op A starts running (acquires the lock, enters applyAdd, awaits gate). - const ackA = host.addProvider(fakeProvider("A"), true); - await tick(); - // Op B is queued behind the running op A. - const ackB = host.removeProvider(fakeProvider("B")); - await tick(); - - // Dispose while A is actively running and B is still queued. - host.dispose(); - - // B (queued, not started) auto-acks without ever running. - await expect(ackB).resolves.toBeUndefined(); - expect(queuedRan).toBe(false); - expect(addFinished).toBe(false); - - // A is left to finish; its ack resolves after run() completes. - gate.resolve(); - await expect(ackA).resolves.toBeUndefined(); - expect(addFinished).toBe(true); - }); - - it("settles the ack (and keeps pumping) if the command lock itself throws", async () => { - // A command lock that rejects on its first acquisition — the failure is - // in the gating, not in op.run. The op must still settle so its ack - // never hangs, and a following op must still be applied. - let lockCalls = 0; - const flakyLock = (cb: () => Promise): Promise => { - lockCalls++; - if (lockCalls === 1) { - return Promise.reject(new Error("lock exploded")); - } - return cb(); - }; - let secondApplied = false; - const host = new AppAgentHostApplicator(flakyLock, { - applyAdd: async () => { - secondApplied = true; - }, - applyRemove: async () => {}, - }); - - const firstAck = host.removeProvider(fakeProvider("A")); - const secondAck = host.addProvider(fakeProvider("B"), true); - - await expect(firstAck).rejects.toThrow(/lock exploded/); - await expect(secondAck).resolves.toBeUndefined(); - expect(secondApplied).toBe(true); - }); - - it("preserves FIFO across a longer mixed sequence", async () => { - const order: string[] = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - await Promise.all([ - host.addProvider(fakeProvider("A"), true), - host.removeProvider(fakeProvider("A")), - host.addProvider(fakeProvider("B"), true), - host.addProvider(fakeProvider("C"), true), - host.removeProvider(fakeProvider("B")), - host.removeProvider(fakeProvider("C")), - ]); - expect(order).toEqual([ - "add:A", - "remove:A", - "add:B", - "add:C", - "remove:B", - "remove:C", - ]); - }); - - it("replaceProvider holds one command-lock section across remove → wait → add (no interleave)", async () => { - const order: string[] = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - - const ready = deferred(); - let quiesced = false; - const replaceAck = host.replaceProvider( - fakeProvider("v1"), - async () => { - quiesced = true; - await ready.promise; - return fakeProvider("v2"); - }, - ); - - await tick(); - // The teardown leg has run and the host has quiesced, but the barrier is - // not released yet, so the add is parked — the op still holds the lock. - expect(order).toEqual(["remove:v1"]); - expect(quiesced).toBe(true); - - // A user op enqueued now is stuck behind the parked replace (single - // command-lock section): it must NOT interleave between remove and add. - const userAck = host.removeProvider(fakeProvider("user"), true, false); - await tick(); - expect(order).toEqual(["remove:v1"]); - - // Release the barrier: the replace adds v2, then the queued user op runs. - ready.resolve(); - await Promise.all([replaceAck, userAck]); - expect(order).toEqual(["remove:v1", "add:v2", "remove:user"]); - }); - - it("replaceProvider omits the add for an uninstall (old → ∅)", async () => { - const order: string[] = []; - let quiesced = false; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p, _n, dropConfig) => { - order.push(`remove:${p.getAppAgentNames()[0]}:${dropConfig}`); - }, - }); - - const ack = host.replaceProvider( - fakeProvider("gone"), - async () => { - quiesced = true; - return undefined; - }, - true, - true, - ); - await ack; - - // Only the teardown leg ran (with dropConfig threaded); no add. - expect(order).toEqual(["remove:gone:true"]); - expect(quiesced).toBe(true); - }); - - it("replaceProvider omits the add when the thunk returns undefined (post-barrier rollback/uninstall decision)", async () => { - const order: string[] = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - - // A thunk is supplied, but it resolves to `undefined` — the source - // decided post-barrier to add nothing (a rolled-back uninstall, or an - // update that reverted with no version to re-add). The teardown leg runs; - // the add is skipped without touching the single-agent invariant. - let thunkCalls = 0; - const ack = host.replaceProvider( - fakeProvider("v1"), - async () => { - thunkCalls++; - return undefined; - }, - true, - ); - await ack; - - expect(order).toEqual(["remove:v1"]); - expect(thunkCalls).toBe(1); - }); - - it("replaceProvider rejects a multi-agent old provider before any apply", async () => { - let calls = 0; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => { - calls++; - }, - applyRemove: async () => { - calls++; - }, - }); - await expect( - host.replaceProvider(fakeMultiProvider("a", "b"), async () => - fakeProvider("v2"), - ), - ).rejects.toThrow(/single-agent old provider/i); - expect(calls).toBe(0); - }); - - it("replaceProvider rejects a multi-agent new provider after the teardown leg", async () => { - const order: string[] = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - await expect( - host.replaceProvider(fakeProvider("v1"), async () => - fakeMultiProvider("a", "b"), - ), - ).rejects.toThrow(/single-agent new provider/i); - // The teardown leg still ran (the old version is down); only the add is - // skipped because the new provider violates the invariant. - expect(order).toEqual(["remove:v1"]); - }); - - it("dispose auto-acks a queued replace without running it", async () => { - const commandLock = createLimiter(1); - let calls = 0; - const host = new AppAgentHostApplicator(commandLock, { - applyAdd: async () => { - calls++; - }, - applyRemove: async () => { - calls++; - }, - }); - - // Keep the session busy so the replace stays queued. - const busy = deferred(); - const busyDone = commandLock(async () => { - await busy.promise; - }); - - let quiesced = false; - const ack = host.replaceProvider(fakeProvider("v1"), async () => { - quiesced = true; - return fakeProvider("v2"); - }); - await tick(); - - host.dispose(); - await expect(ack).resolves.toBeUndefined(); - // Never ran: neither leg applied and the host never quiesced. - expect(calls).toBe(0); - expect(quiesced).toBe(false); - - busy.resolve(); - await busyDone; - expect(calls).toBe(0); - }); - - it("a replace parked in its replacement resolver skips the add after dispose (closed re-check)", async () => { - const order: string[] = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - - const ready = deferred(); - let quiesced = false; - const ack = host.replaceProvider(fakeProvider("v1"), async () => { - quiesced = true; - await ready.promise; - return fakeProvider("v2"); - }); - - await tick(); - // The teardown leg ran and the host quiesced; the op is parked mid-run - // in the replacement resolver (running, so dispose leaves it to finish). - expect(order).toEqual(["remove:v1"]); - expect(quiesced).toBe(true); - - host.dispose(); - // Release the barrier: the parked op resumes but must NOT add v2 into a - // torn-down session — the closed re-check short-circuits the add leg. - ready.resolve(); - await expect(ack).resolves.toBeUndefined(); - expect(order).toEqual(["remove:v1"]); - }); - - it("replaceProvider on an already-closed host auto-acks without onQuiesced or legs", async () => { - // A host that is CLOSED at enqueue time auto-acks with a resolved promise - // and never runs `run()`, so `onQuiesced`/the legs/the thunk never fire. - // The source barrier's success continuation depends on this: it re-calls - // `quiesce` to fill the phase-1 slot for exactly such a host. - let calls = 0; - let quiesced = false; - let thunkCalls = 0; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => { - calls++; - }, - applyRemove: async () => { - calls++; - }, - }); - host.dispose(); - - const ack = host.replaceProvider( - fakeProvider("v1"), - async () => { - quiesced = true; - thunkCalls++; - return fakeProvider("v2"); - }, - true, - false, - ); - await expect(ack).resolves.toBeUndefined(); - expect(calls).toBe(0); - expect(quiesced).toBe(false); - expect(thunkCalls).toBe(0); - }); - - it("replaceProvider calls the replacement resolver exactly once", async () => { - const order: string[] = []; - let thunkCalls = 0; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - - const ready = deferred(); - const ack = host.replaceProvider(fakeProvider("v1"), async () => { - await ready.promise; - thunkCalls++; - return fakeProvider("v2"); - }); - - await tick(); - // Parked on the barrier: the teardown ran but the thunk is NOT called - // until the source decides (post-barrier), so the add version is chosen - // from post-barrier state. - expect(thunkCalls).toBe(0); - expect(order).toEqual(["remove:v1"]); - - ready.resolve(); - await ack; - expect(thunkCalls).toBe(1); - expect(order).toEqual(["remove:v1", "add:v2"]); - }); - - it("replaceProvider releases the lock and rejects when the thunk throws (teardown already applied)", async () => { - const order: string[] = []; - let quiesced = false; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (p) => { - order.push(`add:${p.getAppAgentNames()[0]}`); - }, - applyRemove: async (p) => { - order.push(`remove:${p.getAppAgentNames()[0]}`); - }, - }); - - const ack = host.replaceProvider(fakeProvider("v1"), async () => { - quiesced = true; - throw new Error("build v2 failed"); - }); - await expect(ack).rejects.toThrow(/build v2 failed/); - // The teardown leg ran + quiesced (irreversible); the add is skipped. - expect(order).toEqual(["remove:v1"]); - expect(quiesced).toBe(true); - // The single command-lock slot was released on the throw: a following op - // still applies (the lock was not leaked). - await host.addProvider(fakeProvider("after")); - expect(order).toEqual(["remove:v1", "add:after"]); - }); - - it("replaceProvider suppresses per-leg notify and defaults dropConfig=false (update)", async () => { - const seen: { - addNotify?: boolean; - removeNotify?: boolean; - dropConfig?: boolean; - } = {}; - const replaceCalls: Array<{ - op: "update" | "remove" | undefined; - oldName: string; - newName: string | undefined; - }> = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async (_p, n) => { - seen.addNotify = n; - }, - applyRemove: async (_p, n, d) => { - seen.removeNotify = n; - seen.dropConfig = d; - }, - notifyReplace: (op, oldProvider, newProvider) => { - replaceCalls.push({ - op, - oldName: oldProvider.getAppAgentNames()[0], - newName: newProvider?.getAppAgentNames()[0], - }); - }, - }); - // No dropConfig supplied → the update default (Model B: preserve the - // enable preference across a version bump) must thread `false`. The - // per-leg notifications are SUPPRESSED (false) in favor of the single - // consolidated `notifyReplace` call. - await host.replaceProvider( - fakeProvider("v1"), - async () => fakeProvider("v2"), - true, - ); - expect(seen).toEqual({ - addNotify: false, - removeNotify: false, - dropConfig: false, - }); - // One consolidated "update" notification (v1 -> v2). - expect(replaceCalls).toEqual([ - { op: "update", oldName: "v1", newName: "v2" }, - ]); - }); - - it("replaceProvider consolidated notify reports 'remove' for an uninstall (old → ∅)", async () => { - const replaceCalls: Array<{ - op: "update" | "remove" | undefined; - oldName: string; - newName: string | undefined; - }> = []; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => {}, - applyRemove: async () => {}, - notifyReplace: (op, oldProvider, newProvider) => { - replaceCalls.push({ - op, - oldName: oldProvider.getAppAgentNames()[0], - newName: newProvider?.getAppAgentNames()[0], - }); - }, - }); - await host.replaceProvider( - fakeProvider("gone"), - async () => undefined, - true, - true, - ); - expect(replaceCalls).toEqual([ - { op: "remove", oldName: "gone", newName: undefined }, - ]); - }); - - it("replaceProvider consolidated notify stays silent on a rollback (old restored)", async () => { - const replaceCalls: unknown[] = []; - const old = fakeProvider("v1"); - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => {}, - applyRemove: async () => {}, - notifyReplace: (op) => { - replaceCalls.push(op); - }, - }); - // The resolver returns the SAME old provider (a rollback restores the - // exact version): nothing changed, so op is undefined (stay silent). - await host.replaceProvider(old, async () => old, true); - expect(replaceCalls).toEqual([undefined]); - }); - - it("replaceProvider does not notify when notify=false", async () => { - let called = false; - const host = new AppAgentHostApplicator(createLimiter(1), { - applyAdd: async () => {}, - applyRemove: async () => {}, - notifyReplace: () => { - called = true; - }, - }); - await host.replaceProvider( - fakeProvider("v1"), - async () => fakeProvider("v2"), - false, - ); - expect(called).toBe(false); - }); -}); - -describe("AppAgentManager.removeProvider", () => { - it("is a no-op for a provider whose agent was never registered", async () => { - const manager = new AppAgentManager(undefined, new PortRegistrar()); - // Unknown provider: removeAgent skips names it does not know, so the - // whole removeProvider is a no-op that must not throw. - await expect( - manager.removeProvider(fakeProvider("never-registered")), - ).resolves.toBeUndefined(); - expect(manager.getAppAgentNames()).toEqual([]); - }); - - it("tears down a registered single-agent provider (schemas dropped, agent unloaded)", async () => { - const manager = new AppAgentManager(undefined, new PortRegistrar()); - let unloaded: string | undefined; - const provider: AppAgentProvider = { - getAppAgentNames: () => ["foo"], - getAppAgentManifest: async () => ({}) as any, - loadAppAgent: async () => ({}) as any, - unloadAppAgent: async (name: string) => { - unloaded = name; - }, - }; - - // Seed a registered agent record + its action config directly (the same - // internals-seeding approach used by agentReadiness.spec.ts), so we can - // exercise removeProvider's teardown without the heavy addProvider load - // path. `appAgent` is set so removeAgent reaches unloadAppAgent. - (manager as any).agents.set("foo", { - name: "foo", - provider, - schemas: new Set(["foo"]), - actions: new Set(), - commands: false, - manifest: { description: "foo", emojiChar: "🤖" }, - appAgent: {}, - schemaErrors: new Map(), - }); - (manager as any).actionConfigs.set("foo", { - schemaName: "foo", - transient: false, - }); - - expect(manager.getAppAgentNames()).toEqual(["foo"]); - expect(manager.getSchemaNames()).toContain("foo"); - - await manager.removeProvider(provider); - - // Agent gone, schema config dropped, provider unloaded by identity. - expect(manager.getAppAgentNames()).toEqual([]); - expect(manager.getSchemaNames()).not.toContain("foo"); - expect(unloaded).toBe("foo"); - }); -}); - -describe("emitAgentChangeNotification (sibling system messages, )", () => { - function captureClientIO() { - const messages: string[] = []; - const events: string[] = []; - return { - messages, - events, - clientIO: { - notify: ( - _requestId: unknown, - event: string, - message: string, - ) => { - events.push(event); - messages.push(message); - }, - } as any, - }; - } - - it("a fanned-out add to a sibling reports disabled + how to enable", () => { - const { messages, clientIO } = captureClientIO(); - emitAgentChangeNotification( - clientIO, - "add", - fakeProvider("foo"), - false, - ); - expect(messages).toEqual([ - "Agent 'foo' was added — disabled (`@config agent foo` to enable).", - ]); - }); - - it("an enabled add reports plainly (no config hint)", () => { - const { messages, clientIO } = captureClientIO(); - emitAgentChangeNotification(clientIO, "add", fakeProvider("foo"), true); - expect(messages).toEqual(["Agent 'foo' was added — enabled."]); - }); - - it("emits an Inline event so the message shows in the conversation", () => { - const { events, clientIO } = captureClientIO(); - emitAgentChangeNotification(clientIO, "add", fakeProvider("foo"), true); - expect(events).toEqual([AppAgentEvent.Inline]); - }); - - it("a fanned-out remove reports removal", () => { - const { messages, clientIO } = captureClientIO(); - emitAgentChangeNotification( - clientIO, - "remove", - fakeProvider("foo"), - false, - ); - expect(messages).toEqual(["Agent 'foo' was removed."]); - }); - - it("an enabled update reports plainly (no config hint)", () => { - const { messages, clientIO } = captureClientIO(); - emitAgentChangeNotification( - clientIO, - "update", - fakeProvider("foo"), - true, - ); - expect(messages).toEqual(["Agent 'foo' was updated."]); - }); - - it("a disabled update reports updated + how to enable", () => { - const { messages, clientIO } = captureClientIO(); - emitAgentChangeNotification( - clientIO, - "update", - fakeProvider("foo"), - false, - ); - expect(messages).toEqual([ - "Agent 'foo' was updated — disabled (`@config agent foo` to enable).", - ]); - }); -}); - -describe("reconcileKnownAgents (load-time reconciliation, Model B)", () => { - function makeContext(opts: { - available: string[]; - known: string[] | undefined; - // Agents considered "enabled" for the notification wording. - enabled?: string[]; - }) { - const enabled = new Set(opts.enabled ?? opts.available); - const messages: string[] = []; - let saved: string[] | undefined; - const context = { - agents: { - getAppAgentNames: () => opts.available, - isCommandEnabled: (name: string) => enabled.has(name), - getSchemaNames: () => [] as string[], - isSchemaEnabled: () => false, - }, - session: { - getKnownAgents: () => opts.known, - setKnownAgents: (names: readonly string[]) => { - saved = [...names]; - }, - }, - clientIO: { - notify: ( - _requestId: unknown, - _event: unknown, - message: string, - ) => { - messages.push(message); - }, - }, - } as any; - return { context, messages, getSaved: () => saved }; - } - - it("records a silent baseline when no known set exists", () => { - const { context, messages, getSaved } = makeContext({ - available: ["browser", "email"], - known: undefined, - }); - reconcileKnownAgents(context); - expect(messages).toEqual([]); - expect(getSaved()).toEqual(["browser", "email"]); - }); - - it("reports an agent that appeared while offline (enabled)", () => { - const { context, messages, getSaved } = makeContext({ - available: ["browser", "email"], - known: ["browser"], - }); - reconcileKnownAgents(context); - expect(messages).toEqual(["Agent set changed: email added — enabled."]); - expect(getSaved()).toEqual(["browser", "email"]); - }); - - it("reports an appeared agent as disabled with how-to-enable", () => { - const { context, messages } = makeContext({ - available: ["browser", "email"], - known: ["browser"], - enabled: ["browser"], - }); - reconcileKnownAgents(context); - expect(messages).toEqual([ - "Agent set changed: email added — disabled (`@config agent email` to enable).", - ]); - }); - - it("reports an agent that disappeared while offline", () => { - const { context, messages, getSaved } = makeContext({ - available: ["browser"], - known: ["browser", "email"], - }); - reconcileKnownAgents(context); - expect(messages).toEqual(["Agent set changed: email removed."]); - expect(getSaved()).toEqual(["browser"]); - }); - - it("summarizes mixed add + remove in a single message", () => { - const { context, messages } = makeContext({ - available: ["browser", "player"], - known: ["browser", "email"], - }); - reconcileKnownAgents(context); - expect(messages).toEqual([ - "Agent set changed: player added — enabled; email removed.", - ]); - }); - - it("stays silent (but re-baselines) when nothing changed", () => { - const { context, messages, getSaved } = makeContext({ - available: ["browser", "email"], - known: ["browser", "email"], - }); - reconcileKnownAgents(context); - expect(messages).toEqual([]); - expect(getSaved()).toEqual(["browser", "email"]); - }); -}); diff --git a/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts b/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts new file mode 100644 index 000000000..49726327e --- /dev/null +++ b/ts/packages/dispatcher/dispatcher/test/appAgentLifecycle.spec.ts @@ -0,0 +1,244 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, expect, it } from "@jest/globals"; +import { AppAgentEvent } from "@typeagent/agent-sdk"; +import { AppAgentProvider } from "../src/agentProvider/agentProvider.js"; +import { AppAgentManager } from "../src/context/appAgentManager.js"; +import { + emitAgentChangeNotification, + reconcileKnownAgents, +} from "../src/context/commandHandlerContext.js"; +import { PortRegistrar } from "../src/context/portRegistrar.js"; + +function fakeProvider(name: string): AppAgentProvider { + return { + getAppAgentNames: () => [name], + getAppAgentManifest: async () => ({}) as any, + loadAppAgent: async () => ({}) as any, + unloadAppAgent: async () => {}, + }; +} + +describe("AppAgentManager.removeProvider", () => { + it("is a no-op for a provider whose agent was never registered", async () => { + const manager = new AppAgentManager(undefined, new PortRegistrar()); + + await expect( + manager.removeProvider(fakeProvider("never-registered")), + ).resolves.toBeUndefined(); + expect(manager.getAppAgentNames()).toEqual([]); + }); + + it("tears down a registered single-agent provider", async () => { + const manager = new AppAgentManager(undefined, new PortRegistrar()); + let unloaded: string | undefined; + const provider: AppAgentProvider = { + getAppAgentNames: () => ["foo"], + getAppAgentManifest: async () => ({}) as any, + loadAppAgent: async () => ({}) as any, + unloadAppAgent: async (name: string) => { + unloaded = name; + }, + }; + + (manager as any).agents.set("foo", { + name: "foo", + provider, + schemas: new Set(["foo"]), + actions: new Set(), + commands: false, + manifest: { description: "foo", emojiChar: "🤖" }, + appAgent: {}, + schemaErrors: new Map(), + }); + (manager as any).actionConfigs.set("foo", { + schemaName: "foo", + transient: false, + }); + + await manager.removeProvider(provider); + + expect(manager.getAppAgentNames()).toEqual([]); + expect(manager.getSchemaNames()).not.toContain("foo"); + expect(unloaded).toBe("foo"); + }); +}); + +describe("emitAgentChangeNotification", () => { + function captureClientIO() { + const messages: string[] = []; + const events: string[] = []; + return { + messages, + events, + clientIO: { + notify: ( + _requestId: unknown, + event: string, + message: string, + ) => { + events.push(event); + messages.push(message); + }, + } as any, + }; + } + + it("reports a disabled add with the enable command", () => { + const { messages, clientIO } = captureClientIO(); + emitAgentChangeNotification( + clientIO, + "add", + fakeProvider("foo"), + false, + ); + expect(messages).toEqual([ + "Agent 'foo' was added — disabled (`@config agent foo` to enable).", + ]); + }); + + it("reports an enabled add", () => { + const { messages, clientIO } = captureClientIO(); + emitAgentChangeNotification(clientIO, "add", fakeProvider("foo"), true); + expect(messages).toEqual(["Agent 'foo' was added — enabled."]); + }); + + it("uses an inline event", () => { + const { events, clientIO } = captureClientIO(); + emitAgentChangeNotification(clientIO, "add", fakeProvider("foo"), true); + expect(events).toEqual([AppAgentEvent.Inline]); + }); + + it("reports removal", () => { + const { messages, clientIO } = captureClientIO(); + emitAgentChangeNotification( + clientIO, + "remove", + fakeProvider("foo"), + false, + ); + expect(messages).toEqual(["Agent 'foo' was removed."]); + }); + + it("reports enabled and disabled updates", () => { + const enabled = captureClientIO(); + emitAgentChangeNotification( + enabled.clientIO, + "update", + fakeProvider("foo"), + true, + ); + expect(enabled.messages).toEqual(["Agent 'foo' was updated."]); + + const disabled = captureClientIO(); + emitAgentChangeNotification( + disabled.clientIO, + "update", + fakeProvider("foo"), + false, + ); + expect(disabled.messages).toEqual([ + "Agent 'foo' was updated — disabled (`@config agent foo` to enable).", + ]); + }); +}); + +describe("reconcileKnownAgents", () => { + function makeContext(options: { + available: string[]; + known: string[] | undefined; + enabled?: string[]; + }) { + const enabled = new Set(options.enabled ?? options.available); + const messages: string[] = []; + let saved: string[] | undefined; + const context = { + agents: { + getAppAgentNames: () => options.available, + isCommandEnabled: (name: string) => enabled.has(name), + getSchemaNames: () => [] as string[], + isSchemaEnabled: () => false, + }, + session: { + getKnownAgents: () => options.known, + setKnownAgents: (names: readonly string[]) => { + saved = [...names]; + }, + }, + clientIO: { + notify: ( + _requestId: unknown, + _event: unknown, + message: string, + ) => { + messages.push(message); + }, + }, + } as any; + return { context, messages, getSaved: () => saved }; + } + + it("records a silent baseline when no known set exists", () => { + const { context, messages, getSaved } = makeContext({ + available: ["browser", "email"], + known: undefined, + }); + reconcileKnownAgents(context); + expect(messages).toEqual([]); + expect(getSaved()).toEqual(["browser", "email"]); + }); + + it("reports agents that appeared while offline", () => { + const { context, messages, getSaved } = makeContext({ + available: ["browser", "email"], + known: ["browser"], + }); + reconcileKnownAgents(context); + expect(messages).toEqual(["Agent set changed: email added — enabled."]); + expect(getSaved()).toEqual(["browser", "email"]); + }); + + it("reports a disabled appeared agent with the enable command", () => { + const { context, messages } = makeContext({ + available: ["browser", "email"], + known: ["browser"], + enabled: ["browser"], + }); + reconcileKnownAgents(context); + expect(messages).toEqual([ + "Agent set changed: email added — disabled (`@config agent email` to enable).", + ]); + }); + + it("reports agents that disappeared while offline", () => { + const { context, messages, getSaved } = makeContext({ + available: ["browser"], + known: ["browser", "email"], + }); + reconcileKnownAgents(context); + expect(messages).toEqual(["Agent set changed: email removed."]); + expect(getSaved()).toEqual(["browser"]); + }); + + it("summarizes mixed changes", () => { + const { context, messages } = makeContext({ + available: ["browser", "player"], + known: ["browser", "email"], + }); + reconcileKnownAgents(context); + expect(messages).toEqual([ + "Agent set changed: player added — enabled; email removed.", + ]); + }); + + it("stays silent when nothing changed", () => { + const { context, messages, getSaved } = makeContext({ + available: ["browser", "email"], + known: ["browser", "email"], + }); + reconcileKnownAgents(context); + expect(messages).toEqual([]); + expect(getSaved()).toEqual(["browser", "email"]); + }); +}); diff --git a/ts/packages/dispatcher/dispatcher/test/appAgentProviderSetController.spec.ts b/ts/packages/dispatcher/dispatcher/test/appAgentProviderSetController.spec.ts new file mode 100644 index 000000000..79bcdcff5 --- /dev/null +++ b/ts/packages/dispatcher/dispatcher/test/appAgentProviderSetController.spec.ts @@ -0,0 +1,296 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { describe, expect, it } from "@jest/globals"; +import { createLimiter } from "@typeagent/common-utils"; +import { + AppAgentProvider, + AppAgentProviderSetMutation, +} from "../src/agentProvider/agentProvider.js"; +import { + AppAgentProviderSetApplyFns, + AppAgentProviderSetControllerImpl, +} from "../src/context/appAgentSetController.js"; + +function fakeProvider(name: string): AppAgentProvider { + return { + getAppAgentNames: () => [name], + getAppAgentManifest: async () => ({}) as any, + loadAppAgent: async () => ({}) as any, + unloadAppAgent: async () => {}, + }; +} + +function fakeMultiProvider(...names: string[]): AppAgentProvider { + return { + getAppAgentNames: () => names, + getAppAgentManifest: async () => ({}) as any, + loadAppAgent: async () => ({}) as any, + unloadAppAgent: async () => {}, + }; +} + +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +const tick = () => new Promise((resolve) => setTimeout(resolve, 0)); + +function createController(apply?: Partial) { + return new AppAgentProviderSetControllerImpl(createLimiter(1), { + applyAdd: async () => {}, + applyRemove: async () => {}, + ...apply, + }); +} + +describe("AppAgentProviderSetController", () => { + it("exposes add and remove only through runExclusive", async () => { + const calls: string[] = []; + const controller = createController({ + applyAdd: async (provider) => { + calls.push(`add:${provider.getAppAgentNames()[0]}`); + }, + applyRemove: async (provider) => { + calls.push(`remove:${provider.getAppAgentNames()[0]}`); + }, + }); + + const result = await controller.runExclusive(async (mutation) => { + await mutation.removeProvider(fakeProvider("old")); + await mutation.addProvider(fakeProvider("new")); + return "done"; + }); + + expect(result).toEqual({ status: "completed", value: "done" }); + expect(calls).toEqual(["remove:old", "add:new"]); + }); + + it("holds the command lock across remove, wait, and add", async () => { + const commandLock = createLimiter(1); + const calls: string[] = []; + const controller = new AppAgentProviderSetControllerImpl(commandLock, { + applyAdd: async () => { + calls.push("add"); + }, + applyRemove: async () => { + calls.push("remove"); + }, + }); + const decision = deferred(); + const replacement = controller.runExclusive(async (mutation) => { + await mutation.removeProvider(fakeProvider("foo")); + calls.push("waiting"); + await decision.promise; + await mutation.addProvider(fakeProvider("foo")); + }); + const command = commandLock(async () => { + calls.push("command"); + }); + + await tick(); + expect(calls).toEqual(["remove", "waiting"]); + decision.resolve(); + await Promise.all([replacement, command]); + expect(calls).toEqual(["remove", "waiting", "add", "command"]); + }); + + it("rejects a captured mutation after its callback returns", async () => { + const controller = createController(); + let captured!: AppAgentProviderSetMutation; + await controller.runExclusive((mutation) => { + captured = mutation; + }); + + await expect( + captured.addProvider(fakeProvider("late")), + ).rejects.toThrow( + "The app-agent-provider-set mutation capability is no longer active.", + ); + }); + + it("waits for accepted unawaited actions before releasing the lock", async () => { + const gate = deferred(); + const controller = createController({ + applyAdd: async () => { + await gate.promise; + }, + }); + let completed = false; + const running = controller + .runExclusive((mutation) => { + void mutation.addProvider(fakeProvider("foo")); + }) + .then(() => { + completed = true; + }); + + await tick(); + expect(completed).toBe(false); + gate.resolve(); + await running; + expect(completed).toBe(true); + }); + + it("propagates an accepted unawaited action failure", async () => { + const failure = new Error("add failed"); + const controller = createController({ + applyAdd: async () => { + throw failure; + }, + }); + + await expect( + controller.runExclusive((mutation) => { + void mutation.addProvider(fakeProvider("foo")); + }), + ).rejects.toBe(failure); + }); + + it("rejects recursive exclusive entry", async () => { + const controller = createController(); + await controller.runExclusive(async () => { + await expect( + controller.runExclusive(async () => {}), + ).rejects.toThrow( + "runExclusive cannot be called recursively for the same app-agent-provider-set controller.", + ); + }); + }); + + it("rejects providers that do not expose exactly one agent", async () => { + const controller = createController(); + await expect( + controller.runExclusive((mutation) => + mutation.addProvider(fakeMultiProvider("one", "two")), + ), + ).rejects.toThrow(/requires a single-agent provider/); + }); + + it("revokes the active mutation when closed", async () => { + const controller = createController(); + const entered = deferred(); + const resume = deferred(); + const running = controller.runExclusive(async (mutation) => { + entered.resolve(mutation); + await resume.promise; + await mutation.addProvider(fakeProvider("late")); + }); + const mutation = await entered.promise; + + controller.dispose(); + await expect( + mutation.addProvider(fakeProvider("closed")), + ).rejects.toThrow("The app-agent-provider-set controller is closed."); + resume.resolve(); + await expect(running).rejects.toThrow( + "The app-agent-provider-set controller is closed.", + ); + }); + + it("returns closed without running a queued callback after dispose", async () => { + const commandLock = createLimiter(1); + const controller = new AppAgentProviderSetControllerImpl(commandLock, { + applyAdd: async () => {}, + applyRemove: async () => {}, + }); + const gate = deferred(); + const busy = commandLock(() => gate.promise); + let called = false; + const queued = controller.runExclusive(() => { + called = true; + }); + + controller.dispose(); + expect(await queued).toEqual({ status: "closed" }); + expect(called).toBe(false); + gate.resolve(); + await busy; + }); + + it("reports an add from the net mutation", async () => { + const notifications: string[] = []; + const provider = fakeProvider("foo"); + const controller = createController({ + notifyChange: (kind) => notifications.push(kind), + }); + + await controller.runExclusive((mutation) => + mutation.addProvider(provider, { notify: true }), + ); + expect(notifications).toEqual(["add"]); + }); + + it("reports remove plus a different add as one update", async () => { + const notifications: string[] = []; + const oldProvider = fakeProvider("foo"); + const newProvider = fakeProvider("foo"); + const controller = createController({ + notifyChange: (kind) => notifications.push(kind), + }); + + await controller.runExclusive(async (mutation) => { + await mutation.removeProvider(oldProvider, { notify: true }); + await mutation.addProvider(newProvider, { notify: true }); + }); + expect(notifications).toEqual(["update"]); + }); + + it("stays silent when rollback restores the same provider", async () => { + const notifications: string[] = []; + const finalized: string[] = []; + const provider = fakeProvider("foo"); + const controller = createController({ + applyRemove: async () => ({ + agentNames: ["foo"], + schemaNames: ["foo.schema"], + }), + finalizeRemove: ({ agentNames }) => finalized.push(agentNames[0]), + notifyChange: (kind) => notifications.push(kind), + }); + + await controller.runExclusive(async (mutation) => { + await mutation.removeProvider(provider, { notify: true }); + await mutation.addProvider(provider, { notify: true }); + }); + expect(notifications).toEqual([]); + expect(finalized).toEqual([]); + }); + + it("finalizes config cleanup for a net removal", async () => { + const finalized: string[] = []; + const provider = fakeProvider("foo"); + const controller = createController({ + applyRemove: async () => ({ + agentNames: ["foo"], + schemaNames: ["foo.schema"], + }), + finalizeRemove: ({ agentNames }) => finalized.push(agentNames[0]), + }); + + await controller.runExclusive((mutation) => + mutation.removeProvider(provider), + ); + expect(finalized).toEqual(["foo"]); + }); + + it("stays silent when an add is canceled by remove in the same run", async () => { + const notifications: string[] = []; + const provider = fakeProvider("foo"); + const controller = createController({ + notifyChange: (kind) => notifications.push(kind), + }); + + await controller.runExclusive(async (mutation) => { + await mutation.addProvider(provider, { notify: true }); + await mutation.removeProvider(provider, { notify: true }); + }); + expect(notifications).toEqual([]); + }); +}); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 14ff469e9..3dc768a98 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -4341,6 +4341,9 @@ importers: '@typeagent/config': specifier: workspace:* version: link:../config + '@typeagent/dispatcher-types': + specifier: workspace:* + version: link:../dispatcher/types agent-cache: specifier: workspace:* version: link:../cache