Files

T

Tianyi Cui 3633cf90c8 fix(review): contain provider-removed listener failures; pin the model-via-request path

ds-review-bot round 2, both warnings:

- subagent/provider-removed now routes through emitLifecycle (per-listener
  containment, the subagent/start|end precedent) instead of raw ctx.emit,
  whose dispatch halts on the first throw: a throwing subscriber can no
  longer starve a later mirror into keeping a stale tool, nor disrupt the
  backend fiber's teardown mid-disposer. provider-added deliberately keeps
  propagation (register-time rollback semantics, like the system-prompt
  registries); the asymmetry is documented on emitLifecycle, the event
  JSDoc, and the provider-lifecycle RFC.

- The documented model-via-agent/request fallback composes with a
  {{model}} persona via the ownership rule itself: the plugin supplying
  the model late states it early on the system-prompt/assemble waterfall.
  Declined re-ordering render after agent/request — it would break the
  agent/pre-step contract (compaction must measure the prompt the model
  sees). New loop test pins the supply path end-to-end; the RFC's
  {{model}} consequence bullet now covers supply as well as switch.

2026-07-06 00:34:47 +08:00

5.8 KiB

Raw Blame History

RFC: Subagent provider-lifecycle events — `subagent/provider-added` / `subagent/provider-removed`

Status: implemented

Problem

The prompt-variables RFC makes dsh-tool-subagent DERIVE its model-facing wording from its provider: SubagentProvider.inheritsParentContext (spawn/ACP false, fork true) drives both the tool description and the prompt parameter description (providerWording), so the fork tool stops lying about context inheritance. That fix created a cross-fiber data dependency: a tool's description is fixed at TOOL REGISTRATION (deliberately — the description is where tool-choice guidance lives), but the provider arrives on its own plugin fiber, on no particular schedule.

The first implementation resolved the provider at the tool plugin's apply time and threw when it was absent — an implicit load-order requirement ("list the backend before the tool in cordis.yml"). Review reproduced the failure that requirement hides: the cordis Loader starts sibling entries CONCURRENTLY (Promise.all over the group) and Entry.init() does not await activation, so a backend whose activation is delayed leaves the tool's fiber permanently failed even when "listed first". The ordering the requirement leaned on is not a contract the Loader offers — "async state is not synchronous state" (defensive patterns).

Decision

The registry announces provider membership as typed events, and the consumer mirrors them instead of assuming order:

subagent/provider-added(provider) — a provider became resolvable in the ctx.subagents registry. Emitted on registration.
subagent/provider-removed(name) — a provider left the registry (its plugin's fiber was disposed — an unload or an HMR reload). Emitted from the registration's disposer.

dsh-tool-subagent mirrors its named provider's lifecycle: it registers the tool when the provider is (or becomes) available — deriving the wording from that provider at that moment — unregisters the tool when the provider goes away, and re-derives on re-registration (HMR reload). While the provider is absent the tool does not exist, which cannot lie to the model. There is deliberately NO load-order requirement left to document: the events make the ordering question disappear instead of pinning it.

The events also complete the seam's vocabulary: ctx.subagents is a named registry on which multiple delegation backends coexist (spawn, fork, acp), and a registry whose contents other plugins derive state from should announce membership changes as typed events rather than requiring polling or load-order faith.

Alternatives considered

Resolving the provider at apply time and throwing when absent (a load-order requirement) — the first implementation, rejected after review reproduced the failure above. Documenting the requirement ("list backends first") would pin a guarantee the Loader does not make.
Retrying the lookup (poll until the provider appears) — converges eventually but invents a private readiness protocol beside the one the framework already has (effect registration + disposal); it also cannot notice a provider LEAVING, so HMR would strand a tool whose wording describes a disposed backend.
Section-only subagent wording, lazily resolved at assemble time — tolerates any load order too, but moves tool-choice guidance out of the DESCRIPTION, contradicting the ownership rule the prompt-variables RFC establishes (per-tool semantics and when-to-use belong in the description). Reactive registration keeps the description authoritative AND order-free.
Keying wording off the provider NAME instead of the provider object — providerName is itself config, so a renamed provider silently gets the wrong words; deriving from the resolved provider's own inheritsParentContext cannot drift.

Consequences

Consumers deriving state from a named provider react to subagent/provider-added/-removed instead of reading the registry at apply time; dsh-tool-subagent is the reference implementation.
The two emits carry asymmetric failure semantics, deliberately. provider-removed fires inside the registration's disposer and is delivered with PER-LISTENER containment (the service's emitLifecycle, not raw ctx.emit, which halts dispatch on the first throw): a throwing subscriber is logged, never starves a later mirror into holding a stale tool, and never disrupts the backend fiber's teardown — dispose reaches quiescence. provider-added propagates: it fires at registration time, where a throwing listener unwinds the yielded rollback — the same fail-loud register-time semantics as the system-prompt registries. The run-time backstop bounds what a stale mirror could cost anyway: start() re-resolves the provider by name per run, so a tool that outlived its provider fails that call cleanly instead of dispatching into a dead backend. The events catalog carries the exact signatures, and the producer/consumer map shows dsh-subagent emitting and dsh-tool-subagent consuming both events.
A window where the tool is absent. Between backend disposal and re-registration (an HMR reload), the model sees no subagent tool. This is the honest state — the alternative is a tool that dispatches into nothing — and the tool registry's tools/change emit keeps prompt assembly current.
Two waiting fibers sharing a toolName is an invalid config caught late. If two loads of dsh-tool-subagent name different providers but the same toolName, both wait, and whichever provider arrives first registers; the second registration throws only when ITS provider arrives. TODO(subagent-dup-toolname) in the plugin records this blast radius; the tool registry's duplicate-name rejection remains the backstop.

5.8 KiB Raw Blame History

RFC: Subagent provider-lifecycle events — subagent/provider-added / subagent/provider-removed