docs: address simplification RFC review
This commit is contained in:
Tianyi Cui
@@ -0,0 +1,39 @@
|
||||
# RFC: Stream workflow progress through tool calls
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The workflow engine intentionally emits balanced `workflow/*` observation events for run, phase, narration, and child-agent progress, but no production consumer presents them. Editors therefore show one pending workflow tool card until the final result even while the engine already reports which phase is active, what the script logged, and which children started or settled. The [dynamic-workflows decision](../../implemented/feature/2026-07-05-dynamic-workflows.md) explicitly reserves ACP progress UI for this event stream.
|
||||
|
||||
Making `dsh-acp` listen to workflow events directly would invert the capability boundary: the generic UI bridge would depend on an optional workflow package and special-case one tool name. The tool pipeline already owns the routing facts a live update needs—agent and call id—but exposes only pure pending/final presenters, so a long-running tool has no provider-neutral way to report transient UI state between them.
|
||||
|
||||
## Proposal
|
||||
|
||||
Add a live progress channel to `dsh-tools`. The registry-owned `ToolExecution` gains `reportProgress(view): boolean`, where `view` is a detached provider-neutral generic progress snapshot containing an optional replacement title and UI-facing content blocks. Progress cannot change the call's args-derived card tag, kind, raw input, locations, terminal intent, or diff intent; it updates only the live title/content within the presentation chosen up front. While the execution is active, the method validates and snapshots the view, then dispatches a contained, agent-scoped `tools/progress` observation carrying the authoritative execution identity and snapshot. Once final-result processing begins it returns `false` and emits nothing, so a late asynchronous reporter cannot overwrite a terminal card. Observer exceptions are logged and cannot fail the tool.
|
||||
|
||||
`dsh-acp` consumes `tools/progress` generically. It resolves the execution's agent through its existing agent-to-session map and emits an in-progress `tool_call_update` for the same call id. Because reporting is available only inside the tool execution pipeline, the durable `tool/call` and its ACP `tool_call` always precede the first update; closing the reporter before `tools/result` ensures no progress update follows the completed/failed card. Progress is live UI state rather than model input or durable history: session replay continues to reconstruct the pending and final cards from `tool/call` and `tool/result` without replaying transient updates.
|
||||
|
||||
`dsh-tool-workflow` becomes the first producer. It keeps a plugin-owned reducer keyed by `WorkflowRun.id`, installed synchronously after `ctx.workflows.start()` returns and before worker messages can be delivered. The reducer consumes the existing phase, log, agent-start, agent-end, and end events, reporting a replacement snapshot with the current phase, latest log line, active child labels, and completed/failed/cancelled counts. It does not accumulate a narration transcript; settled children leave the active set and become counters. The initial snapshot comes from the returned run's id/meta, and `workflow/end`, tool settlement, or plugin disposal removes the reducer entry. The six workflow events, their metadata, paired child lifecycle, run handle, cancellation channels, and observer containment remain unchanged; third-party observers can continue consuming them directly.
|
||||
|
||||
Update the tool execution/presentation docs, generated event and API catalogs, workflow package docs, and the workflow data-structure catalog. ACP integration coverage must exercise the real workflow tool and worker seam with a scripted model boundary; the primary ACP snapshot suite adds one workflow-progress scenario because this changes the editor-facing transcript.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Delete the workflow observation surface.** Rejected in [the collapse-workflow simplification](../../rejected/simplification/2026-07-12-collapse-workflow-to-foreground-core.md): the events and their balanced lifecycle are intentional, and the missing piece is a consumer.
|
||||
|
||||
**Teach ACP about workflows directly.** This could map `WorkflowRunInfo` to a session and card, but it would make the generic bridge depend on an optional capability and bypass the rule that tools own presentation intent. A tool-progress channel solves the same routing problem for every long-running tool.
|
||||
|
||||
**Persist every progress update as a session event.** That would make live narration replayable, but it would permanently enlarge logs with state whose authoritative durable outcome is already the tool call/result pair. If resumable workflow progress becomes a product requirement, it needs a workflow-journaling design rather than UI snapshots disguised as durable facts.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `ToolExecution.reportProgress()` is registry-owned, agent-scoped, snapshotting, observer-contained, and returns `false` without dispatch after terminal processing starts.
|
||||
- ACP routes progress to the correct call in the correct live session; concurrent workflows in different sessions cannot cross-talk, and no `tool_call_update` appears before its `tool_call` or after its terminal update.
|
||||
- Workflow progress shows the current phase, latest log line, active children, and outcome counts while preserving all existing `workflow/*` events and run semantics.
|
||||
- Cancellation, worker death, tool failure, session close, and plugin disposal release reducer state; replay emits only the durable pending/final card pair.
|
||||
- Unit, workflow integration, ACP integration, snapshot, typecheck, coverage, doc-sync, module-graph, build, and hygiene gates pass.
|
||||
|
||||
## Risks
|
||||
|
||||
This adds a public live-progress method and event to the tool seam, so implementations must keep the active/terminal boundary exact and detach snapshots before observers see them. A workflow can emit many progress changes; the bounded reducer avoids transcript growth but still sends one UI update per meaningful event. If measured clients need coalescing, it must be a defaulted validated bridge configuration rather than a hardcoded throttle. Transient progress intentionally disappears on replay, so the final tool result remains the only durable workflow card content.
|
||||
@@ -18,7 +18,7 @@ Make an agent's registry id equal its session id. `CreateAgentOptions` accepts o
|
||||
|
||||
The config-driven path must first settle its currently hidden resume-or-create policy. Today it uses a stable agent label and fresh UUID-suffixed session id to avoid colliding with a durable log on the next run. Under unification it must deliberately resume the fixed id, mint a fresh combined id, or expose an explicit policy; implementation must not pick silently.
|
||||
|
||||
`agent/created` and `agent/disposed` remain outside this proposal. They are paired publication lifecycle events, not identity aliases; any later consumer-free removal belongs in the dedicated [registry-event simplification](./2026-07-12-drop-unconsumed-registry-events.md) after a fresh search.
|
||||
`agent/created` and `agent/disposed` remain outside this proposal. They are paired publication lifecycle events, not identity aliases; any later consumer-free removal needs its own proposal after a fresh search.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
|
||||
@@ -1,37 +0,0 @@
|
||||
# RFC: Collapse workflows to the exercised foreground core
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The workflow capability executes foreground JavaScript that composes subagents, but it also carries an unconsumed progress-observation system. No production listener subscribes to any of the six `workflow/*` events; listeners exist only in workflow tests. Nevertheless the seam defines run/phase/agent outcome payloads, the worker sends phase/log/agent lifecycle protocol messages, the host forwards them through a `liveAgents` pairing ledger, and the engine maintains run ids solely to correlate those notifications.
|
||||
|
||||
The progress vocabulary is not merely unused; it cannot serve its only named future owner without redesign. `WorkflowRunInfo` contains `{id, meta}` but no parent agent, session, or tool-call identity, while the model-facing tool never exposes the run id. A global ACP listener could not route an event to the correct client session. `meta.phases` is never consulted, `phase(title)` does not validate against it, phase `detail`/`model` and agent `label`/`phase` feed only events, and `whenToUse` is validated and copied but never rendered or selected. `phase()` and `log()` still cross the worker boundary despite having no receiver.
|
||||
|
||||
The live handle repeats event-era data after those observers disappear. `WorkflowRun.id` has no non-event consumer, while the tool reads `run.meta.name` only to render a value it already owns as `args.meta.name`; neither belongs on the execution/cancellation handle.
|
||||
|
||||
Cancellation also has two public channels for one synchronous start. `WorkflowStartRequest.signal` is passed to the worker host, while the sole production caller separately bridges the same signal to `WorkflowRun.cancel()`. Because `start()` returns the run before control can yield, there is no readiness window that requires request-time cancellation; the duplicate signal adds host listener/disarm state without closing a race.
|
||||
|
||||
`WorkflowError.fatal` is the same speculative branch in miniature: every production construction is fatal, `fatal: false` exists only in tests, and combinators already distinguish workflow failures with `instanceof`.
|
||||
|
||||
## Proposal
|
||||
|
||||
Keep the exercised core: `agent(prompt, { schema, model })`, `parallel`, `pipeline`, `args`, concurrency/agent caps, cancellation, bounded disposal, structured results, worker isolation, and foreground tool collection. Remove all `workflow/*` events and their event-only info/outcome types; remove `phase()`, `log()`, agent `label`/`phase`, phase declarations, `whenToUse`, and their worker messages/host observers; collapse workflow metadata to the name the tool actually uses; remove event-only run ids/meta snapshots and the synthesized agent-end ledger. Shrink `WorkflowRun` to `result`, `cancel()`, and `dispose()`; the tool renders the request-owned name. Remove `WorkflowStartRequest.signal` and the worker host's input-signal listener/disarm state, retaining the caller-owned bridge from its abort signal to `run.cancel()`. Make `WorkflowError` one fatal error class without a boolean mode or `isFatalWorkflowError()` helper.
|
||||
|
||||
Amend the implemented dynamic-workflow RFC and update the seam/tool/worker READMEs, tool schema, generated catalogs and package graph, worker type-equivalence records, unit tests, and workflow snapshot/header fixtures. Progress UI work, if commissioned, starts from a correlation contract that names the parent agent/session/tool call instead of reviving this protocol unchanged.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Keep the prebuilt observation vocabulary for a future UI.** The current shape resembles Claude Code dynamic-workflow metadata, and the host deliberately pairs each forwarded agent start with either the worker's end or a synthesized terminal end. Removing it gives up compatibility-by-shape and makes progress UI a new design task, but the existing payloads still lack routable ownership, so balanced lifecycles alone cannot make the named ACP owner viable without redesign.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The workflow public seam contains only execution, cancellation, result, and disposal contracts with a production consumer.
|
||||
- No workflow event, phase/log protocol message, run-id generator, progress-only metadata, host pairing ledger, or fatal-mode branch remains.
|
||||
- The run handle has no id/meta echoes, and cancellation has one holder-owned channel after synchronous `start()` returns.
|
||||
- Parallel/pipeline behavior, caps, cancellation quiescence, worker containment, structured output, and the model-facing workflow scenarios retain coverage.
|
||||
- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass.
|
||||
|
||||
## Risks
|
||||
|
||||
This is a compile-visible contraction of the workflow DSL, event taxonomy, handle, and start request. Existing workflow calls that supply descriptive metadata, and scripts that use `phase`, `log`, or labels, must shrink; programmatic callers bridge their own abort source to the returned handle; and a future observer must add a better-correlated seam. The execution semantics that make workflows useful do not change.
|
||||
@@ -1,32 +0,0 @@
|
||||
# RFC: Drop unconsumed registry events
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
Four registry notifications are produced but have no production listener. The generated producer/consumer matrix and exact event-name searches find only declarations, emit sites, invariant metadata, tests, generated catalogs, and prose for `tools/change`, `system-prompt/change`, `skill/provider-added`, and `skill/provider-removed`.
|
||||
|
||||
No shipped path uses these signals for invalidation: request assembly deliberately reruns for every step, tool/system-prompt membership may be agent-scoped, and skill discovery reads providers on demand. The payloadless tool/system-prompt notices are also insufficient for a scoped observer because a change may be local to one agent but the event cannot identify that scope.
|
||||
|
||||
Earlier registry work retained tool/system-prompt notifications as low-cost hooks for a hypothetical live UI even while the equivalent LLM and web notifications were removed. The new evidence is that no owner has appeared, per-step assembly needs no signal, and scope-local membership has made the old payload insufficient for that hypothetical owner. This proposal does not include `subagent/provider-added`/`removed`, which `tool-subagent` consumes to tolerate concurrent sibling-plugin loading.
|
||||
|
||||
## Proposal
|
||||
|
||||
Delete the four declarations and every emit path, rollback-order branch, invariant-table entry, test, and generated catalog/matrix row that exists only for them. Remove the corresponding registry README/JSDoc contract. Where tests used an event to observe cleanup, assert public lookup or assembled output instead.
|
||||
|
||||
Amend the [agent-scope RFC](../../implemented/architecture/2026-07-08-agent-scope-contexts.md) and reconstructable-request documentation so current behavior has one home: request inputs are recomputed at the request boundary, not maintained by invalidation signals.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Keep cheap notifications for future plugins.** An external plugin could subscribe later, and provider lifecycle signals can solve sibling-load races. The subagent event demonstrates the bar: it has a real concurrent loader consumer and a payload tailored to that job. These four have neither; a future consumer should introduce the scoped identity and timing it demonstrably needs.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The generated event matrix contains no row or registry-subject inventory entry for the four notifications.
|
||||
- Tool schema assembly, system-prompt assembly, skill discovery, ordinary effect rollback/disposal, and registry lookup cleanup behave unchanged; listener-triggered rollback disappears with the events.
|
||||
- The real subagent provider lifecycle consumer remains covered.
|
||||
- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass.
|
||||
|
||||
## Risks
|
||||
|
||||
This deliberately removes pre-release plugin observation points. A future live registry UI would need a new scoped snapshot/change contract instead of subscribing to payloadless global notifications.
|
||||
@@ -0,0 +1,32 @@
|
||||
# RFC: Drop unconsumed skill provider events
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
Two skill-registry notifications are produced but have no production listener. The generated producer/consumer matrix and exact event-name searches find only declarations, emit sites, tests, generated catalogs, and prose for `skill/provider-added` and `skill/provider-removed`.
|
||||
|
||||
Skill discovery reads the current provider map on demand, provider registration synchronously clears completed catalogs, and the post-await revision check prevents stale discovery from entering the cache. No sibling plugin waits for a skill provider through these events, unlike the live `subagent/provider-added` consumer that tolerates concurrent sibling loading.
|
||||
|
||||
`tools/change` and `system-prompt/change` are explicitly outside this proposal. Existing simplification decisions retain them as intentional observation points for live tool and prompt UIs, and self-referential mounted plugins already use `tools/change`. This proposal also leaves `subagent/provider-added`/`removed` unchanged because `tool-subagent` has a production lifecycle consumer.
|
||||
|
||||
## Proposal
|
||||
|
||||
Delete the two skill-provider declarations and every emit path, rollback-order branch, test, and generated catalog/matrix row that exists only for them. Remove the corresponding skill-registry README/JSDoc contract. Where tests used an event to observe cleanup, assert provider lookup or collected output instead.
|
||||
|
||||
Amend the skill-system RFC and package documentation so provider registration is described as direct effect-owned state with cache invalidation, not as a lifecycle notification contract.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Keep skill-provider notifications for future plugins.** A third-party plugin could observe provider availability, but direct provider registration and on-demand lookup are the extension contract; no current consumer needs a push signal. If a future sibling-load race appears, it can introduce a notification with the identity and readiness semantics that consumer requires, as the subagent registry did.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The generated event matrix contains no row for `skill/provider-added` or `skill/provider-removed`.
|
||||
- Skill discovery, direct runtime registration, provider effect rollback/disposal, cache invalidation, and registry lookup cleanup behave unchanged; listener-triggered rollback disappears with the events.
|
||||
- `tools/change`, `system-prompt/change`, and the real subagent provider lifecycle consumer remain documented and covered.
|
||||
- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass.
|
||||
|
||||
## Risks
|
||||
|
||||
This removes pre-release skill-provider observation points while retaining both ways third-party plugins contribute skills: direct runtime registration and provider registration. A future consumer that needs live provider availability must add a purpose-built notification rather than relying on these generic events.
|
||||
@@ -1,27 +0,0 @@
|
||||
# RFC: Prune unused skill registry surface
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The skill service's embedded-runtime subsystem has zero production caller of `ctx.skills.register()`. It adds a reserved `runtime` provider name, a runtime map/rank/source, duplicate policy, a second revision in cache keys, normalization, disposers, and tests alongside the provider seam every shipped skill already uses. `SkillSummary.whenToUse` and candidate/definition `path` are parsed and copied but never read by a production consumer: the model catalog renders name/description, resource loading uses `resourceBase`, and providers own their locator. The deliberately open `metadata` extension point stays.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove `SkillService.register()`, `SkillRegistration`, the runtime pseudo-provider and reserved-name rules, runtime revisions/cache branches, and runtime-only source/rank normalization. Tests that need an embedded skill register a small real provider. Retain `providerRevision` as the in-flight discovery epoch, but key completed catalogs by cwd alone: every provider mutation synchronously clears the cache, and the post-await revision comparison already prevents inserting stale work. Remove `whenToUse`, `SkillCandidate.path`, and `SkillDefinition.path` from the skill contract and local-provider copies while retaining provider locator/root paths; retain `metadata`, `disableModelInvocation`, `source`, `provider`, `locator`, and `resourceBase` as either deliberate extension vocabulary or production-consumed fields.
|
||||
|
||||
Amend the skill-system RFC, README, JSDoc, catalogs, and tests. Agent-scoped system-prompt sections, tool providers, and variables are explicitly outside this proposal: final #224 made all three part of the `setup(agent.ctx)` contributor contract, so absence of a fixed in-repo scoped registration is not evidence of non-consumption.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Keep runtime skill registration for embedders.** It is a deliberate synchronous direct-definition convenience in the implemented skill RFC. A small provider wrapper can expose the same embedded data under effect-owned lifetime, but it must implement async `list()`/`get()`, carry provider identity, and accept provider duplicate semantics. The proposal chooses that one regular path over preserving a second ranking, validation, cache-invalidation, and lookup path.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- Skill collection has one provider-backed path, a cwd-only completed-cache key, and a revision epoch only for in-flight invalidation; retained skill fields have a production reader or a recorded deliberate extension contract.
|
||||
- Agent-scoped prompt sections, variables, tool providers, tool guards, and structured-output commit behavior in native and Code Mode remain unchanged.
|
||||
- Typecheck, coverage, snapshots, doc-sync, module-graph verification, build, and hygiene pass.
|
||||
|
||||
## Risks
|
||||
|
||||
This is a compile-visible contraction of the pre-release skill registry. External programmatic `list()`/`get()` consumers lose `whenToUse` routing hints and candidate/definition `path`; the shipped model catalog never renders them, and resource resolution keeps its explicit `resourceBase` plus provider-owned opaque locator, but those fields are not observationally identical. Skill-local frontmatter parsing must continue to preserve and validate the supported metadata schema, and external providers remain able to supply embedded, filesystem, remote, or other skill sources.
|
||||
Reference in New Issue
Block a user