docs: propose simplification RFCs
This commit is contained in:
Tianyi Cui
@@ -51,6 +51,10 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r
|
||||
|---|---|
|
||||
| [Unify the agent id and the session id](proposed/simplification/2026-06-20-unify-agent-and-session-id.md) | 2026-06-20 |
|
||||
| [Stop mirroring durable boundaries as agent events](proposed/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) | 2026-06-20 |
|
||||
| [Narrow the subagent seam to synchronous collect](proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md) | 2026-07-04 |
|
||||
| [Drop idle registry observation surfaces](proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md) | 2026-07-04 |
|
||||
| [Prune the bash task roster from the public seam](proposed/simplification/2026-07-04-prune-bash-task-roster.md) | 2026-07-04 |
|
||||
| [Remove defaults from the tool-schema DSL](proposed/simplification/2026-07-04-remove-tool-schema-defaults.md) | 2026-07-04 |
|
||||
|
||||
### Architecture
|
||||
|
||||
|
||||
+50
@@ -0,0 +1,50 @@
|
||||
# RFC: Drop idle registry observation surfaces
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
Several registry services expose "something changed" or "what is registered" observation surfaces with no production observer. The older [LLM adapter-change simplification](../../implemented/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) removed `llm/adapter-change` because it had declarations, emits, docs, and tests but no listener. The same pattern now exists in the remaining registry-change events: `tools/change`, `system-prompt/change`, and `web/providers-change`.
|
||||
|
||||
`tools/change` is declared by `dsh-tools` and emitted from `ToolRegistry.register()` on register and dispose ([packages/core/tools/src/index.ts](../../../../packages/core/tools/src/index.ts)). `system-prompt/change` is declared by `dsh-system-prompt` and emitted when sections or tool-schema providers register and dispose ([packages/core/system-prompt/src/index.ts](../../../../packages/core/system-prompt/src/index.ts)). `web/providers-change` is declared by `dsh-web` and emitted when search or fetch providers register and dispose ([packages/web/web/src/index.ts](../../../../packages/web/web/src/index.ts)). Grepping those event names outside `docs/rfc/**` finds declarations, emit sites, READMEs, generated catalogs, and tests, but no production listener in `packages/*/src` or examples.
|
||||
|
||||
Those events carry real complexity. Each registry yields a rollback disposer before emitting so a throwing change listener unwinds the just-added entry instead of leaking it into the registry. The packages then carry tests for listener-throw rollback paths that only the unused events can trigger. `web/providers-change` repeated the same pattern after the LLM adapter-change event was already proven unnecessary.
|
||||
|
||||
There is a related one-shot observation surface in `dsh-llm`: `ctx.llm.models()` returns registered model names, but no production caller uses it. Search finds only service docs and tests, including adapter tests that use it as a registration assertion. The shipped model-call path resolves by `options.model` at `ctx.llm.stream()` time; no UI, router, or product config enumerates model names from the service.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove the idle registry-observation surfaces that have no production consumer:
|
||||
|
||||
- Delete `tools/change`, its emits, its JSDoc/README/generated-catalog entries, and listener-throw rollback tests.
|
||||
- Delete `system-prompt/change`, its emits, its JSDoc/README/generated-catalog entries, and listener-throw rollback tests.
|
||||
- Delete `web/providers-change`, its emits, its JSDoc/README/generated-catalog entries, and listener-throw rollback tests.
|
||||
- Delete `LlmService.models()` and update LLM adapter/service tests to assert registration behavior through `stream()` resolution, duplicate-registration errors, disposal, or other behavior that a real caller observes.
|
||||
|
||||
Registration should remain effect-scoped and HMR-safe: duplicate checks still happen before mutation, the disposer still removes the registered entry, and existing consumers still read the live registry at use time. What disappears is only the speculative observer surface.
|
||||
|
||||
## What stays
|
||||
|
||||
This RFC does not remove live query or execution surfaces. `ctx.tools.schemas()` stays because the system-prompt registry and generated tool catalog use it. `ctx.web.searchStatus()` and `ctx.web.fetchStatus()` stay because `dsh-tool-web` reads them for diagnostics and they share execution-resolution semantics with `ctx.web.search()` and `ctx.web.fetch()`. `ctx.agents.list()`, `ctx.sessions.list()`, and `ctx.sessionPersistence.list()` stay because production code uses them for background-task ownership, invariant seeding, write coordination, and ACP load-cwd validation.
|
||||
|
||||
This RFC also does not touch live event seams such as `llm/stream`, `tools/execute`, `system-prompt/assemble`, `session/event`, `session/flush`, `agent/status`, or `fs/*`. Those have production listeners or are the documented extension points the architecture depends on.
|
||||
|
||||
## Why not keep them for a future UI?
|
||||
|
||||
A live tool palette, prompt-section inspector, web-provider status panel, or model picker might eventually want registry-change signals. But none exists today, and the current event payloads are so minimal that a real UI would likely need to revisit them anyway. A future observer can reintroduce the smallest signal it actually consumes, with tests that prove the observer sees it.
|
||||
|
||||
The pre-release stance cuts in favor of narrowing now. A public event with no listener is still API surface; if it survives until release, every later cleanup has to decide whether external consumers might be relying on it.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `rg "tools/change|system-prompt/change|web/providers-change" packages examples docs --glob '!docs/rfc/**'` finds no remaining declared event, emit, README row, generated-catalog entry, or test outside historical RFC text.
|
||||
- `rg "ctx\\.llm\\.models\\(|\\.models\\(\\)" packages/llm packages/core/agent-loop examples docs --glob '!docs/rfc/**'` finds no remaining `LlmService.models()` API use or docs entry.
|
||||
- Registration/disposal tests still prove HMR cleanup for tools, prompt sections/tool providers, web providers, and LLM adapters without depending on observer events.
|
||||
- The [web capability seam RFC](../../implemented/architecture/2026-06-24-web-capability-seam.md), package READMEs, the Cordis catalog, and core data-structure docs are updated to remove the event promises.
|
||||
- `pnpm run test:coverage`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation.
|
||||
|
||||
## Risks
|
||||
|
||||
- Removing emitted events is a public-surface change. The repo is unreleased, and the consumer audit says the current consumers are tests and docs only.
|
||||
- Tests lose an easy way to assert that registration happened. They should assert behavior instead: a registered tool appears in `schemas()`, a registered prompt section appears in `assemble()`, a web provider can be resolved by status/execution, and an adapter can stream for its model.
|
||||
- A future UI may need observer hooks. That is fine; the hook should return with that UI, not ahead of it.
|
||||
@@ -0,0 +1,55 @@
|
||||
# RFC: Narrow the subagent seam to synchronous collect
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The implemented [subagent seam](../../implemented/feature/2026-06-21-subagent-capability-seam.md) shipped as a named-provider registry plus a synchronous model-facing consumer, but its public contract still carries several deferred capabilities that no production caller can exercise. `dsh-tool-subagent` builds a `SubagentStartRequest` with only `prompt`, `parent`, optional `signal`, and optional `agentOptions` ([packages/subagent/tool-subagent/src/index.ts](../../../../packages/subagent/tool-subagent/src/index.ts)); it never sends `outputSchema`, `maxDepth`, or `toolFilter`, never reads `SubagentResult.structured`, and never calls `SubagentRun.sendMessage` or `SubagentRun.resume`.
|
||||
|
||||
That means the current start-time capability descriptor is mostly a contract between tests and docs. `SubagentCapabilities.outputSchema` and `toolFilter` are advertised false by every production provider, and the support mock is the only backend that exercises structured output. `depthLimit` is more subtle: the in-process providers advertise it and the shared driver can reject `request.maxDepth`, but no production tool request sets `maxDepth`, so the advertised recursion guard is dormant in the product path.
|
||||
|
||||
The service also exposes registry-observation helpers and lifecycle events that have no production consumer. Grepping `ctx.subagents.getProvider`, `ctx.subagents.list`, `subagent/start`, and `subagent/end` finds declarations, emits, docs, generated catalogs, and tests, but no listener or caller in `packages/*/src` or examples. Keeping those events is not free: `SubagentService.start()` contains custom per-listener dispatch and containment only to protect a run from lifecycle subscribers that do not exist.
|
||||
|
||||
The result is an over-wide first-cut seam: every provider and every doc page has to explain structured output, tool filtering, depth flags, steering, resume, provider enumeration, and lifecycle telemetry even though the only real product behavior is "start a named child, await its final result, cancel or dispose it."
|
||||
|
||||
## Proposal
|
||||
|
||||
Make the subagent seam describe the behavior the harness actually uses today: synchronous collect only.
|
||||
|
||||
- Remove `SubagentCapabilities` and the `SubagentProvider.capabilities` field.
|
||||
- Remove `SubagentStartRequest.outputSchema`, `maxDepth`, and `toolFilter`, along with `SubagentService.assertCapabilities`.
|
||||
- Remove `SubagentResult.structured`.
|
||||
- Remove optional runtime methods `SubagentRun.sendMessage` and `SubagentRun.resume`.
|
||||
- Remove the public `SubagentService.getProvider()` and `SubagentService.list()` helpers; provider lookup stays private to `start(name, request)`.
|
||||
- Remove `subagent/start` and `subagent/end` from the Cordis event vocabulary and delete the custom `emitLifecycle` path.
|
||||
- Remove in-process depth vocabulary that exists only to honor `maxDepth`: `AgentOptions.subagentDepth`, `depthOf`, `SubagentDepthError`, and the child-depth check in `startInProcessRun`.
|
||||
- Update `dsh-subagent-spawn`, `dsh-subagent-fork`, `dsh-subagent-acp`, `dsh-subagent-mock`, `dsh-tool-subagent`, READMEs, [docs/core-data-structures/subagent.md](../../../core-data-structures/subagent.md), and the generated Cordis catalog to the narrower contract.
|
||||
|
||||
After the cut, the provider contract is roughly: `name`, `start(request)`, and a `SubagentRun` with `{ id, result, cancel(), dispose() }`. The start request still carries the load-bearing fields: prompt, parent, optional signal, and optional child agent options.
|
||||
|
||||
## Why not keep the dormant guard?
|
||||
|
||||
The strongest counterargument is recursion: an in-process child can inherit the subagent tool and spawn again. That is a real product concern, but the current `maxDepth` field does not protect the production tool path because `dsh-tool-subagent` never sends it. A dormant guard reads like a safety property while providing none.
|
||||
|
||||
If a hard recursion limit is needed, it should come back as an actually wired product policy, probably owned by `dsh-tool-subagent` config or a tool/filtering policy that every production subagent request passes through. That future implementation should be judged against the then-current product shape, not preserved as an optional per-request field that no caller supplies.
|
||||
|
||||
## What we give up
|
||||
|
||||
Programmatic callers lose prebuilt hooks for structured subagent output, child tool scoping, live steering, follow-up resume, provider enumeration, and lifecycle telemetry. In an unreleased repo, that is an acceptable contraction: none of those hooks has a production caller, and preserving them makes every provider pay an explanation and test cost for speculative behavior.
|
||||
|
||||
The in-process backends also lose the dormant depth bookkeeping. That does not weaken the shipped model-facing behavior because no shipped request uses it today. It makes the missing recursion policy honest.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The public subagent contract contains only the synchronous collect surface: provider registration, `start(name, request)`, `SubagentRun.result`, `cancel`, and `dispose`.
|
||||
- `rg "outputSchema|structured|maxDepth|toolFilter|sendMessage|resume\\(" packages/subagent packages/support/subagent-mock packages/subagent/tool-subagent docs --glob '!docs/rfc/**'` finds no remaining contract surface except unrelated prose or new historical references.
|
||||
- `rg "subagent/start|subagent/end|getProvider\\(|ctx\\.subagents\\.list\\(" packages examples docs --glob '!docs/rfc/**'` finds no production API surface.
|
||||
- The Cordis catalog, core data-structure docs, package READMEs, and type-equivalence manifest are updated.
|
||||
- Focused subagent tests still prove registration HMR safety, duplicate provider rejection, missing provider rejection, in-process spawn/fork result collection, ACP result collection, abort bridging, and always-dispose behavior.
|
||||
- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation.
|
||||
|
||||
## Risks
|
||||
|
||||
- A future subagent UI may want lifecycle events. Reintroduce them with that UI and a payload it actually consumes rather than keeping no-op telemetry now.
|
||||
- A future structured-output subagent may want `outputSchema`. Reintroduce it when a provider and consumer both honor it end to end, including validation semantics and model-facing schema design.
|
||||
- A future recursion limit may be necessary. The replacement should be wired through the production subagent tool path instead of relying on an optional field the tool never sets.
|
||||
@@ -0,0 +1,40 @@
|
||||
# RFC: Prune the bash task roster from the public seam
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
The bash executor seam exposes four public background-task operations: direct task lookup via `get(id)`, full roster listing via `list()`, ownership lookup via `ownerOf(id)`, and id-targeted operations `readOutput(id)` / `kill(id)` ([packages/bash/bash/src/index.ts](../../../../packages/bash/bash/src/index.ts)). The model-facing `dsh-tool-bash` consumer uses `start`, `ownerOf`, `readOutput`, `kill`, `onTaskDone`, `run`, and `resolve`, but it never calls `get` or `list` in production.
|
||||
|
||||
The consumer's access policy is deliberately id based. A background task id is returned in the `bash` tool result, then later supplied to `bash_output` or `bash_kill`; those tools compare `ctx.bash.ownerOf(id)` with the calling session token before calling `readOutput(id)` or `kill(id)`. Completion notices also work from a single completed `BashTask` passed through `onTaskDone`, then scan live agents by session owner. None of those flows need a public "show me every task" API.
|
||||
|
||||
Searches for `ctx.bash.get(`, `ctx.bash.list(`, and bash `list(): BashTask[]` call sites outside tests and RFCs find only implementation, docs, generated catalogs, and tests. The local executor still needs its private `tasks` map, but exposing that map as a seam method makes every future bash backend promise roster semantics no current product code consumes.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove `BashExecutor.get(id)` and `BashExecutor.list()` from the abstract service and first implementation.
|
||||
|
||||
- Delete the abstract methods from `@deepseek-ai/dsh-bash`.
|
||||
- Delete the public methods from `@deepseek-ai/dsh-bash-local`; keep its private task map for `ownerOf`, `readOutput`, `kill`, completion, and disposal.
|
||||
- Update [docs/core-data-structures/bash.md](../../../core-data-structures/bash.md), package READMEs, and the generated Cordis catalog.
|
||||
- Rewrite tests that inspect the roster to assert behavior through returned task handles, `ownerOf`, `readOutput`, `kill`, `onTaskDone`, and disposal.
|
||||
|
||||
The remaining public background contract is direct and smaller: `start()` returns the task handle, `ownerOf(id)` answers the access-policy token, `readOutput(id)` streams incremental output, `kill(id)` stops a known task, and `onTaskDone()` reports completed tasks to interested plugins.
|
||||
|
||||
## Why not keep a roster for UI?
|
||||
|
||||
A UI might eventually show live background tasks. The current seam does not have that UI, and a raw executor-level roster is probably the wrong final surface anyway: a product UI would need task ownership, session routing, presentation state, and maybe persistence or replay. The existing `onTaskDone` callback and tool-result task ids are enough for today's behavior; a future task monitor can introduce an explicit product-facing task inventory if it actually lands.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `BashExecutor` no longer declares `get` or `list`; `LocalBashExecutor` no longer exposes them publicly.
|
||||
- `rg "ctx\\.bash\\.(get|list)\\(|\\.list\\(\\)[^\\n]*BashTask|\\.get\\([^\\n]*BashTask" packages examples docs --glob '!docs/rfc/**'` finds no public seam surface or production caller.
|
||||
- `bash_output`, `bash_kill`, and completion notices still use `ownerOf`, `readOutput`, `kill`, and `onTaskDone` exactly as before.
|
||||
- The Cordis catalog, core data-structure docs, package READMEs, and tests are updated.
|
||||
- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation.
|
||||
|
||||
## Risks
|
||||
|
||||
- Programmatic consumers lose an easy way to inspect all tasks. In the unreleased repo, the consumer audit says none exist outside tests.
|
||||
- Tests may become slightly less direct because they cannot assert the private map contents through `list()`. That is a useful pressure: public tests should prove observable behavior rather than pin the executor's storage shape.
|
||||
- A future task dashboard would need a new inventory surface. That should be designed with ownership and UI semantics, not inherited accidentally from an executor map.
|
||||
@@ -0,0 +1,42 @@
|
||||
# RFC: Remove defaults from the tool-schema DSL
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`SchemaProp.default?: unknown` exists in the first-party tool-schema DSL ([packages/core/tools/src/schema.ts](../../../../packages/core/tools/src/schema.ts)). The converter copies it into the JSON Schema sent to the model, but the runtime validator does not apply defaults: an omitted optional argument remains omitted, and a missing required argument still fails. The code already marks this with `XXX(unused-default)`.
|
||||
|
||||
No first-party tool definition in the repo sets `default`. Grepping `SchemaProp` defaults finds only the DSL itself, [docs/core-data-structures/tools.md](../../../core-data-structures/tools.md), and tests that assert the converter preserves a synthetic default. The behavior those tests pin is therefore model-visible metadata that no shipped tool emits and no runtime behavior honors.
|
||||
|
||||
This is exactly the kind of small speculative knob that makes a custom DSL harder to explain. The [custom schema DSL RFC](../../implemented/architecture/2026-06-11-custom-schema-dsl.md) accepted a deliberately small subset until real tools demanded more; `default` was included in that early subset, but the real tools have not demanded it.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove `default` from the first-party `SchemaProp` DSL.
|
||||
|
||||
- Delete `default?: unknown` from `SchemaProp`.
|
||||
- Delete the `prop.default` to JSON Schema conversion line.
|
||||
- Delete tests that assert synthetic defaults round-trip through `schemaSpecToJsonSchema`.
|
||||
- Update `validateArgs` docs so they no longer describe default non-application as part of the DSL semantics.
|
||||
- Update [docs/core-data-structures/tools.md](../../../core-data-structures/tools.md), the type-equivalence manifest output if needed, and any generated docs affected by the public type change.
|
||||
|
||||
This does not ban defaults from every possible tool schema. `ToolRegistry.register()` still accepts raw model-facing `ToolSchema` objects, so a future MCP or raw-JSON-Schema producer can pass through provider-specific JSON Schema fields if needed. The simplification is only for the first-party typed DSL that `defineTool()` owns.
|
||||
|
||||
## Why not apply defaults instead?
|
||||
|
||||
Applying defaults would be a behavior change at the model boundary: `defineTool()` would need to synthesize missing arguments before the typed `execute` body runs, decide whether defaults apply recursively, and document how defaulted values interact with required fields and `InferArgs`. That is a real feature, not a cleanup, and no current tool needs it.
|
||||
|
||||
Keeping metadata-only defaults is worse than doing nothing because it suggests the tool runtime has a defaulting story when it does not. Removing the field leaves one clear rule: optional arguments may be absent, required arguments must be present, and tools that want defaults put them in their own execution code.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `SchemaProp` no longer has a `default` field, and `schemaSpecToJsonSchema()` no longer emits defaults from first-party DSL specs.
|
||||
- `rg "unused-default|default\\?: unknown|prop\\.default|default:" packages/core/tools docs/core-data-structures/tools.md --glob '!docs/rfc/**'` finds no remaining DSL-default surface except unrelated JavaScript `default` syntax.
|
||||
- Tool schema conversion, validation, type inference, and `defineTool()` tests still cover requiredness, enums, nested objects, arrays, invalid args, and presentation metadata.
|
||||
- `pnpm run doc-sync`, including `doc-typecheck` and type-equivalence verification, passes after implementation.
|
||||
- `pnpm run test:coverage` and `pnpm run hygiene` pass after implementation.
|
||||
|
||||
## Risks
|
||||
|
||||
- A future tool may want to tell the model a default value. That tool can either default inside `execute` and describe the behavior in prose, or a later RFC can reintroduce DSL defaults with real runtime semantics and at least one first-party consumer.
|
||||
- Removing a type field breaks any external first-party DSL consumer. The repo is unreleased, so tightening the public type now is preferable to shipping a field whose semantics are "emitted but ignored."
|
||||
Reference in New Issue
Block a user