From 955e8d2913eb3bf25d032e6bc63c1a48ebb1a1f5 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 02:39:50 +0800 Subject: [PATCH 01/10] docs: propose simplification RFCs --- docs/rfc/README.md | 4 ++ ...drop-idle-registry-observation-surfaces.md | 50 +++++++++++++++++ ...-04-narrow-subagent-synchronous-collect.md | 55 +++++++++++++++++++ .../2026-07-04-prune-bash-task-roster.md | 40 ++++++++++++++ .../2026-07-04-remove-tool-schema-defaults.md | 42 ++++++++++++++ 5 files changed, 191 insertions(+) create mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 8470b29545..3b17edb027 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -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 diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md b/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md new file mode 100644 index 0000000000..74f05d78a9 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md @@ -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. diff --git a/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md b/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md new file mode 100644 index 0000000000..c0598edab4 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md @@ -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. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md b/docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md new file mode 100644 index 0000000000..9577f5930d --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md @@ -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. diff --git a/docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md b/docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md new file mode 100644 index 0000000000..4d2428a2aa --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md @@ -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." From e13bbcb5d55c185d1bfae02d07e4ae0a36674369 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 03:00:25 +0800 Subject: [PATCH 02/10] docs(rfc): propose nine simplification RFCs from a five-domain survey Survey of master for surface area whose consumers are tests/docs only, classified per candidate (production vs non-production corpus, rg + call-site reads). New proposed/simplification RFCs: - prune-producerless-vocabulary-variants: CacheHint/cache? fields, MessageSource 'agent', TurnTrigger 'continuation' (the TurnEndReasonMap omitted-until-emitted policy, applied) - drop-inert-request-knobs: GenerateOptions.prefill (both adapters throw UNSUPPORTED), ToolSchema.strict (zero setters; beta-URL-only feature) - drop-web-providers-change-event: the llm/adapter-change precedent replayed - drop-image-content-block: no producer; every consumer silently drops it - prune-write-only-fs-surface: fs-local STREAM_MIN_SIZE/streamMinSize, FsTarget.inputPath, FsEditOutcome.replacements/replaceAll, FileReadOutcome.limit/version - prune-unimplemented-subagent-vocabulary: outputSchema/structured, toolFilter, sendMessage/resume (depthLimit stays) - trim-acp-bridge-unreachable-surface: agentName/agentVersion knobs (resolves TODO(double-default)), toolKindFor name-sniffing - prune-dead-core-spine-surface: SurfaceManager.invalidate(), runLoop/Inbox exports, ToolExecutionResult.callId - share-app-bin-boot-glue: the twin coverage-exempt bin helpers Also supplements three existing proposed RFCs with survey evidence: the bash seam consumption census (generic-long-running-tool-runtime), three more static inventories (discover-package-inventory), and the bridge's already-1:1 id usage (unify-agent-and-session-id). --- docs/rfc/README.md | 9 +++++ ...06-20-generic-long-running-tool-runtime.md | 4 +++ .../2026-06-20-discover-package-inventory.md | 6 +++- .../2026-06-20-unify-agent-and-session-id.md | 2 +- .../2026-07-04-drop-image-content-block.md | 27 +++++++++++++++ .../2026-07-04-drop-inert-request-knobs.md | 33 +++++++++++++++++++ ...6-07-04-drop-web-providers-change-event.md | 28 ++++++++++++++++ ...026-07-04-prune-dead-core-spine-surface.md | 30 +++++++++++++++++ ...-prune-producerless-vocabulary-variants.md | 31 +++++++++++++++++ ...prune-unimplemented-subagent-vocabulary.md | 33 +++++++++++++++++++ .../2026-07-04-prune-write-only-fs-surface.md | 29 ++++++++++++++++ .../2026-07-04-share-app-bin-boot-glue.md | 27 +++++++++++++++ ...-04-trim-acp-bridge-unreachable-surface.md | 27 +++++++++++++++ 13 files changed, 284 insertions(+), 2 deletions(-) create mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-share-app-bin-boot-glue.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 8470b29545..c6e470004d 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -51,6 +51,15 @@ 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 | +| [Prune producer-less vocabulary variants (block cache hints, the `agent` message source, the `continuation` turn trigger)](proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md) | 2026-07-04 | +| [Drop `GenerateOptions.prefill` and `ToolSchema.strict` — request knobs with no working end-to-end path](proposed/simplification/2026-07-04-drop-inert-request-knobs.md) | 2026-07-04 | +| [Drop the unconsumed `web/providers-change` event](proposed/simplification/2026-07-04-drop-web-providers-change-event.md) | 2026-07-04 | +| [Drop the `image` content block until a path can honor it](proposed/simplification/2026-07-04-drop-image-content-block.md) | 2026-07-04 | +| [Prune write-only fields and a dead routing knob from the fs seam](proposed/simplification/2026-07-04-prune-write-only-fs-surface.md) | 2026-07-04 | +| [Prune the unimplemented subagent seam vocabulary](proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md) | 2026-07-04 | +| [Trim unreachable ACP bridge surface — the branding knobs and the kind-sniffing fallback](proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md) | 2026-07-04 | +| [Prune dead core-spine surface — `SurfaceManager.invalidate()`, the loop-internal exports, `ToolExecutionResult.callId`](proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md) | 2026-07-04 | +| [Share the app bins' boot glue instead of maintaining twin copies](proposed/simplification/2026-07-04-share-app-bin-boot-glue.md) | 2026-07-04 | ### Architecture diff --git a/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md index 4f034e3020..90103a19b1 100644 --- a/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md +++ b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md @@ -22,6 +22,10 @@ The runtime should own: `dsh-bash` then keeps the bash-specific execution contract: resolve a request into a command spec, run a foreground command, or start a process and hand its streams/process handle to the generic runtime. `dsh-tool-bash` keeps the model-facing command tool, but the follow-up operations become generic long-running-tool operations or a shared utility that bash registers with, rather than bespoke `bash_output`/`bash_kill` plumbing. +## Current seam consumption + +A consumer census of the surface the runtime would carve up. Production (`packages/bash/tool-bash/src/index.ts`) consumes `resolve`, `run`, `start`, `ownerOf`, `readOutput`, `kill`, and `onTaskDone`. `get()`/`list()` and the per-task `BashTask.done` promise have test-harness consumers only — `get()`/`list()` were removed once and reverted on the merits (the implementation note in [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records the test-migration cost dwarfing the surface removed), and `done` doubles as `dsh-bash-local`'s dispose-to-quiescence primitive. The seam therefore carries two public completion representations — the per-task promise and the global `onTaskDone` listener registry — of which production consumes one: the runtime should pick exactly one public completion surface and record which. One shape wart for the split to dissolve: `BashExecSpec.timeoutMs` is required but ignored by `start()`, an artifact of sharing one spec type between foreground and background execution. + ## Acceptance criteria - The bash-specific packages no longer define the generic task registry, owner-token authorization, polling, cancellation, or completion-notification machinery. diff --git a/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md index 784a36c593..abdd9e9a41 100644 --- a/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` lists all 18 packages as explicit project `references`. These lists are small today, but every new package or gate creates another manual synchronization point. +Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` and the root `tsconfig.json` each hand-list every package as explicit project `references`. `knip.json` restates a per-package `entry` stanza for each package that gains an `*.e2e.ts` suite — byte-identical overrides that exist only because the shared `packages/*/*` stanza omits the e2e glob (an entry glob matching no files is inert, so the default stanza could carry it for every package). The ACP snapshot suite's scenario table (`examples/acp-agent/tests/acp.snapshot.ts`) hand-maintains a `childSessions` count per scenario that duplicates the number of `session..jsonl` fixture siblings on disk. These lists are small today, but every new package or gate creates another manual synchronization point. The [package hierarchy](../../implemented/architecture/2026-06-20-package-hierarchy.md) already removed several of these by hand: `scripts/publint-all.ts` now derives its list from the `packages//` layout, and the two `tsconfig` `paths` maps collapsed to one `@deepseek-ai/dsh-*` wildcard. What remains is the inventory that cannot be globbed away — chiefly `tsconfig.build.json`'s project `references`, which TypeScript requires as an explicit array (no wildcard form). @@ -16,12 +16,16 @@ Make the remaining package/gate inventories discoverable. A single canonical sou The hierarchy does not need to encode every fact about a package, but it should encode the broad maintenance policy: core/product packages, integrations, capability seams, and support/test/example packages should not all require a hand-maintained exception list before scripts can tell them apart. +Two of the cataloged items need no generator at all: folding the e2e entry glob into knip's default stanza deletes the per-package restatements outright, and `childSessions` can be discovered from each scenario's fixture directory, leaving the scenario table to declare only policy (`recorded`, `hasModelTurn`). + ## Acceptance criteria - `tsconfig.build.json` project `references` are generated from the hierarchy (a generator emits them; a `--check` gate fails when the committed copy is stale), rather than hand-maintained. - Adding a package does not require editing a static package list for any gate. - Docs describe the source of truth rather than repeating generated inventories. - CI invokes the aggregate commands and lets those commands own their sub-gate lists. +- `knip.json` carries a per-package override only where it encodes real information (an extra entry file, an ignored dependency), never a restatement of the default stanza. +- Snapshot scenarios declare policy, not facts discoverable from their fixture directories. ## What we give up diff --git a/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md index cc0db091dc..b0c4498c17 100644 --- a/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md +++ b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md @@ -14,7 +14,7 @@ The agent factory carries TWO ids for what is, in every live consumer, one thing - **Config-driven create** (`AgentLoop.create`): a stable `agentId` (e.g. `"echo"`) with a fresh per-run `sessionId` (`${id}-session-`). - **Resume**: a caller-supplied `agentId` (e.g. `"main"`) on a persisted `resumeSessionId`. -Everywhere a live consumer actually looks an agent up — the **ACP bridge, the only production path** — the two are already unified: `agentId === sessionId === `. +Everywhere a live consumer actually looks an agent up — the **ACP bridge, the only production path** — the two are already unified: `agentId === sessionId === `. Concretely, both bridge factory call sites brand `AgentId(sessionId)` directly, and the bridge's reverse lookup keys on the `Agent` object itself — there is no id translation anywhere in the bridge to migrate. The separation is **latent generality no consumer exercises**: nothing reads a *stable* `agentId` back across runs (each process starts fresh, and persistence keys off the session id, never the agent id). The config path's "stable agentId, fresh sessionId" buys nothing concrete — it is cosmetic. And the `agentId !== sessionId` case is precisely what opens the bash owner-token alias hole: the bash completion-notice routes by `session.header.id`, but the registry enforces uniqueness only on `agentId`, so a programmatic caller registering two agents with different agent ids but the SAME session id can mis-route a notice (see [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md) § Seam precondition). The current code documents this as a precondition rather than guaranteeing it. diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md new file mode 100644 index 0000000000..b0153d5d22 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md @@ -0,0 +1,27 @@ +# RFC: Drop the `image` content block until a path can honor it + +Status: proposed + +## Problem + +`ImageBlock` (`packages/llm/llm/src/types.ts`) has no production producer, and every consumer on every path DROPS it: the deepseek adapter's serializer skips image blocks (a documented MVP limitation), the pi-ai converter skips them as unrepresentable, the ACP codec neither advertises image prompt capability nor forwards image blocks outbound and REJECTS image prompt content inbound, and the compaction estimator charges a flat token constant and renders `[image]`. An `ImageBlock` constructed today would silently vanish from the wire — the vocabulary advertises a capability no path honors, which is the silent-data-loss shape AGENTS.md's defensive patterns warn against. The only constructors anywhere are tests pinning the skip/drop/estimate branches. + +## Proposal + +Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. + +## Why not keep it? + +This is the most contested cut in the batch. Multimodal input (screenshots) is a plausible near-term coding-agent feature, and the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md) reserved the slot deliberately. Two responses. First, `ContentBlockMap` is merge-extensible by design: a real multimodal feature reintroduces `image` in core in the same coordinated change that maps it in the adapters, advertises and renders it in ACP, and prices it in compaction — the producer and its consumers arrive together, which is how the map is meant to grow. Second, the middle option — keep the type but make adapters throw UNSUPPORTED instead of silently dropping — converts this into exactly the shape the [request-knobs RFC](2026-07-04-drop-inert-request-knobs.md) argues against: surface whose only implementation is rejection. Absence (a compile error at the would-be producer) is strictly clearer than either silent loss or universal throw. + +If review lands on keeping the slot, the fallback this RFC records is: keep `ImageBlock` but replace every silent skip with a loud rejection, and document that policy in the vocabulary — the current silent drop is the one state with no defender. + +## Acceptance criteria + +- No `ImageBlock` / harness `type: 'image'` block construction outside this RFC; the codec's inbound ACP-image rejection still passes its tests. +- Adapter/codec/compaction switches handle the case through their unknown-block default arms (pinned by the existing plugin-added-block tests where present). +- Doc pastes, the manifest, and the architecture vocabulary list updated; `pnpm run doc-sync` green. + +## Risks + +Re-adding a core vocabulary type later touches several packages at once — but that coordinated change is the shape a real multimodal feature needs anyway (adapter mapping, ACP advertisement, compaction pricing), and none of it exists today to preserve. diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md b/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md new file mode 100644 index 0000000000..3c44a2126f --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md @@ -0,0 +1,33 @@ +# RFC: Drop `GenerateOptions.prefill` and `ToolSchema.strict` — request knobs with no working end-to-end path + +Status: proposed + +## Problem + +Two request-contract knobs ride the whole request pipeline, yet neither can do anything today: + +- **`prefill`** (`packages/llm/llm/src/types.ts`) has no production setter — the loop assembles `model`/`system`/`tools`/`messages` plus `sessionId`/`signal`, and the compaction backend adds only `maxTokens` — and BOTH adapters reject it: `packages/llm/llm-deepseek/src/serialize.ts` and `packages/llm/llm-pi-ai/src/adapter.ts` each throw `LlmError('UNSUPPORTED')` on a non-undefined `prefill`. The field's entire observable behavior is two throws, each pinned by one adapter test. DeepSeek's chat-prefix completion is a Beta feature on a base URL neither adapter targets. +- **`strict`** (`ToolSchema`, same file) is threaded through `DefineToolOptions`/`defineTool` (`packages/core/tools/src/schema.ts`), the registry's `schemas()` allowlist (`packages/core/tools/src/index.ts`), the deepseek wire mapping (`packages/llm/llm-deepseek/src/serialize.ts`, whose wire-type note records that strict mode requires the `/beta` base URL the adapter does not use), and a per-tool payload-patching pass in `packages/llm/llm-pi-ai/src/adapter.ts`. No shipped tool sets it — `rg` across every `tool-*` package src and `examples/` finds zero `strict:` producers; the only setters are dsh-tools unit tests. + +Both knobs are adapter-symmetric, so removal sheds them from both twins together — the [twin-adapter design](../../implemented/architecture/2026-06-13-twin-llm-adapters.md) is untouched. + +## Proposal + +- Remove `prefill` from `GenerateOptions`, both adapters' UNSUPPORTED guards, the tests pinning the throws, the paste lines in [core.md](../../../core-data-structures/core.md), and the adapter README rows documenting the rejection. +- Remove `strict` from `ToolSchema`, `DefineToolOptions`, `defineTool`, and the `schemas()` allowlist; drop the deepseek serializer branch; simplify the pi-ai payload fixup to the unconditional scrub of pi-ai's own strict default (that half exists for wire parity with the hand-rolled twin and survives); drop the setter tests and the core.md paste line. + +This RFC deliberately does NOT touch `temperature`, `stop`, or `maxTokens`: those are honored end-to-end by both adapters and are the natural first targets of a request-mutating hook plugin on `agent/request`. + +## Why not keep them? + +"An explicit UNSUPPORTED throw is honest contract behavior" — but a knob whose only implementation across both twins is rejection promises nothing, and deleting it upgrades the failure mode: an accidental setter becomes a compile error instead of a runtime throw. "Strict schema adherence is an officially documented provider feature with complete plumbing" — but a knob is not product surface until a shipped tool sets it AND an endpoint honors it; today neither is true. Each returns with its first real producer: `prefill` together with an adapter that implements chat-prefix completion (and a stated policy for adapters that do not), `strict` together with a tool that wants it and a beta-endpoint story. + +## Acceptance criteria + +- `rg prefill` and a tool-schema-scoped `rg strict` return only this RFC (and unrelated prose such as `strictEqual`). +- Both adapters compile and their contract tests pass without the guards; the pi-ai fixup still scrubs the library's strict default (wire parity pinned by its serializer tests). +- Doc pastes and the type-equiv manifest in sync; `pnpm run doc-sync` green. + +## Risks + +A hooks/config plugin arriving via the interception seams may want to set request fields — it will reach for `temperature`/`stop` (kept, working), not a field adapters reject. If chat-prefix completion or strict mode become product features, the re-add lands with the adapter/endpoint work, where the contract can say what actually happens rather than "everyone throws". diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md b/docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md new file mode 100644 index 0000000000..ed40e54afe --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md @@ -0,0 +1,28 @@ +# RFC: Drop the unconsumed `web/providers-change` event + +Status: proposed + +## Problem + +`WebService` declares and emits `web/providers-change` (`packages/web/web/src/index.ts`) on every provider registration and disposal, and orders each registration effect's rollback yield BEFORE the emit solely so a throwing change listener unwinds the registration. No listener exists outside the package's own two unit tests (one of which exists to pin that rollback ordering). The remaining references are the generated catalog and README/doc prose. + +The seam's own design removed the natural consumer. `dsh-tool-web` registers tools by product ENABLEMENT, deliberately not by provider availability (`packages/web/tool-web/src/index.ts`), and `searchStatus()`/`fetchStatus()` are derived per call, never cached — so there is no cache to invalidate and no registration set to recompute when providers come and go. HMR cleanup is already carried by the effect disposers themselves. + +This is shape-for-shape the surface the repo already cut once: [drop the unconsumed `llm/adapter-change` event](../../implemented/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) removed the same notification, the same rollback-before-emit machinery, and the same listener-throw test from `LlmService`. That RFC's keep/cut criterion — keep `tools/change` for its plausible user-facing tool-list consumer, cut the boot-time backend-registry signal — puts a web-provider registry squarely on the cut side. + +## Proposal + +Delete the event declaration, both emits, and the rollback-before-emit ordering (the plain `ctx.effect` disposer keeps HMR cleanup); delete the two event tests; run `pnpm run gen-cordis-catalog` and commit the regenerated catalog; update `packages/web/web/README.md` and the [web.md](../../../core-data-structures/web.md) prose. The implementing PR amends the [web capability seam RFC](../../implemented/architecture/2026-06-24-web-capability-seam.md)'s facts (it specifies the event in its interface sketch and test list), per [implemented/AGENTS.md](../../implemented/AGENTS.md). + +## Why not keep it? + +The web seam RFC specified the event deliberately — days after the adapter-change removal — as a minimal HMR-visibility signal. But the same RFC also made every status read derived-on-call and tool registration availability-independent, which is precisely why no consumer can need the signal: the design's other choices starved this one. Per AGENTS.md "RFCs are proposals, not golden truth", the event is the part of that proposal the code has since shown to over-reach; validating it against the repo's own precedent yields the verdict the precedent already recorded. + +## Acceptance criteria + +- No `providers-change` spelling outside this RFC and the amended seam RFC; the catalog is regenerated and fresh (`verify-cordis-catalog` green). +- Registration/disposal HMR-safety tests still prove cleanup via `searchStatus()`/`fetchStatus()` derivation rather than via the event. + +## Risks + +A future provider-picker UI or diagnostics panel that wants live change notifications re-adds the event with that consumer — the identical judgment, and its reversal condition, is already recorded on the llm precedent. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md new file mode 100644 index 0000000000..df4551ac20 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md @@ -0,0 +1,30 @@ +# RFC: Prune dead core-spine surface — `SurfaceManager.invalidate()`, the loop-internal exports, `ToolExecutionResult.callId` + +Status: proposed + +## Problem + +Three pieces of public spine surface share one defect class: their only possible role is to be ignored, or their trigger is unreachable. + +1. **`SurfaceManager.invalidate()`** (`packages/core/session/src/surface.ts`). Its documented trigger — "the log has been replaced wholesale (e.g. after Session seed)" — is structurally unreachable: seeding happens inside the `Session` constructor, `_surface` is created lazily on first access, and the log reference is never reassigned afterward, so no constructed `SurfaceManager` ever observes a wholesale replacement. Sole caller: its own unit test. A rollback primitive protecting a scenario the implementation cannot produce. +2. **The `runLoop`, `Inbox`, and `InboxMessage` exports** (`packages/core/agent-loop/src/index.ts`). `runLoop` has zero importers anywhere; `Inbox`/`InboxMessage` are imported only by the package's own inbox spec (switchable to the source module). The exports contradict the package's own docs — the inbox module doc says the public surface is `Agent.send()`/`Agent.steer()` — and the [architecture dependency rule](../../../architecture.md): nothing programs against `dsh-agent-loop`; a replacement loop is a different bundle built on `dsh-agent`, not a consumer of this package's internals. `ReactLoopAgent` stays exported (cross-package tests construct it by package name). +3. **`ToolExecutionResult.callId`** (`packages/core/tools/src/index.ts`; the *input* `ToolExecution.callId` stays). Zero readers. The loop deliberately ignores it and documents it as a footgun — the correlation id must be the loop's own `call.id`, because a `tools/execute` waterfall listener returning a mismatched id would otherwise orphan the call↔result pairing — and a regression test exists solely to prove the field is ignored. So every waterfall short-circuiter must fabricate a field whose only power is to be a bug if trusted; the ACP bridge correlates via the session event's `data.callId`, never via the execution result. + +## Proposal + +Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites, and `toolErrorResult`, along with the loop's ignore-comment and the proves-ignored regression test — the hazard they guard disappears with the field. + +Sequencing: the in-flight surface-cache work (tool-pairing balance caching) neither uses nor touches `invalidate`, so that removal lands after or alongside it mechanically. The `callId` removal waits for the in-flight interception-seams work that splits `tools/execute` into pre/post phases and currently carries the field verbatim — the argument transfers unchanged (post-execute listeners receive the execution object alongside the result), so the removal targets whichever seam shape is on master when implemented. + +## Why not keep them? + +A future consumer that swaps a session's log in place would want a reset primitive — it re-adds `invalidate` with itself. A replacement-loop author might want to reuse the inbox or the driver — the architecture already answers that a replacement loop is a different bundle. An isolated result-logging listener might want self-contained correlation on the result — the execution object is in scope at every listener, and a field that exists only to be ignored is worse than absent: it invites exactly the orphaned-pairing bug the loop comment warns about. + +## Acceptance criteria + +- The three surfaces appear only in this RFC; the agent-loop README lists only the consumed public surface; the inbox spec imports the source module. +- The tools/execute contract tests pass with the shrunk result type; no waterfall test fabricates a `callId` on a result. + +## Risks + +All three are compile-visible removals with no runtime behavior change on any shipped path. The `callId` change lands on whatever execute-seam shape is current, as noted under sequencing. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md new file mode 100644 index 0000000000..e663c4c90f --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md @@ -0,0 +1,31 @@ +# RFC: Prune producer-less vocabulary variants (block cache hints, the `agent` message source, the `continuation` turn trigger) + +Status: proposed + +## Problem + +The merge-extensible vocabulary maps are designed to grow by declaration merging, and the codebase already states the admission policy on `TurnEndReasonMap` (`packages/core/session/src/types.ts`): a variant like `refusal` is "deliberately omitted until" an adapter or loop first emits it. Three declared vocabulary items violate that policy — each has no producer and no consumer, and two have not even a test: + +- **`CacheHint` and the three `cache?: CacheHint` fields** on `TextBlock`/`ToolResultBlock`/`ImageBlock` (`packages/llm/llm/src/types.ts`). Nothing constructs a block with `cache:` anywhere — src, tests, and doc pastes all come up empty — and neither adapter reads `.cache`: DeepSeek prompt caching is automatic, so the adapters map `prompt_cache_hit_tokens` OUT of responses without ever sending a hint IN. This is Anthropic-style `cache_control` surface with no provider that can honor it. +- **`MessageSourceMap.agent`** (`{ kind: 'agent'; agentId: string }`, same file). Zero constructors, tests included. Its intended producer shipped without it: the subagent backends send the parent's prompt to the child with no `source`, so it logs as `{ kind: 'user' }`, and the generic envelope renderer interpolates `source.kind` without ever routing on it. The variant is pasted into [core.md](../../../core-data-structures/core.md). +- **`TurnTriggerMap.continuation`** (`packages/core/session/src/types.ts`). The loop structurally cannot emit it — continuation happens *within* a turn as further steps, never as a new turn — and it constructs only `message` and `injection` triggers. The only writers are two hand-built test fixtures that need an arbitrary non-message trigger (`packages/support/llm-replay/tests/llm-replay.spec.ts`, `packages/support/ui-stdio/tests/ui-stdio.spec.ts`); the only production trigger reader, the ACP bridge, filters on `kind === 'message'`. The variant is pasted into [session.md](../../../core-data-structures/session.md). + +## Proposal + +Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the two test fixtures to `injection` triggers (any non-`message` trigger serves their purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change. + +Each variant returns the day it gains a real producer, exactly as the maps are designed to grow: a caching feature re-adds `cache` together with the adapter that transmits it; subagent attribution re-adds `agent` together with the backend that stamps it and a consumer that routes on it; an auto-continue feature that genuinely starts new turns re-adds `continuation` with the plugin that emits it. + +## Why not keep them? + +The [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md) lists "cache hints … have a home" as a design consequence, and reserved slots do advertise intent. But an empty slot is contract surface every implementation and consumer must consider (must my adapter honor `cache`? must my renderer route `agent` sources?), and the sibling map's own JSDoc already rejects reservation-without-emitter — `refusal` and `max_turn_requests` are named as variants to add *when something first emits them*, not declared in advance. Holding already-declared dead variants to the same standard makes the vocabulary mean something: if it is in the map, something produces it. + +## Acceptance criteria + +- `rg` for `CacheHint`, the `agent` message-source spelling, and the `continuation` trigger spelling returns only this RFC. +- The core-data-structures pastes and the type-equiv manifest are in sync (`pnpm run doc-sync` green). +- The two fixtures assert the same replay behavior with `injection` triggers; the suite is green. + +## Risks + +None operational — nothing can construct these values today. The in-flight event-taxonomy work reworks the transient `agent/*` mirror events, not the durable vocabulary declarations, so there is no collision. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md new file mode 100644 index 0000000000..f5c8c424d3 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md @@ -0,0 +1,33 @@ +# RFC: Prune the unimplemented subagent seam vocabulary + +Status: proposed + +## Problem + +The [subagent seam](../../implemented/feature/2026-06-21-subagent-capability-seam.md) shipped a two-tier capability design: start-time capability flags checked by the service, and optional runtime methods on `SubagentRun`. Three start-time features and both optional runtime methods have zero implementations and zero callers: + +- **`outputSchema`/`structured` and `toolFilter`** (`SubagentCapabilities`, `SubagentStartRequest`, `SubagentResult` in `packages/subagent/subagent/src/types.ts`): every real provider declares `outputSchema: false, toolFilter: false` (`packages/subagent/subagent-spawn/src/index.ts`, `packages/subagent/subagent-fork/src/index.ts`, `packages/subagent/subagent-acp/src/index.ts`); the sole production `ctx.subagents.start` caller (`packages/subagent/tool-subagent/src/index.ts`) builds `{ prompt, parent, signal?, agentOptions? }` and structurally cannot set either; `structured` is produced only by the test mock (`packages/support/subagent-mock`) for its own spec. The service's capability check carries two assert rows whose only exercisers are the rejection tests. +- **`SubagentRun.sendMessage` / `SubagentRun.resume`** (same file): implemented by NO provider — not even the mock; the spawn spec asserts their *absence*. + +The only reason `dsh-subagent` depends on `dsh-tools` at all is `outputSchema`'s `SchemaSpec` type. Three subsequent subagent workstreams (per-session snapshot replay, the fork seed boundary, the ACP backend) landed around this surface without growing a single consumer. + +## Proposal + +Remove `outputSchema`/`structured`, `toolFilter`, `sendMessage`, and `resume` from the seam; shrink `SubagentCapabilities` to `{ depthLimit }`; drop the two capability-assert rows, the all-false flags on the three providers, the mock's structured branch and its `capabilities`/`structured` config knobs, and the tests that exist to pin the removed surface (the two rejection rows, the spawn absence test, the mock structured specs). Drop the `dsh-tools` peer/dev dependency from `packages/subagent/subagent/package.json`. Update the [subagent.md](../../../core-data-structures/subagent.md) pastes and the type-equiv manifest, and the README rows in `packages/subagent/subagent`, `packages/subagent/subagent-spawn`, `packages/subagent/subagent-fork`, and `packages/support/subagent-mock`. The implementing PR amends the seam RFC's capability catalog per [implemented/AGENTS.md](../../implemented/AGENTS.md). + +**Keep** `depthLimit`/`maxDepth` and the capability-check mechanism itself: the in-process backend genuinely enforces the cap (`SubagentDepthError` in `packages/subagent/subagent-inprocess/src/index.ts`), recursion is the seam RFC's named risk, and one live capability row keeps the two-tier design demonstrated rather than merely remembered. + +This is the seam-vocabulary echo of [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md): members every implementation must declare for nobody — weaker even, since here zero implementations exist. + +## Why not keep it? + +The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/execute` veto, not schema filtering) — re-adding against a real implementing provider will pin a better contract than the current speculative one. + +## Acceptance criteria + +- The removed spellings appear only in this RFC and the amended seam RFCs; `SubagentCapabilities` is `{ depthLimit: boolean }`; the `dsh-tools` dependency edge is gone (`hygiene` green). +- Depth-enforcement tests are unchanged and green. + +## Risks + +The in-flight hooks stack enriches subagent lifecycle event payloads (agent type, last assistant message) — adjacent files, no field overlap; coordinate landing order mechanically. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md new file mode 100644 index 0000000000..91fa0ab739 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md @@ -0,0 +1,29 @@ +# RFC: Prune write-only fields and a dead routing knob from the fs seam + +Status: proposed + +## Problem + +The [fs seam split](../../implemented/simplification/2026-06-26-fsspec-style-fs-seam.md) moved read routing and policy out of the backend into `dsh-tool-fs` and `dsh-fs-policy`. Four pieces of surface kept the pre-split shape — populated on every call, read by nobody: + +1. **`STREAM_MIN_SIZE` + `FsIoInternals.streamMinSize` in `dsh-fs-local`** (`packages/fs/fs-local/src/fsio.ts`, re-exported from `packages/fs/fs-local/src/index.ts`): zero readers anywhere, including fs-local's own source and tests. The backend has no read routing — `readWholeText`/`streamWholeText` are separate primitives the caller chooses between — and the real routing constant lives in the consumer (`packages/fs/tool-fs/src/read.ts`, compared against `info.size`). Two mirrors of the 10 MiB fact; the backend's is dead, and the knob's JSDoc claims a "read routing" override that does not exist. +2. **`FsTarget.inputPath`** (`packages/fs/fs/src/types.ts`): every backend and every test fake must fabricate a "diagnostics only" value with zero production readers — the policy plugin and every error message use `targetKey`/`displayPath`. The `listDir` producer exposes the semantic wobble: directory children get the bare entry name, which was nobody's "input". +3. **`FsEditOutcome.replacements` + `.replaceAll`** (`packages/fs/fs/src/types.ts`): `replacements` has zero production readers (the single-match policy itself stays — it is enforced by the `FS_AMBIGUOUS_EDIT`/`FS_EDIT_NOT_FOUND` throws inside the backend, whose error message keeps the internal count); `replaceAll` is read only by `formatEditOutput` in `packages/fs/tool-fs/src/edit.ts` — as an echo of the `replace_all` argument the tool already holds. Shrunk, `FsEditOutcome` becomes `{ version, before, after }`, parallel to `FsWriteOutcome`'s genuinely backend-discovered fields. +4. **`FileReadOutcome.limit` + `.version`** (`packages/fs/tool-fs/src/read-render.ts`): populated by the read tool, but `formatReadOutput` renders `offset`/`lines`/`totalLines`/`truncatedByBytes` only, and the `fs/observed` emit uses `info.version` directly rather than the outcome copy. + +## Proposal + +Delete the fs-local constant, its re-export, and the `streamMinSize` knob (the remaining `FsIoInternals` knobs are genuinely used by the atomic-write tests); drop `inputPath` from `FsTarget`; shrink `FsEditOutcome` to `{ version, before, after }` and pass `replaceAll` to `formatEditOutput` from the parsed args; drop `limit`/`version` from `FileReadOutcome`. Update the [filesystem.md](../../../core-data-structures/filesystem.md) pastes, the type-equiv manifest, `packages/fs/fs/README.md`, and the test fakes that currently must fabricate the removed fields. + +## Why not keep them? + +A future permission/containment layer might want the pre-resolution path for error text — but it would want the *request*, which every call site still holds. "N occurrences replaced" might become model-facing text — a behavior change to design when wanted, and the backend-internal count survives for its error message. A read footer might display `limit` — everything the footer shows already derives from `lines`/`totalLines`. Meanwhile every current and future backend (remote, native) must fabricate wire fields nobody consumes, and every test fake must satisfy them. + +## Acceptance criteria + +- The removed spellings appear only in this RFC; doc pastes and the manifest in sync; the suite is green with the shrunk fakes. +- `formatEditOutput`'s emitted text is unchanged for both `replace_all` branches, so no snapshot golden churns. + +## Risks + +The in-flight fs discovery work (glob/grep tools) touches the same `dsh-fs` type files — a textual, not design, conflict; land in either order and reconcile mechanically. Backends gain no new obligations; they shed four. diff --git a/docs/rfc/proposed/simplification/2026-07-04-share-app-bin-boot-glue.md b/docs/rfc/proposed/simplification/2026-07-04-share-app-bin-boot-glue.md new file mode 100644 index 0000000000..adc3ec6da3 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-share-app-bin-boot-glue.md @@ -0,0 +1,27 @@ +# RFC: Share the app bins' boot glue instead of maintaining twin copies + +Status: proposed + +## Problem + +`packages/ui/stdio-agent/src/bin.ts` and `packages/ui/acp-agent/src/bin.ts` carry four near-twin helpers — `loadEnv`, `installFailLoud`, `assertEntriesLoaded`, `boot` — whose bodies differ essentially in the diagnostic prefix, plus two copies of the hardest-won boot lore in the repo: the `Promise.allSettled` swallow inside `loader.await()`, the silent-exit-0 import-failure guard, and the `--expose-internals` resolution note (the failure classes behind AGENTS.md's "real entry path means the published artifact" pattern). Drift has already begun: `boot(configPath)` resolves the path internally in one bin but requires a pre-resolved absolute path in the other, and the twin JSDoc prose has forked. + +The duplication is aggravated by a coverage hole: all of this logic sits OUTSIDE the per-file 100% gate — `vitest.config.ts` excludes `packages/*/*/src/bin.ts` because importing a self-executing bin (top-level `await main()`) runs it — which also makes the `export` keywords on these helpers decorative: no spec can import them, so the only exercisers are the subprocess smokes, and the two `built-bin.e2e.ts` suites duplicate their temp-node_modules scaffolding as well. The genuinely per-app pieces are small and real: the ACP bin owns snapshot-mode config selection (`resolveConfigPath`), replay-mode env skipping, the stdin-EOF dispose lifecycle, and stdout purity; the stdio bin owns nothing extra. + +## Proposal + +Extract the four helpers, parameterized by the bin's diagnostic prefix, into an importable non-bin module shared by both apps — a small published package in the `ui` group (the bins are published artifacts, so their runtime dependency must be published too, not `support/`). Each `bin.ts` becomes a thin self-executing `main()` plus its app-specific glue. The shared module gains unit tests and falls under the coverage gate; the loader-failure lore gets one home; the subprocess smokes remain the artifact-level guard — the published-bin smoke is NOT replaced by unit tests, per the "real entry path" defensive pattern. The implementing PR amends the [extract example app packages RFC](../../implemented/architecture/2026-06-20-extract-example-app-packages.md)'s facts ("boot glue moved into that bin, owned by the app" is the sentence that changes). + +## Why not keep the duplication? + +The bins were framed as independently-owned published artifacts, and a new package carries fixed overhead (manifest, README, tsconfig reference, publint surface) that rivals the deduplicated line count. But app-vs-app sharing was never weighed by that RFC — it consolidated three example `start.ts` copies INTO the bins and stopped there; the drift is now observed fact rather than speculation; and the coverage-gap argument is independent of the dedup argument: this is the only nontrivial runtime logic in the repo exempt from the per-file 100% gate. The alternative of a copy-by-convention shared source file is the current state with extra steps. + +## Acceptance criteria + +- The four helpers exist once, unit-tested, under the coverage gate; both bins are thin mains plus app-specific glue. +- Both built-bin smokes still pass under plain node in the node_modules-shaped temp dir, including the missing-config non-zero exit. +- The app-packages RFC's facts are amended in the same change. + +## Risks + +Churn in two published bins and one new package boundary; the shared module must stay dependency-light (cordis plus the loader). If the implementing PR finds the package overhead genuinely exceeds the dedup — the honest failure mode of this proposal — the fallback that still pays is extracting only the coverage-exempt pure logic (`assertEntriesLoaded`, `resolveConfigPath`) into an importable module within each app package, ending the coverage exemption without a new package. diff --git a/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md b/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md new file mode 100644 index 0000000000..3e5fccb60a --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md @@ -0,0 +1,27 @@ +# RFC: Trim unreachable ACP bridge surface — the branding knobs and the kind-sniffing fallback + +Status: proposed + +## Problem + +Two pieces of `dsh-acp` surface are unreachable from any shipped configuration: + +1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model, systemPrompt }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — can set the knobs at all; they are settable solely by direct-mounting the bridge, which only a unit test does. Every snapshot golden pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carries a live `TODO(double-default)`: the literals exist twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home. +2. **The `toolKindFor` name heuristic** (same file) special-cases `bash*`/`read*`/`write`/`edit*` tool names in the generic-fallback path. Since the [render-intent union](../../implemented/architecture/2026-07-02-tool-render-intent-union.md), every first-party tool those arms match ships its own `presentCall` carrying its kind, and the presenter-less production tools (`subagent`, `subagent_fork`) fall through to `other` anyway. The arms are production-reachable only when a tool's `presentCall` THROWS (the containment fallback) — and the bridge's own module doc states the design rule the heuristic violates: "the bridge never special-cases tool names". + +## Proposal + +Hardcode `agentInfo` at the `initialize` site (`{ name: 'deepseek-harness-acp', version: '0.0.1' }`), deleting the two config fields, their schema defaults, the `??` fallbacks, and the `TODO(double-default)` whose subject vanishes; drop the knob half of the direct-mount config test, the two rows in `packages/ui/acp/README.md`, and the `packages/ui/acp/acp-feature-support.md` cell that cites the knobs. Zero golden churn — the emitted wire value is unchanged. Replace `toolKindFor` with the constant `'other'` in both fallback sites (the presenter fallback and `nullToolPresenter`) and delete the heuristic with its test rows. + +## Why not keep them? + +`agentInfo` is client-visible branding a deployment will eventually want configurable — but a knob no shipped config can reach is not configurability, it is drift surface (the double-default TODO is its symptom), and the honest re-add must include the `dsh-acp-agent` plumb-through that does not exist today either; both arrive together with the deployment that needs them. For the heuristic: a hypothetical third-party presenter-less tool named `read_docs` would lose its inferred `read` icon — but inferring kinds from unknown plugins' names is exactly the special-casing the render-intent design rejected. The behavior delta on shipped paths is confined to the presenter-throw fallback, where rendering kind `other` makes the client show the raw input instead of a masquerading first-party card — strictly better diagnostics for a broken presenter. + +## Acceptance criteria + +- `agentName`/`agentVersion` and `toolKindFor` appear only in this RFC; snapshot goldens are byte-identical; bridge tests are green with the constant fallback. +- The `initialize` handshake continues to report `deepseek-harness-acp`/`0.0.1` (pinned by the handshake snapshot). + +## Risks + +None beyond the presenter-throw rendering delta described above — an error path whose new behavior is more diagnosable than the old. From 9389c5df19360cb52af0b1198e46d596a38f12c2 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 04:21:38 +0800 Subject: [PATCH 03/10] docs(rfc): address Codex review findings on the RFC batch Two overstated evidence claims, reworded to the precise fact: - prune-dead-core-spine-surface: runLoop has no importer OUTSIDE the package (its own agent.ts drives the loop with it); the dead surface is the public re-export, not the function. - generic-long-running-tool-runtime census: BashTask.done has no consumer through the public seam, but is production-load-bearing inside dsh-bash-local (disposal awaits it); only the public completion surface is single-consumer. Also fold the reviewer-located doc sites into the removal sets so the implementing PRs need no re-discovery: the llm/pi-ai/compact-basic README rows and the adding-an-llm-adapter cookbook line (prefill/image), the content-block-vocabulary RFC's has-a-home consequence lines (cache/prefill/image), the tools.md paste + type-equiv manifest row + tools README row (callId), and the session-surface RFC's full-rebuild-after-replacement sentence (invalidate). --- .../2026-06-20-generic-long-running-tool-runtime.md | 2 +- .../simplification/2026-07-04-drop-image-content-block.md | 2 +- .../simplification/2026-07-04-drop-inert-request-knobs.md | 2 +- .../2026-07-04-prune-dead-core-spine-surface.md | 4 ++-- .../2026-07-04-prune-producerless-vocabulary-variants.md | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md index 90103a19b1..87336a4868 100644 --- a/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md +++ b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md @@ -24,7 +24,7 @@ The runtime should own: ## Current seam consumption -A consumer census of the surface the runtime would carve up. Production (`packages/bash/tool-bash/src/index.ts`) consumes `resolve`, `run`, `start`, `ownerOf`, `readOutput`, `kill`, and `onTaskDone`. `get()`/`list()` and the per-task `BashTask.done` promise have test-harness consumers only — `get()`/`list()` were removed once and reverted on the merits (the implementation note in [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records the test-migration cost dwarfing the surface removed), and `done` doubles as `dsh-bash-local`'s dispose-to-quiescence primitive. The seam therefore carries two public completion representations — the per-task promise and the global `onTaskDone` listener registry — of which production consumes one: the runtime should pick exactly one public completion surface and record which. One shape wart for the split to dissolve: `BashExecSpec.timeoutMs` is required but ignored by `start()`, an artifact of sharing one spec type between foreground and background execution. +A consumer census of the surface the runtime would carve up. Production (`packages/bash/tool-bash/src/index.ts`) consumes `resolve`, `run`, `start`, `ownerOf`, `readOutput`, `kill`, and `onTaskDone`. `get()`/`list()` have test-harness consumers only — they were removed once and reverted on the merits (the implementation note in [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records the test-migration cost dwarfing the surface removed). The per-task `BashTask.done` promise has no consumer through the public seam either (`dsh-tool-bash` completes via `onTaskDone`), but it is production-load-bearing INSIDE the implementation: `dsh-bash-local`'s disposal awaits it to reach quiescence. The seam therefore exposes two public completion representations — the per-task promise and the global `onTaskDone` listener registry — and the shipped consumer uses only the latter: the runtime should pick exactly one public completion surface and record which. One shape wart for the split to dissolve: `BashExecSpec.timeoutMs` is required but ignored by `start()`, an artifact of sharing one spec type between foreground and background execution. ## Acceptance criteria diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md index b0153d5d22..c949286b57 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md @@ -8,7 +8,7 @@ Status: proposed ## Proposal -Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. +Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation row, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. ## Why not keep it? diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md b/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md index 3c44a2126f..5553645337 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md @@ -13,7 +13,7 @@ Both knobs are adapter-symmetric, so removal sheds them from both twins together ## Proposal -- Remove `prefill` from `GenerateOptions`, both adapters' UNSUPPORTED guards, the tests pinning the throws, the paste lines in [core.md](../../../core-data-structures/core.md), and the adapter README rows documenting the rejection. +- Remove `prefill` from `GenerateOptions`, both adapters' UNSUPPORTED guards, the tests pinning the throws, the paste lines in [core.md](../../../core-data-structures/core.md), the adapter README rows documenting the rejection, and the cookbook line using prefill as the UNSUPPORTED example ([adding-an-llm-adapter.md](../../../cookbook/adding-an-llm-adapter.md)); amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s consequence line naming prefill as having a home, per [implemented/AGENTS.md](../../implemented/AGENTS.md). - Remove `strict` from `ToolSchema`, `DefineToolOptions`, `defineTool`, and the `schemas()` allowlist; drop the deepseek serializer branch; simplify the pi-ai payload fixup to the unconditional scrub of pi-ai's own strict default (that half exists for wire parity with the hand-rolled twin and survives); drop the setter tests and the core.md paste line. This RFC deliberately does NOT touch `temperature`, `stop`, or `maxTokens`: those are honored end-to-end by both adapters and are the natural first targets of a request-mutating hook plugin on `agent/request`. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md index df4551ac20..bb9d0a8447 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md @@ -7,12 +7,12 @@ Status: proposed Three pieces of public spine surface share one defect class: their only possible role is to be ignored, or their trigger is unreachable. 1. **`SurfaceManager.invalidate()`** (`packages/core/session/src/surface.ts`). Its documented trigger — "the log has been replaced wholesale (e.g. after Session seed)" — is structurally unreachable: seeding happens inside the `Session` constructor, `_surface` is created lazily on first access, and the log reference is never reassigned afterward, so no constructed `SurfaceManager` ever observes a wholesale replacement. Sole caller: its own unit test. A rollback primitive protecting a scenario the implementation cannot produce. -2. **The `runLoop`, `Inbox`, and `InboxMessage` exports** (`packages/core/agent-loop/src/index.ts`). `runLoop` has zero importers anywhere; `Inbox`/`InboxMessage` are imported only by the package's own inbox spec (switchable to the source module). The exports contradict the package's own docs — the inbox module doc says the public surface is `Agent.send()`/`Agent.steer()` — and the [architecture dependency rule](../../../architecture.md): nothing programs against `dsh-agent-loop`; a replacement loop is a different bundle built on `dsh-agent`, not a consumer of this package's internals. `ReactLoopAgent` stays exported (cross-package tests construct it by package name). +2. **The `runLoop`, `Inbox`, and `InboxMessage` exports** (`packages/core/agent-loop/src/index.ts`). `runLoop` has no importer outside the package — the only callers are the package's own internals (the agent constructs its loop with it), so the public re-export has zero consumers; `Inbox`/`InboxMessage` likewise reach outside code only through the package's own inbox spec (switchable to the source module). The exports contradict the package's own docs — the inbox module doc says the public surface is `Agent.send()`/`Agent.steer()` — and the [architecture dependency rule](../../../architecture.md): nothing programs against `dsh-agent-loop`; a replacement loop is a different bundle built on `dsh-agent`, not a consumer of this package's internals. `ReactLoopAgent` stays exported (cross-package tests construct it by package name). 3. **`ToolExecutionResult.callId`** (`packages/core/tools/src/index.ts`; the *input* `ToolExecution.callId` stays). Zero readers. The loop deliberately ignores it and documents it as a footgun — the correlation id must be the loop's own `call.id`, because a `tools/execute` waterfall listener returning a mismatched id would otherwise orphan the call↔result pairing — and a regression test exists solely to prove the field is ignored. So every waterfall short-circuiter must fabricate a field whose only power is to be a bug if trusted; the ACP bridge correlates via the session event's `data.callId`, never via the execution result. ## Proposal -Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites, and `toolErrorResult`, along with the loop's ignore-comment and the proves-ignored regression test — the hazard they guard disappears with the field. +Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites, and `toolErrorResult`, along with the loop's ignore-comment and the proves-ignored regression test — the hazard they guard disappears with the field. Update the `ToolExecutionResult` paste in [tools.md](../../../core-data-structures/tools.md) (and its `scripts/type-equiv.manifest.json` row) and the result-shape row in `packages/core/tools/README.md`; for the `invalidate()` removal, amend the [session-surface RFC](../../implemented/architecture/2026-06-18-session-surface.md)'s full-rebuild-after-wholesale-replacement sentence per [implemented/AGENTS.md](../../implemented/AGENTS.md). Sequencing: the in-flight surface-cache work (tool-pairing balance caching) neither uses nor touches `invalidate`, so that removal lands after or alongside it mechanically. The `callId` removal waits for the in-flight interception-seams work that splits `tools/execute` into pre/post phases and currently carries the field verbatim — the argument transfers unchanged (post-execute listeners receive the execution object alongside the result), so the removal targets whichever seam shape is on master when implemented. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md index e663c4c90f..b026c76349 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md @@ -12,7 +12,7 @@ The merge-extensible vocabulary maps are designed to grow by declaration merging ## Proposal -Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the two test fixtures to `injection` triggers (any non-`message` trigger serves their purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change. +Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the two test fixtures to `injection` triggers (any non-`message` trigger serves their purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change, and amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s consequence line naming cache hints as having a home, per [implemented/AGENTS.md](../../implemented/AGENTS.md). Each variant returns the day it gains a real producer, exactly as the maps are designed to grow: a caching feature re-adds `cache` together with the adapter that transmits it; subagent attribution re-adds `agent` together with the backend that stamps it and a consumer that routes on it; an auto-continue feature that genuinely starts new turns re-adds `continuation` with the plugin that emits it. From f04b25478e00d930fc7cdc6ca7b75dfe392ff7b3 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 04:33:06 +0800 Subject: [PATCH 04/10] docs(rfc): align acceptance criterion with corrected runLoop scope; two more image doc sites The core-spine RFC's acceptance criterion still asserted all three surfaces 'appear only in this RFC', contradicting the corrected scope (runLoop/Inbox stay as package-internal symbols; only the public re-exports go). Also add the deepseek README image-skip row and the compact-basic [image]-placeholder row to the image RFC's removal set. --- .../simplification/2026-07-04-drop-image-content-block.md | 2 +- .../simplification/2026-07-04-prune-dead-core-spine-surface.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md index c949286b57..789226032b 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md @@ -8,7 +8,7 @@ Status: proposed ## Proposal -Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation row, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. +Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the deepseek README's image-skip row, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation and `[image]`-placeholder rows, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. ## Why not keep it? diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md index bb9d0a8447..80a9fa0a62 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md @@ -22,7 +22,7 @@ A future consumer that swaps a session's log in place would want a reset primiti ## Acceptance criteria -- The three surfaces appear only in this RFC; the agent-loop README lists only the consumed public surface; the inbox spec imports the source module. +- `invalidate()` and the result `callId` appear only in this RFC; `runLoop`/`Inbox`/`InboxMessage` remain package-internal only — no re-export from the package index and no outside-package importer; the agent-loop README lists only the consumed public surface; the inbox spec imports the source module. - The tools/execute contract tests pass with the shrunk result type; no waterfall test fabricates a `callId` on a result. ## Risks From 95b9ac0d3e503a0518cb518799f9ff0339820177 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:04:49 +0800 Subject: [PATCH 05/10] docs: refresh simplification RFC sweep --- docs/rfc/README.md | 7 ++- .../2026-07-04-hook-snapshot-matrix.md | 2 +- .../2026-07-04-generate-rfc-index-tables.md | 43 +++++++++++++++++ ...drop-idle-registry-observation-surfaces.md | 17 ++++--- .../2026-07-04-fold-stdio-ui-helper.md | 42 +++++++++++++++++ .../2026-07-04-narrow-pre-tool-gate.md | 45 ++++++++++++++++++ ...-04-narrow-subagent-synchronous-collect.md | 22 +++++---- .../2026-07-04-prune-bash-task-roster.md | 40 ---------------- .../2026-07-04-trim-hook-protocol-surface.md | 46 +++++++++++++++++++ examples/acp-agent/tests/acp.e2e.ts | 3 ++ examples/acp-agent/tests/acp.snapshot.ts | 3 ++ examples/acp-agent/tests/hooks.e2e.ts | 2 +- scripts/gen-cordis-catalog.ts | 3 ++ 13 files changed, 217 insertions(+), 58 deletions(-) create mode 100644 docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 8dbc459c4d..8358ab2ca2 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -52,9 +52,11 @@ 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 | | [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 | +| [Drop idle registry and status observation surfaces](proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md) | 2026-07-04 | | [Remove defaults from the tool-schema DSL](proposed/simplification/2026-07-04-remove-tool-schema-defaults.md) | 2026-07-04 | +| [Trim unused hook protocol and bridge surface](proposed/simplification/2026-07-04-trim-hook-protocol-surface.md) | 2026-07-04 | +| [Narrow the pre-tool gate to shipped behavior](proposed/simplification/2026-07-04-narrow-pre-tool-gate.md) | 2026-07-04 | +| [Fold the stdio UI helper into the stdio app](proposed/simplification/2026-07-04-fold-stdio-ui-helper.md) | 2026-07-04 | ### Architecture @@ -71,6 +73,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [API extractor reports](proposed/process/2026-06-11-api-extractor-reports.md) | 2026-06-11 | | [Supply chain checks and vendor drift verification](proposed/process/2026-06-11-supply-chain-and-vendor-drift.md) | 2026-06-11 | | [Discover package inventories instead of maintaining static lists](proposed/process/2026-06-20-discover-package-inventory.md) | 2026-06-20 | +| [Generate the RFC index tables](proposed/process/2026-07-04-generate-rfc-index-tables.md) | 2026-07-04 | ### Testing diff --git a/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md b/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md index bf4926e86b..f8ec029d22 100644 --- a/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md +++ b/docs/rfc/implemented/testing/2026-07-04-hook-snapshot-matrix.md @@ -4,7 +4,7 @@ Status: implemented ## Problem -The hook bridges — [`dsh-hooks-claude`](../../../../packages/hooks/hooks-claude) (7 Claude Code hook points) and [`dsh-hooks-codex`](../../../../packages/hooks/hooks-codex) (5 Codex points) — map external hook commands onto the harness interception seams. They carry deep unit and coverage-spec coverage (every decision arm, every payload dialect, driven against a mocked seam) plus one key-gated e2e (`hooks.e2e.ts`, a live `PreToolUse` block). But the full-transcript snapshot tier — the one net that boots the real `acp-agent` subprocess, replays a recorded session keyless, and diffs the normalized ACP stdout + re-persisted log against committed goldens — covered exactly ONE hook: a Claude `UserPromptSubmit` block (`hook-prompt-block`). +The hook bridges — [`dsh-hooks-claude`](../../../../packages/hooks/hooks-claude) (7 Claude Code hook points) and [`dsh-hooks-codex`](../../../../packages/hooks/hooks-codex) (5 Codex points) — map external hook commands onto the harness interception seams. They carry deep unit and coverage-spec coverage (every decision arm, every payload dialect, driven against a mocked seam) plus one key-gated e2e (`hooks.e2e.ts`, a live `PreToolUse` block). But the full-transcript snapshot tier — the one net that boots the real `acp-agent` subprocess, replays a recorded session keyless, and diffs the normalized ACP stdout + re-persisted log against committed goldens — covered exactly ONE hook: a Claude `UserPromptSubmit` block (`hook-cc-promptsubmit-block`). That is the tier a mocked unit test structurally cannot be: it exercises the REAL bridge translating a REAL hook process's outcome into the REAL seam decision, then the REAL loop's reaction, rendered exactly as an editor sees it. A bridge-translation or loop-structure regression that left every unit green would still escape it for every hook point but one — and for the Codex bridge, the ACP example did not even LOAD it, so no Codex hook could fire end-to-end at all. diff --git a/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md b/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md new file mode 100644 index 0000000000..f68d34f2eb --- /dev/null +++ b/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md @@ -0,0 +1,43 @@ +# RFC: Generate the RFC index tables + +Status: proposed + +## Problem + +`docs/rfc/README.md` is hand-maintained even though the repo already has a machine-readable RFC layout: every RFC lives at `docs/rfc/{lifecycle}/{class}/yyyy-mm-dd-topic.md`, and `scripts/verify-rfc-classification.ts` walks that tree to verify structure and index completeness. The current gate prevents drift, but every new RFC still edits the same README tables by hand. + +The stacked hook work made the cost visible. PR #138 added implemented feature/testing/process rows while this simplification sweep added proposed simplification rows, and the only merge conflict when retargeting the sweep onto #138 was the RFC index table. That is predictable: high-churn proposal waves all touch the same few lines even though the truth is already in filenames and H1 titles. + +[The classification RFC](../../implemented/process/2026-06-20-rfc-classification.md) explicitly rejected auto-generating the README index so the file could stay curated. That was a reasonable first cut, but the repo now has enough RFC volume and stacked-PR churn that the hand-written table is the unstable part, not the curated prose. The verifier already does the expensive parsing; it just reports instead of writing. + +## Proposal + +Keep the curated prose in `docs/rfc/README.md`, but generate the per-lifecycle/per-class tables from the filesystem. + +- Add a `gen-rfc-index` script (or extend `verify-rfc-classification.ts` with `--write`) that scans RFC files, reads each H1, derives the first-proposed date from the filename, and writes the table rows under stable generated markers for each `## {Lifecycle}` / `### {Class}` section. +- Keep the class set and lifecycle set closed in one script-owned source of truth. +- Make `verify-rfc-classification` check that the generated sections are fresh, analogous to `verify-cordis-catalog`. +- Preserve manually curated prose, classification descriptions, and "when to write one" guidance outside the generated table blocks. +- Update [the classification RFC](../../implemented/process/2026-06-20-rfc-classification.md) to say the earlier "verify, do not generate" choice was superseded after stacked-PR conflicts made the tradeoff worse. + +The generated output should stay boring Markdown: the same tables reviewers read today, just mechanically produced from the path + title source of truth. + +## Why not keep the current verifier-only model? + +The current model catches mistakes but still forces every proposal to edit a shared hotspot. A failed verifier is also more annoying than a generator for a purely mechanical row: the author has already named and placed the file correctly, then has to copy the same facts into the index. That is exactly the kind of hand-maintained inventory the repo already proposes removing elsewhere. + +This does not turn the whole README into a build artifact. The prose remains curated. Only the parts whose content is derivable from RFC files become generated. + +## Acceptance criteria + +- `pnpm run gen-rfc-index` (or the chosen command) rewrites only the generated RFC table regions. +- `pnpm run verify-rfc-classification` fails when those generated regions are stale and passes after regeneration. +- Adding, moving, or deleting an RFC requires editing the RFC file itself; the README rows are produced mechanically. +- The generated rows use each RFC's H1 title and filename date, and preserve the existing lifecycle/class grouping. +- `pnpm run doc-sync` passes after implementation. + +## Risks + +- Generated regions inside a curated README can be jarring. Use explicit markers and keep the table output minimal so reviewers know what is owned by the script. +- Reading H1 titles makes malformed RFC headers a generator concern. That is useful pressure: a missing or nonstandard H1 should fail clearly. +- This supersedes an implemented process decision. The implementing PR must amend the old classification RFC so the historical record explains why the tradeoff changed. diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md b/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md index 74f05d78a9..142972f3c7 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md @@ -1,4 +1,4 @@ -# RFC: Drop idle registry observation surfaces +# RFC: Drop idle registry and status observation surfaces Status: proposed @@ -12,6 +12,8 @@ Those events carry real complexity. Each registry yields a rollback disposer bef 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. +The same "status without observer" pattern now shows up in the web seam. `ctx.web.searchStatus()` and `ctx.web.fetchStatus()` are documented as diagnostics for `dsh-tool-web`, but the current tools execute directly through `ctx.web.search()` and `ctx.web.fetch()` ([packages/web/tool-web/src/search.ts](../../../../packages/web/tool-web/src/search.ts), [packages/web/tool-web/src/fetch.ts](../../../../packages/web/tool-web/src/fetch.ts)). The execution path already resolves the selected provider at call time and throws a structured `WebError` (`WEB_PROVIDER_UNAVAILABLE`, `WEB_PROVIDER_AMBIGUOUS`, `WEB_PROVIDER_CONFIGURED_MISSING`, `WEB_PROVIDER_CONFIGURED_UNAVAILABLE`) when the capability cannot run. The status methods duplicate that selection logic for tests and stale docs, not for a live product surface. + ## Proposal Remove the idle registry-observation surfaces that have no production consumer: @@ -20,18 +22,19 @@ Remove the idle registry-observation surfaces that have no production consumer: - 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. +- Delete `WebService.searchStatus()` / `fetchStatus()` and the `WebCapabilityStatus` contract if no other live type needs it. Web provider `status()` stays internal to provider resolution; callers observe availability by attempting `search()` / `fetch()` and handling `WebError`. 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 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.search()` and `ctx.web.fetch()` stay because they are the model-facing web tools' execution path and they already carry the provider-selection error taxonomy. `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. +A live tool palette, prompt-section inspector, web-provider status panel, or model picker might eventually want registry-change signals or status queries. But none exists today, and the current event payloads/status shapes 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. @@ -39,12 +42,14 @@ The pre-release stance cuts in favor of narrowing now. A public event with no li - `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. +- `rg "searchStatus|fetchStatus|WebCapabilityStatus" packages/web docs --glob '!docs/rfc/**'` finds no remaining public web status surface, docs entry, generated-catalog entry, or tests except provider-private status concepts that still feed execution. - 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. +- 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 and status 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. +- 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 execute or throw the expected `WebError`, and an adapter can stream for its model. +- Web tests lose a cheap status assertion. They should assert the behavior a real caller observes: successful `search()` / `fetch()` for a usable provider and structured `WebError` codes for unavailable, ambiguous, or misconfigured provider sets. +- A future UI may need observer hooks or status queries. That is fine; the hook/query should return with that UI, not ahead of it. diff --git a/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md b/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md new file mode 100644 index 0000000000..4ef1a264b0 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md @@ -0,0 +1,42 @@ +# RFC: Fold the stdio UI helper into the stdio app + +Status: proposed + +## Problem + +`@deepseek-ai/dsh-ui-stdio` lives under `packages/support/`, but its only runtime importer is the product app package `@deepseek-ai/dsh-stdio-agent` ([packages/ui/stdio-agent/src/index.ts](../../../../packages/ui/stdio-agent/src/index.ts)). Direct `createStdioChat()` uses are package-local tests and the production wrapper inside the same support package. The examples reach it by loading `dsh-stdio-agent`, not by composing the UI helper themselves. + +That leaves an awkward package boundary. `support/` is documented as lower-compat dev/test/example infrastructure, and the `ui-stdio` README says it is a convenience REPL, not a product surface. But `dsh-stdio-agent` is a shipped app package whose front-door cluster always includes the readline UI, console logger, JSONL persistence, and a pre-created `main` agent. In practice the helper is not an independent swappable capability; it is an implementation detail of the stdio app. + +The boundary adds package metadata, workspace references, generated module-graph rows, README entries, publish lint surface, and a cross-group dependency from `packages/ui/stdio-agent` to `packages/support/ui-stdio`. It also creates a policy mismatch: a product UI app depends on a support package whose docs say it should not be treated as load-bearing product surface. + +## Proposal + +Fold the stdio UI helper into `@deepseek-ai/dsh-stdio-agent`. + +- Move the `createStdioChat` implementation, its `StdioRuntime` test seam, and its unit tests into `packages/ui/stdio-agent`. +- Delete the `packages/support/ui-stdio` package, package references, path aliases, dependency entries, module-graph rows, and support README row. +- Keep the testable runtime seam inside `dsh-stdio-agent` so EOF handling, rendering, disposal, and piped-vs-TTY behavior remain covered without hijacking process globals. +- Update docs that currently point at `../support/ui-stdio` to describe stdio rendering as part of the stdio app. + +After the fold, the stdio app owns its front door the same way `dsh-acp-agent` owns its ACP bridge cluster. The examples still load one app package; no leaf config has to learn a new plugin. + +## Why not promote it to `packages/ui/` instead? + +Promotion would fix the support/product mismatch but keep the extra package boundary. That would make sense if more than one product app composed `createStdioChat()` directly, or if the readline UI were a swappable UI integration in its own right. The current consumer audit says neither is true. The stdio app is the consumer and the owner. + +Re-extraction stays cheap while the repo is unreleased. If a second product app needs the same readline UI independently, split it back out then, with that consumer shaping the package contract. + +## Acceptance criteria + +- `rg "@deepseek-ai/dsh-ui-stdio|support/ui-stdio|createStdioChat" packages examples docs scripts --glob '!docs/rfc/**' --glob '!**/lib/**'` finds no deleted package dependency or docs reference; `createStdioChat` remains only as an internal/tested helper under `packages/ui/stdio-agent` if the name survives. +- The stdio app still prints transcript events, handles stdin lines/EOF, renders todo updates, and disposes readline listeners under HMR. +- Echo/coding-agent keyless smoke tests still boot through the real Loader path and guard the named-export shape. +- Package manifests, tsconfig project references, generated module graph, and docs are updated. +- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, `pnpm run build`, and `pnpm run hygiene` pass after implementation. + +## Risks + +- `dsh-ui-stdio` currently has focused tests with a small package-local setup. Moving them risks blurring app composition tests with UI rendering tests; keep the helper test seam and colocated unit tests to avoid that. +- A future standalone terminal UI may want the helper as a package. Reintroduce it when a second product consumer exists rather than keeping a boundary for hypothetical reuse. +- Docs that mention the stdio UI as a support example need careful wording so they still distinguish the non-product terminal demo from the ACP product surface. diff --git a/docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md b/docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md new file mode 100644 index 0000000000..ee17588235 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md @@ -0,0 +1,45 @@ +# RFC: Narrow the pre-tool gate to shipped behavior + +Status: proposed + +## Problem + +The `tools/pre-execute` seam advertises two pieces of deferred capability that are not actually supported end to end: interactive `ask` permission and pre-tool argument rewrite. + +`PreToolDecision` includes `{ kind: 'ask' }`, but `ToolRegistry.execute()` treats every non-`allow` decision as a denied tool result because no permission UI exists yet ([packages/core/tools/src/index.ts](../../../../packages/core/tools/src/index.ts)). The only production producer is `dsh-hooks-claude`, which maps Claude Code `permissionDecision: "ask"` into that variant; Codex has no allow/ask path. The durable hook log can still record that an external hook asked, but the canonical typed seam cannot do anything distinct with it. The public union therefore has a third branch whose runtime semantics are "deny with a different default string." + +The same seam also has an unadvertised argument-rewrite escape hatch. The docs correctly say input rewrite is not offered because `assistant/message`, `tool/call`, and live presentation all see the model's original arguments before execution; changing only `exec.arguments` would make the UI/audit/history disagree with what ran. Yet `ToolExecution.arguments` is mutable, and dispatch reads `exec.arguments` after `tools/pre-execute`, so a listener can rewrite it anyway. A test shim does exactly that to thread a generated bash task id ([packages/bash/tool-bash/tests/integration.spec.ts](../../../../packages/bash/tool-bash/tests/integration.spec.ts)). The proposed [pre-tool input rewrite RFC](../feature/2026-06-30-pre-tool-input-rewrite.md) exists because doing this consistently is a design unit, not a hidden mutation. + +Both shapes are honest feature deferrals, but the public seam currently encodes them as if they were ready. That makes bridge code, docs, generated catalogs, and tests explain behavior whose only shipped result is "deny" or "mutate at your own risk." + +## Proposal + +Make `tools/pre-execute` express the behavior it can actually provide today: allow or deny a pending tool call, without argument mutation. + +- Remove `{ kind: 'ask' }` from `PreToolDecision`. The Claude bridge should still parse and log hook `ask` decisions, but map them to `deny` at the typed seam with an approval-not-supported reason until a real permission prompt exists. +- Update docs, generated catalogs, hook bridge README tables, and tests so `tools/pre-execute` is an allow/deny gate, not an allow/deny/ask gate. +- Make `ToolExecution.arguments` immutable by contract. At minimum mark it `readonly` and stop relying on a listener-mutated `exec.arguments` for dispatch; if a defensive runtime copy/freeze is needed to make the contract true, add it at the `ToolRegistry.execute()` boundary. +- Rewrite the one test shim that mutates `exec.arguments` to use a behavior-level helper instead of the hidden rewrite path. + +When permission prompts or consistent input rewrite lands, reintroduce the smallest explicit decision shape those features need. `ask` belongs with a real user approval loop; argument rewrite belongs with the audit/history/presentation update described by the proposed rewrite RFC. + +## What we give up + +Claude `permissionDecision: "ask"` no longer has a distinct typed-decision branch inside `dsh-tools`. The bridge can still preserve the external fact in `hook/result.decision` and still deny the call conservatively. That matches current product behavior without requiring every native plugin to handle an unusable branch. + +Internal tests lose a convenient mutable-object trick. That is a good loss: public tests should not depend on an unadvertised inconsistency that production docs warn against. + +## Acceptance criteria + +- `PreToolDecision` contains only `allow` and `deny`. +- `dsh-hooks-claude` still records hook `ask` in hook provenance, but returns a `deny` decision to `tools/pre-execute`. +- `rg "kind: 'ask'|PreToolDecision.*ask|ask.*degrades" packages docs --glob '!docs/rfc/**'` finds no remaining public pre-tool ask contract outside historical RFC text. +- `ToolExecution.arguments` is no longer a writable rewrite path, and `rg "exec\\.arguments\\s*=" packages examples --glob '!docs/rfc/**' --glob '!**/lib/**'` finds no mutation. +- The proposed pre-tool input rewrite RFC remains the future home for a consistent rewrite design. +- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation. + +## Risks + +- A native plugin author may already have experimented with `ask`. The repo is unreleased, and the branch currently cannot prompt a user; collapsing it now avoids shipping a promise that cannot be honored. +- Making arguments immutable may reveal more test helpers that were relying on mutation. Those helpers should move closer to the behavior they actually need instead of preserving a public inconsistency. +- Future permission and rewrite work will add back surface area. That is fine; the new surface should land with the product workflow and consistency guarantees that make it real. diff --git a/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md b/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md index c0598edab4..6f68dceb59 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md +++ b/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md @@ -8,24 +8,26 @@ The implemented [subagent seam](../../implemented/feature/2026-06-21-subagent-ca 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 #138 hook stack made one earlier simplification idea too broad: `subagent/start` and `subagent/end` are now live. `dsh-hooks-claude` listens to `subagent/start` to run a `SubagentStart` hook and inject any returned `additionalContext` into the live child, and listens to `subagent/end` to run `SubagentStop` ([packages/hooks/hooks-claude/src/index.ts](../../../../packages/hooks/hooks-claude/src/index.ts)). Those lifecycle emits should stay. What remains idle is the registry-observation surface around the provider map: `ctx.subagents.getProvider()` and `ctx.subagents.list()` still have declarations, docs, generated-catalog entries, and tests, but no production caller. -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." +The new hook stack also exposes an overreach inside the lifecycle payload. [The subagent observe-enrichment RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md) added `lastAssistantMessage` so a hooks bridge could forward the child output to a `SubagentStop` handler, but the current `SubagentStop` payload builder does not read it; it emits only `agent_id`, `agent_type`, and `stop_hook_active`. The field therefore buys a `structuredClone` branch, clone-failure containment, docs, and tests without changing any shipped hook behavior. If `SubagentStop` should carry the final child message, that should be implemented end to end; until then the payload should be honest. + +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 final-output lifecycle cloning even though the real model-facing behavior is "start a named child, await its final result, cancel or dispose it," plus observe-only lifecycle emits the Claude hook bridge actually consumes. ## Proposal -Make the subagent seam describe the behavior the harness actually uses today: synchronous collect only. +Make the subagent seam describe the behavior the harness actually uses today: synchronous collect plus the two live observe-only lifecycle emits. - 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. +- Keep `subagent/start` and `subagent/end`, but narrow their payloads to the fields the live bridge can use: `provider`, `id`, and on end `stopReason`. Remove `SubagentRunEndInfo.lastAssistantMessage`, the `structuredClone(result.output)` branch, and the clone-failure tests/docs. - 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. +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. The service still emits `subagent/start` / `subagent/end` around that run because the hook bridge now consumes them. ## Why not keep the dormant guard? @@ -35,21 +37,25 @@ If a hard recursion limit is needed, it should come back as an actually wired pr ## 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. +Programmatic callers lose prebuilt hooks for structured subagent output, child tool scoping, live steering, follow-up resume, provider enumeration, and final-output 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. +The Claude bridge would no longer be able to forward a child final message to `SubagentStop` without a later payload change. That is also honest: the current bridge does not forward it now. If that behavior becomes product-owned, reintroduce the field with the bridge payload and snapshot/unit coverage that prove the hook sees it. + ## 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. +- `rg "getProvider\\(|ctx\\.subagents\\.list\\(" packages examples docs --glob '!docs/rfc/**'` finds no production API surface. +- `rg "lastAssistantMessage" packages docs --glob '!docs/rfc/**'` finds no live contract, clone branch, test, or generated-catalog entry. +- `subagent/start` and `subagent/end` still exist, and `dsh-hooks-claude` still handles `SubagentStart` / `SubagentStop`. - 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 subagent UI may want richer lifecycle payloads. Keep the live emits now, but reintroduce extra fields only with that UI and a payload it actually consumes. - 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. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md b/docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md deleted file mode 100644 index 9577f5930d..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-bash-task-roster.md +++ /dev/null @@ -1,40 +0,0 @@ -# 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. diff --git a/docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md b/docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md new file mode 100644 index 0000000000..d7de6d5d66 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md @@ -0,0 +1,46 @@ +# RFC: Trim unused hook protocol and bridge surface + +Status: proposed + +## Problem + +The #138 hook stack added a useful bridge layer, but the current public protocol still exposes a few fields and knobs that no shipped writer or reader uses. They are small individually; together they widen the durable hook log, the shared hook-protocol API, and both bridge configs. + +`HookDialect` includes `'native'`, but real `hook/invoked` writers are the Claude and Codex bridges only. The worked native-plugin test explicitly proves the opposite: a native plugin uses typed Cordis decisions and emits no `hook/*` session events. Grepping `dialect: 'native'` finds a hook-protocol unit test, docs, and type text, not production code. + +`hook/result.durationMs` is durable timing telemetry with no production reader. Both bridges write it; the ACP snapshot normalizer immediately scrubs it to `0` because wall-clock hook runtime is replay noise ([examples/acp-agent/tests/snapshot-normalize.ts](../../../../examples/acp-agent/tests/snapshot-normalize.ts)). The only remaining consumers are tests and generated goldens that exist because the field exists. Persisting a value that replay must erase is a smell: it is neither product behavior nor useful audit state. + +`MergedHookOutcome.systemMessages` is also unused. The codec should still parse `HookOutput.systemMessage` because the external protocols can emit it and both bridges warn when it appears, but the merged aggregate is never read; `rg "systemMessages|\\.systemMessages"` finds the merge helper, README prose, and merge tests only. The bridge already handles warnings per raw output before merge. + +Finally, both bridge configs carry optional process-level defaults that shipped configs do not set. `defaultTimeoutMs` duplicates the reference default (`600_000`) even though each command hook already has its own `timeout`; tests mostly cover schema-bypass fallback. `dsh-hooks-codex` also exposes `Config.model`, but the ACP configs load the Codex bridge with only `configPath`, and every hook payload already has an `Agent` whose `options.model` is the actual model for that run. + +## Proposal + +Remove the unused protocol and config surface while keeping the live external-hook behavior: + +- Change `HookDialect` to `'claude' | 'codex'` until a real native `hook/*` producer exists. Native plugins keep using the typed interception seams directly. +- Remove `durationMs` from the `hook/result` session event, `HookResultRecord`, `RunHookResult`, bridge append calls, docs, generated catalog, snapshots, and the snapshot normalizer's special-case scrub. Remove the injected `now` clock from `runHook()` if it becomes unnecessary after the field disappears. +- Remove `MergedHookOutcome.systemMessages` and its tests/docs. Keep `HookOutput.systemMessage` parsing and the bridge warnings. +- Remove `defaultTimeoutMs` from both bridge configs. Keep per-command `timeoutSec`; when absent, `runHook()` uses a single shared protocol constant for the reference default. +- Remove `dsh-hooks-codex` `Config.model`; stamp Codex payloads from `agent.options.model ?? ''` at the point that has an agent, with `''` only for no-agent fallback paths. + +## What stays + +This RFC does not remove `hook/invoked` / `hook/result` themselves. They are live provenance: bridges append them around actual hook execution and ACP snapshots persist them. It also does not remove parsing/warning for `updatedInput`, `systemMessage`, `continue:false`, or `suppressOutput`; those are deliberate faithful-but-degraded external-protocol fields documented by [the hook bridge RFC](../../implemented/feature/2026-06-30-hook-bridges.md). + +This RFC does not collapse the shared `dsh-hook-protocol` package into the bridges or build a single parameterized bridge engine. [The protocol-library RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) explicitly keeps only the identical wire primitives shared and leaves per-dialect payload/config mapping in each bridge. + +## Acceptance criteria + +- `rg "HookDialect.*native|dialect: 'native'|claude.*/.*codex.*/.*native|claude.*codex.*native" packages/hooks docs/core-data-structures/session.md docs/cordis-catalog/events-and-services.md --glob '!docs/rfc/**'` finds no `HookDialect` branch, test writer, or `hook/*` docs claiming a native durable writer. +- `rg "durationMs" packages/hooks examples/acp-agent/tests docs/core-data-structures/session.md docs/cordis-catalog/events-and-services.md --glob '!docs/rfc/**'` finds no hook-result field, snapshot scrub, or generated-golden requirement outside unrelated timing concepts. +- `rg "systemMessages|\\.systemMessages" packages/hooks docs --glob '!docs/rfc/**'` finds no merged aggregate surface, while `systemMessage` parsing and bridge warnings remain covered. +- `rg "defaultTimeoutMs|Config\\.model|model\\?: string" packages/hooks docs --glob '!docs/rfc/**'` finds no bridge config knob for the removed defaults, while per-hook timeout support and Codex payload model stamping still work. +- Hook bridge unit tests and ACP hook snapshots still prove prompt-submit, pre-tool, post-tool, and stop behavior for both dialects. +- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation. + +## Risks + +- Durable hook timing can be useful diagnostics. If a product UI or trace viewer wants it, add live diagnostics or an intentionally durable telemetry event then; do not keep replay-noisy timing in the base hook-result record without a reader. +- A future native hook provenance logger might want `dialect: 'native'`. Add it with that logger. Until then, documenting native hooks as `hook/*` writers blurs the important design point that native plugins do not need the shell-hook log. +- A deployment could want a process-level Codex model override for hook payloads. The agent already knows its actual model, which is less surprising than a bridge-level default that can drift from the run being observed. diff --git a/examples/acp-agent/tests/acp.e2e.ts b/examples/acp-agent/tests/acp.e2e.ts index ee60a7d131..06ef962743 100644 --- a/examples/acp-agent/tests/acp.e2e.ts +++ b/examples/acp-agent/tests/acp.e2e.ts @@ -56,6 +56,9 @@ interface Spawned { stderr: string[] } +// TODO(acp-test-harness): this subprocess/client boot glue is duplicated with +// hooks.e2e.ts and partly with snapshot-harness.ts. Extract one shared ACP test +// launcher before the TSX/env/permission-stub details drift again. function spawnAcpAgent(cwd: string, env: NodeJS.ProcessEnv = process.env): Spawned { const child = spawn( process.execPath, diff --git a/examples/acp-agent/tests/acp.snapshot.ts b/examples/acp-agent/tests/acp.snapshot.ts index 36ae8edf14..65d2dee9da 100644 --- a/examples/acp-agent/tests/acp.snapshot.ts +++ b/examples/acp-agent/tests/acp.snapshot.ts @@ -106,6 +106,9 @@ const SCENARIOS: Scenario[] = [ { name: 'hook-cc-promptsubmit-context', hasModelTurn: true, recorded: true }, { name: 'hook-cc-pretool-deny', hasModelTurn: true, recorded: true }, { name: 'hook-cc-pretool-ask', hasModelTurn: true, recorded: true }, + // TODO(hook-snapshot-noise): re-record the PostToolUse block fixtures with a + // self-limiting prompt or hook so one rejected result proves the seam without + // repeated block/retry cycles in the committed JSONL. { name: 'hook-cc-posttool-block', hasModelTurn: true, recorded: true }, { name: 'hook-cc-posttool-context', hasModelTurn: true, recorded: true }, { name: 'hook-cc-stop-continue', hasModelTurn: true, recorded: true }, diff --git a/examples/acp-agent/tests/hooks.e2e.ts b/examples/acp-agent/tests/hooks.e2e.ts index 40a8d37457..bdb800186a 100644 --- a/examples/acp-agent/tests/hooks.e2e.ts +++ b/examples/acp-agent/tests/hooks.e2e.ts @@ -29,7 +29,7 @@ import { * Key-gated; owns and disposes its subprocess. * * A keyless companion lives in acp.e2e.ts (stdout purity + session/new); the - * full hook-fires-end-to-end transcript is the keyless `hook-prompt-block` + * full hook-fires-end-to-end transcript is the keyless `hook-cc-promptsubmit-block` * snapshot scenario. This one closes the "green plumbing, broken product" gap: * only a real model deciding to call bash exercises the PreToolUse seam live. */ diff --git a/scripts/gen-cordis-catalog.ts b/scripts/gen-cordis-catalog.ts index 8d84eff57a..c57f42ed8f 100644 --- a/scripts/gen-cordis-catalog.ts +++ b/scripts/gen-cordis-catalog.ts @@ -57,6 +57,9 @@ type Mode = 'emit' | 'waterfall' | 'parallel' | 'serial' * that manifest documents the `…Map` symbols (`ContentBlockMap`) while * signatures reference the derived UNION names (`ContentBlock`), and it lists a * few symbols on two pages. Here each name resolves to exactly one PRIMARY page. + * TODO(catalog-type-links): add a verifier or generator for link-map coverage + * so new hook-era decision types like `PromptDecision` / `PreToolDecision` do + * not silently appear in signatures without a "Types:" link. */ const LINK_MAP: Record = { Agent: 'core.md', From 9ebc38badf15f138f63c48e42738b0ef34adb876 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:24:44 +0800 Subject: [PATCH 06/10] docs(rfc): revise the batch for the hooks-stack base; add three post-stack RFCs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The branch now bases on the hooks stack (PR #138's head), so every 'in-flight' reference to stack content became a current-state fact. Audited all nine RFCs + three supplements claim-by-claim against the merged tree (none invalidated; several strengthened): - prune-dead-core-spine-surface: describe the landed tools/pre-execute → dispatch → tools/post-execute pipeline — listeners return Decisions, the registry builds every result and snapshots it to protect callId, and a second (mutation-guard) test pins the field; drop the resolved wait-for-interception hedge; scope the additionalContext ferry out. - prune-producerless-vocabulary-variants: the ui-stdio fixture migrated off the continuation trigger (llm-replay is the sole writer now); the stack's own additions (rejected, prompt/blocked, hook/invoked+result) all arrived with producers — the admission policy demonstrated live. - prune-unimplemented-subagent-vocabulary: enrichment landed as lastAssistantMessage only (agentType was dropped in the stack's own review — the same judgment this RFC extends); the seam RFC now names tools/pre-execute deny, which exists, sharpening the re-add path. - drop-inert-request-knobs / drop-image-content-block / trim-acp-bridge-unreachable-surface: current-state rewordings (shipped bridges set no request fields; only compact-basic has explicit image arms; 13 hook goldens also pin agentInfo). - generic-long-running-tool-runtime census: second production seam consumer (hook-protocol runHook: resolve+run, stdin/env, foreground only — background machinery stays single-consumer); scrub-duplication blast radius. - discover-package-inventory: identical 54-entry tsconfig reference sets; the comparesLog scenario knob (fixture-derivable, like recorded). - unify-agent-and-session-id: third divergence site (in-process subagent children mint two UUIDs), the hooks bridge id-lookups, and ui-stdio's labelBySession map as a consumer that deletes under unification. New RFCs from the post-stack survey: - remove-agent-steering-mirror: the last mirror-of-durable event; zero production listeners; both retention RFCs deferred its fate, and the 'no durable twin' rationale is contradicted by the adjacent append. - tighten-hook-protocol-contract: producer-less 'native' dialect, parsed-and-discarded suppressOutput, and hook/result semantics (truncation + decision-string) defined twice in the bridges instead of the lib that owns the event. - single-source-acp-replay-config: cordis.yml/cordis.snapshot.yml differ by exactly one plugin entry, with no gate on the forced symmetry. --- docs/rfc/README.md | 3 ++ ...06-20-generic-long-running-tool-runtime.md | 2 +- .../2026-06-20-discover-package-inventory.md | 4 +-- .../2026-06-20-unify-agent-and-session-id.md | 7 +++-- .../2026-07-04-drop-image-content-block.md | 2 +- .../2026-07-04-drop-inert-request-knobs.md | 2 +- ...026-07-04-prune-dead-core-spine-surface.md | 10 +++---- ...-prune-producerless-vocabulary-variants.md | 8 ++--- ...prune-unimplemented-subagent-vocabulary.md | 4 +-- ...2026-07-04-remove-agent-steering-mirror.md | 28 ++++++++++++++++++ ...26-07-04-tighten-hook-protocol-contract.md | 29 +++++++++++++++++++ ...-04-trim-acp-bridge-unreachable-surface.md | 2 +- ...6-07-04-single-source-acp-replay-config.md | 26 +++++++++++++++++ 13 files changed, 107 insertions(+), 20 deletions(-) create mode 100644 docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md create mode 100644 docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index d983022f33..72579726c0 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -60,6 +60,8 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Trim unreachable ACP bridge surface — the branding knobs and the kind-sniffing fallback](proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md) | 2026-07-04 | | [Prune dead core-spine surface — `SurfaceManager.invalidate()`, the loop-internal exports, `ToolExecutionResult.callId`](proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md) | 2026-07-04 | | [Share the app bins' boot glue instead of maintaining twin copies](proposed/simplification/2026-07-04-share-app-bin-boot-glue.md) | 2026-07-04 | +| [Remove the `agent/steering` mirror emit](proposed/simplification/2026-07-04-remove-agent-steering-mirror.md) | 2026-07-04 | +| [Tighten the hook-protocol contract — the `native` dialect, `suppressOutput`, and lib-owned `hook/result` semantics](proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md) | 2026-07-04 | ### Architecture @@ -83,6 +85,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r |---|---| | [Mutation testing as the coverage counterweight](proposed/testing/2026-06-11-mutation-testing.md) | 2026-06-11 | | [Deterministic tests, the replay invariant fixture, and race stress](proposed/testing/2026-06-11-deterministic-and-stress-testing.md) | 2026-06-11 | +| [Single-source the acp-agent replay config](proposed/testing/2026-07-04-single-source-acp-replay-config.md) | 2026-07-04 | ## Implemented diff --git a/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md index 87336a4868..7f7c5ed007 100644 --- a/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md +++ b/docs/rfc/proposed/architecture/2026-06-20-generic-long-running-tool-runtime.md @@ -24,7 +24,7 @@ The runtime should own: ## Current seam consumption -A consumer census of the surface the runtime would carve up. Production (`packages/bash/tool-bash/src/index.ts`) consumes `resolve`, `run`, `start`, `ownerOf`, `readOutput`, `kill`, and `onTaskDone`. `get()`/`list()` have test-harness consumers only — they were removed once and reverted on the merits (the implementation note in [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records the test-migration cost dwarfing the surface removed). The per-task `BashTask.done` promise has no consumer through the public seam either (`dsh-tool-bash` completes via `onTaskDone`), but it is production-load-bearing INSIDE the implementation: `dsh-bash-local`'s disposal awaits it to reach quiescence. The seam therefore exposes two public completion representations — the per-task promise and the global `onTaskDone` listener registry — and the shipped consumer uses only the latter: the runtime should pick exactly one public completion surface and record which. One shape wart for the split to dissolve: `BashExecSpec.timeoutMs` is required but ignored by `start()`, an artifact of sharing one spec type between foreground and background execution. +A consumer census of the surface the runtime would carve up. Production has two seam consumers: `packages/bash/tool-bash/src/index.ts` consumes `resolve`, `run`, `start`, `ownerOf`, `readOutput`, `kill`, and `onTaskDone`; and the hook bridges — via `dsh-hook-protocol`'s `runHook` (`packages/hooks/hook-protocol/src/runner.ts`) — consume `resolve` + `run` only, a foreground-only trusted-plugin caller that sets the seam's `stdin`/`env` fields, so the background machinery stays single-consumer (which sharpens the extraction premise). `get()`/`list()` have test-harness consumers only — they were removed once and reverted on the merits (the implementation note in [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records the test-migration cost dwarfing the surface removed). The per-task `BashTask.done` promise has no consumer through the public seam either (`dsh-tool-bash` completes via `onTaskDone`), but it is production-load-bearing INSIDE the implementation: `dsh-bash-local`'s disposal awaits it to reach quiescence. The seam therefore exposes two public completion representations — the per-task promise and the global `onTaskDone` listener registry — and the shipped consumers use only the latter: the runtime should pick exactly one public completion surface and record which. Two shape facts for the split to dissolve or preserve deliberately: `BashExecSpec.timeoutMs` is required but ignored by `start()` (documented in the seam JSDoc itself), and `stdin`/`env` ride the shared spec for the foreground trusted-plugin path — the carve-up must keep a plain in-process foreground `resolve`+`run` path carrying them, so hook execution is never forced through the long-running runtime. Adjacent blast radius: the credential scrub is duplicated between the two production spawn sites (`packages/bash/bash-local/src/run.ts` and `packages/subagent/subagent-acp/src/run.ts`); if the runtime absorbs spawn-env policy, collapsing that duplication is its work too. ## Acceptance criteria diff --git a/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md index abdd9e9a41..a85e196ffd 100644 --- a/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/process/2026-06-20-discover-package-inventory.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` and the root `tsconfig.json` each hand-list every package as explicit project `references`. `knip.json` restates a per-package `entry` stanza for each package that gains an `*.e2e.ts` suite — byte-identical overrides that exist only because the shared `packages/*/*` stanza omits the e2e glob (an entry glob matching no files is inert, so the default stanza could carry it for every package). The ACP snapshot suite's scenario table (`examples/acp-agent/tests/acp.snapshot.ts`) hand-maintains a `childSessions` count per scenario that duplicates the number of `session..jsonl` fixture siblings on disk. These lists are small today, but every new package or gate creates another manual synchronization point. +Package and gate inventories are repeated by hand. The [package cookbook](../../../cookbook/adding-a-package.md) tells authors to update several files. The [package README](../../../../packages/README.md) carries a hand-written dependency graph. [CI](../../../../.github/workflows/ci.yml) and [development docs](../../../development.md) can drift from the actual `doc-sync` subcommands when new gates are added. `tsconfig.build.json` and the root `tsconfig.json` each hand-list every package as explicit project `references` — two identical sets that grow in lockstep, so a single generator can emit both — and `tsconfig.base.json`'s paths map hand-lists the per-group glob fan-out. `knip.json` restates a per-package `entry` stanza for each package that gains an `*.e2e.ts` suite — byte-identical overrides that exist only because the shared `packages/*/*` stanza omits the e2e glob (an entry glob matching no files is inert, so the default stanza could carry it for every package). The ACP snapshot suite's scenario table (`examples/acp-agent/tests/acp.snapshot.ts`) hand-maintains a `childSessions` count per scenario that duplicates the number of `session..jsonl` fixture siblings on disk. These lists are small today, but every new package or scenario class creates another manual synchronization point. The [package hierarchy](../../implemented/architecture/2026-06-20-package-hierarchy.md) already removed several of these by hand: `scripts/publint-all.ts` now derives its list from the `packages//` layout, and the two `tsconfig` `paths` maps collapsed to one `@deepseek-ai/dsh-*` wildcard. What remains is the inventory that cannot be globbed away — chiefly `tsconfig.build.json`'s project `references`, which TypeScript requires as an explicit array (no wildcard form). @@ -16,7 +16,7 @@ Make the remaining package/gate inventories discoverable. A single canonical sou The hierarchy does not need to encode every fact about a package, but it should encode the broad maintenance policy: core/product packages, integrations, capability seams, and support/test/example packages should not all require a hand-maintained exception list before scripts can tell them apart. -Two of the cataloged items need no generator at all: folding the e2e entry glob into knip's default stanza deletes the per-package restatements outright, and `childSessions` can be discovered from each scenario's fixture directory, leaving the scenario table to declare only policy (`recorded`, `hasModelTurn`). +Two of the cataloged items need no generator at all: folding the e2e entry glob into knip's default stanza deletes the per-package restatements outright, and `childSessions` can be discovered from each scenario's fixture directory, leaving the scenario table to declare only policy (`recorded`, `hasModelTurn`, `comparesLog`) — and even those track fixture-derivable facts today (`comparesLog` ⟺ the committed log has entries beyond its header line; `recorded` ⟺ `hasModelTurn` with no `replay.override.json` sibling), so each new scenario class keeps adding knobs the fixture directory already answers. ## Acceptance criteria diff --git a/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md index b0c4498c17..c7978183ef 100644 --- a/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md +++ b/docs/rfc/proposed/simplification/2026-06-20-unify-agent-and-session-id.md @@ -9,12 +9,13 @@ The agent factory carries TWO ids for what is, in every live consumer, one thing - `agentId` — the `AgentRegistry` handle (the actor identity; the registry rejects a duplicate). - `sessionId` — the event-sourced session / persisted-log identity (`session.header.id`). -`CreateAgentOptions` takes both separately; `ResumeAgentOptions` takes an `agentId` plus a `resumeSessionId`. They diverge in exactly two places: +`CreateAgentOptions` takes both separately; `ResumeAgentOptions` takes an `agentId` plus a `resumeSessionId`. They diverge in exactly three places: - **Config-driven create** (`AgentLoop.create`): a stable `agentId` (e.g. `"echo"`) with a fresh per-run `sessionId` (`${id}-session-`). - **Resume**: a caller-supplied `agentId` (e.g. `"main"`) on a persisted `resumeSessionId`. +- **In-process subagent children**: the backend mints the child's `agentId` and `sessionId` as two independent UUIDs (`packages/subagent/subagent-inprocess/src/index.ts`) that nothing distinguishes — `parentSession` records lineage independently. -Everywhere a live consumer actually looks an agent up — the **ACP bridge, the only production path** — the two are already unified: `agentId === sessionId === `. Concretely, both bridge factory call sites brand `AgentId(sessionId)` directly, and the bridge's reverse lookup keys on the `Agent` object itself — there is no id translation anywhere in the bridge to migrate. +Where a live consumer looks an agent up, no lookup needs an id translation: the ACP bridge — the primary production path — already unifies the two (`agentId === sessionId === `; both factory call sites brand `AgentId(sessionId)` directly, and its reverse lookup keys on the `Agent` object itself), and the CC hooks bridge resolves subagent children directly by the `agentId` its lifecycle event carries. The one production population whose two ids actually DIVERGE is the in-process subagent children — the same cosmetic separation as the config path, and the same one-field simplification under unification. One consumer already pays the two-id tax: ui-stdio keeps a `labelBySession` map (seeded from the registry, maintained by `agent/created`/`agent/disposed` listeners) solely to translate `session.header.id` back to an agent id for its turn labels — machinery that deletes outright when the ids unify. And the CC hooks bridge stamps `session_id: agent.session.header.id` into every hook payload, so under unification a subagent hook's `session_id` and `agent_id` become the same string — one less identity for a hook author to reconcile. The separation is **latent generality no consumer exercises**: nothing reads a *stable* `agentId` back across runs (each process starts fresh, and persistence keys off the session id, never the agent id). The config path's "stable agentId, fresh sessionId" buys nothing concrete — it is cosmetic. And the `agentId !== sessionId` case is precisely what opens the bash owner-token alias hole: the bash completion-notice routes by `session.header.id`, but the registry enforces uniqueness only on `agentId`, so a programmatic caller registering two agents with different agent ids but the SAME session id can mis-route a notice (see [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md) § Seam precondition). The current code documents this as a precondition rather than guaranteeing it. @@ -40,7 +41,7 @@ That was the review's first suggestion. It would couple the generic registry to ## Risks -This touches public factory interfaces (`CreateAgentOptions`, `ResumeAgentOptions`, `AgentFactory`) and the config-agent id scheme, so it is a deliberate cross-package change, not a local patch — it ships as its own PR (converged with Codex), stacked on the bash owner-token work that surfaced the precondition. +This touches public factory interfaces (`CreateAgentOptions`, `ResumeAgentOptions`, `AgentFactory`) and the config-agent id scheme, so it is a deliberate cross-package change, not a local patch — it ships as its own PR (converged with Codex); the bash owner-token precondition it closes is documented in [agent lifecycle and ownership seams](../../implemented/architecture/2026-06-18-agent-lifecycle-and-ownership-seams.md). The genuine risks of collapsing the two ids into one (the case AGAINST this proposal — to be weighed honestly before implementing): diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md index 789226032b..e9144cc3aa 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-image-content-block.md @@ -8,7 +8,7 @@ Status: proposed ## Proposal -Remove `ImageBlock`, its `ContentBlockMap` entry, and the explicit skip/estimate branches in the deepseek serializer, the pi-ai converter, the ACP codec's outbound mapping, and compact-basic — the default arms those switches already carry for plugin-added block types absorb the cases. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the deepseek README's image-skip row, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation and `[image]`-placeholder rows, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. +Remove `ImageBlock`, its `ContentBlockMap` entry, the explicit `image` estimate/placeholder arms in compact-basic, and the image-naming comments in the deepseek serializer's, pi-ai converter's, and ACP codec's default arms — those default arms already absorb the case the way they absorb any unknown block type. Update the vocabulary line in [architecture.md](../../../architecture.md), the block list in `packages/llm/llm/README.md`, the deepseek README's image-skip row, the pi-ai README's images-not-representable row, the compact-basic README's image-estimation and `[image]`-placeholder rows, the pastes in [core.md](../../../core-data-structures/core.md) and [llm-streaming.md](../../../core-data-structures/llm-streaming.md), and the type-equiv manifest; amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s block list and multimodal-home consequence per [implemented/AGENTS.md](../../implemented/AGENTS.md); drop or retarget the tests that construct image blocks to exercise the removed branches. The ACP codec's inbound rejection of image PROMPT content is unaffected — that guard is about protocol content a client can send regardless of our vocabulary, and it stays. ## Why not keep it? diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md b/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md index 5553645337..60375ecf8c 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-inert-request-knobs.md @@ -30,4 +30,4 @@ This RFC deliberately does NOT touch `temperature`, `stop`, or `maxTokens`: thos ## Risks -A hooks/config plugin arriving via the interception seams may want to set request fields — it will reach for `temperature`/`stop` (kept, working), not a field adapters reject. If chat-prefix completion or strict mode become product features, the re-add lands with the adapter/endpoint work, where the contract can say what actually happens rather than "everyone throws". +The shipped hook bridges set no request fields at all, and a request-mutating plugin (an `agent/request` waterfall listener) would reach for `temperature`/`stop` (kept, working), not a field adapters reject. If chat-prefix completion or strict mode become product features, the re-add lands with the adapter/endpoint work, where the contract can say what actually happens rather than "everyone throws". diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md index 80a9fa0a62..85121f3293 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md @@ -8,13 +8,13 @@ Three pieces of public spine surface share one defect class: their only possible 1. **`SurfaceManager.invalidate()`** (`packages/core/session/src/surface.ts`). Its documented trigger — "the log has been replaced wholesale (e.g. after Session seed)" — is structurally unreachable: seeding happens inside the `Session` constructor, `_surface` is created lazily on first access, and the log reference is never reassigned afterward, so no constructed `SurfaceManager` ever observes a wholesale replacement. Sole caller: its own unit test. A rollback primitive protecting a scenario the implementation cannot produce. 2. **The `runLoop`, `Inbox`, and `InboxMessage` exports** (`packages/core/agent-loop/src/index.ts`). `runLoop` has no importer outside the package — the only callers are the package's own internals (the agent constructs its loop with it), so the public re-export has zero consumers; `Inbox`/`InboxMessage` likewise reach outside code only through the package's own inbox spec (switchable to the source module). The exports contradict the package's own docs — the inbox module doc says the public surface is `Agent.send()`/`Agent.steer()` — and the [architecture dependency rule](../../../architecture.md): nothing programs against `dsh-agent-loop`; a replacement loop is a different bundle built on `dsh-agent`, not a consumer of this package's internals. `ReactLoopAgent` stays exported (cross-package tests construct it by package name). -3. **`ToolExecutionResult.callId`** (`packages/core/tools/src/index.ts`; the *input* `ToolExecution.callId` stays). Zero readers. The loop deliberately ignores it and documents it as a footgun — the correlation id must be the loop's own `call.id`, because a `tools/execute` waterfall listener returning a mismatched id would otherwise orphan the call↔result pairing — and a regression test exists solely to prove the field is ignored. So every waterfall short-circuiter must fabricate a field whose only power is to be a bug if trusted; the ACP bridge correlates via the session event's `data.callId`, never via the execution result. +3. **`ToolExecutionResult.callId`** (`packages/core/tools/src/index.ts`; the *input* `ToolExecution.callId` stays). Zero readers — and no listener can even construct a result: `tools/pre-execute`/`tools/post-execute` listeners return Decisions, the registry builds every result itself and always sets `callId` to the input `exec.callId`, and the post-execute dispatch snapshots the outcome before the waterfall precisely so a listener mutating the shared result reference cannot corrupt the id. The loop independently ignores `result.callId` in favor of its own `call.id`, and two regression tests exist solely to prove the field cannot matter (the loop's ignores-result-callId test and the registry's mutation guard). A field that is by construction a copy of its input, defended by snapshot machinery, and pinned by tests proving it is ignored is pure liability surface; the ACP bridge correlates via the session event's `data.callId`, never via the execution result. ## Proposal -Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites, and `toolErrorResult`, along with the loop's ignore-comment and the proves-ignored regression test — the hazard they guard disappears with the field. Update the `ToolExecutionResult` paste in [tools.md](../../../core-data-structures/tools.md) (and its `scripts/type-equiv.manifest.json` row) and the result-shape row in `packages/core/tools/README.md`; for the `invalidate()` removal, amend the [session-surface RFC](../../implemented/architecture/2026-06-18-session-surface.md)'s full-rebuild-after-wholesale-replacement sentence per [implemented/AGENTS.md](../../implemented/AGENTS.md). +Delete the method and its test; delete the three export lines and their `packages/core/agent-loop/README.md` rows, pointing the inbox spec at the source module; drop the result field from the type, the registry's construction sites (the deny result, the dispatch result, `toolErrorResult`, and the post-execute snapshot's `callId` leg), the loop's ignore-comment, the proves-ignored regression test, and the mutation guard's `callId` assertions — the hazard they all pin disappears with the field, while the result's `additionalContext` ferry (a consumed post-execute channel) stays untouched. Update the `ToolExecutionResult` paste in [tools.md](../../../core-data-structures/tools.md) (and its `scripts/type-equiv.manifest.json` row) and the result-shape row in `packages/core/tools/README.md`; for the `invalidate()` removal, amend the [session-surface RFC](../../implemented/architecture/2026-06-18-session-surface.md)'s full-rebuild-after-wholesale-replacement sentence per [implemented/AGENTS.md](../../implemented/AGENTS.md). -Sequencing: the in-flight surface-cache work (tool-pairing balance caching) neither uses nor touches `invalidate`, so that removal lands after or alongside it mechanically. The `callId` removal waits for the in-flight interception-seams work that splits `tools/execute` into pre/post phases and currently carries the field verbatim — the argument transfers unchanged (post-execute listeners receive the execution object alongside the result), so the removal targets whichever seam shape is on master when implemented. +Sequencing: the in-flight surface-cache work (tool-pairing balance caching) neither uses nor touches `invalidate`, so that removal lands after or alongside it mechanically. The execute pipeline is `tools/pre-execute` → dispatch → `tools/post-execute`, and post-execute listeners receive the execution object alongside the result — nothing needs the result's own id. ## Why not keep them? @@ -23,8 +23,8 @@ A future consumer that swaps a session's log in place would want a reset primiti ## Acceptance criteria - `invalidate()` and the result `callId` appear only in this RFC; `runLoop`/`Inbox`/`InboxMessage` remain package-internal only — no re-export from the package index and no outside-package importer; the agent-loop README lists only the consumed public surface; the inbox spec imports the source module. -- The tools/execute contract tests pass with the shrunk result type; no waterfall test fabricates a `callId` on a result. +- The pre-/post-execute pipeline contract tests pass with the shrunk result type; the mutation-guard and proves-ignored tests shed their `callId` legs with the hazard they pin. ## Risks -All three are compile-visible removals with no runtime behavior change on any shipped path. The `callId` change lands on whatever execute-seam shape is current, as noted under sequencing. +All three are compile-visible removals with no runtime behavior change on any shipped path. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md index b026c76349..aa0465eb3b 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md @@ -8,11 +8,11 @@ The merge-extensible vocabulary maps are designed to grow by declaration merging - **`CacheHint` and the three `cache?: CacheHint` fields** on `TextBlock`/`ToolResultBlock`/`ImageBlock` (`packages/llm/llm/src/types.ts`). Nothing constructs a block with `cache:` anywhere — src, tests, and doc pastes all come up empty — and neither adapter reads `.cache`: DeepSeek prompt caching is automatic, so the adapters map `prompt_cache_hit_tokens` OUT of responses without ever sending a hint IN. This is Anthropic-style `cache_control` surface with no provider that can honor it. - **`MessageSourceMap.agent`** (`{ kind: 'agent'; agentId: string }`, same file). Zero constructors, tests included. Its intended producer shipped without it: the subagent backends send the parent's prompt to the child with no `source`, so it logs as `{ kind: 'user' }`, and the generic envelope renderer interpolates `source.kind` without ever routing on it. The variant is pasted into [core.md](../../../core-data-structures/core.md). -- **`TurnTriggerMap.continuation`** (`packages/core/session/src/types.ts`). The loop structurally cannot emit it — continuation happens *within* a turn as further steps, never as a new turn — and it constructs only `message` and `injection` triggers. The only writers are two hand-built test fixtures that need an arbitrary non-message trigger (`packages/support/llm-replay/tests/llm-replay.spec.ts`, `packages/support/ui-stdio/tests/ui-stdio.spec.ts`); the only production trigger reader, the ACP bridge, filters on `kind === 'message'`. The variant is pasted into [session.md](../../../core-data-structures/session.md). +- **`TurnTriggerMap.continuation`** (`packages/core/session/src/types.ts`). The loop structurally cannot emit it — continuation happens *within* a turn as further steps, never as a new turn — and it constructs only `message` and `injection` triggers. The only writer is one hand-built test fixture that needs an arbitrary non-message trigger (`packages/support/llm-replay/tests/llm-replay.spec.ts`); the only production trigger reader, the ACP bridge, filters on `kind === 'message'`. The variant is pasted into [session.md](../../../core-data-structures/session.md). ## Proposal -Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the two test fixtures to `injection` triggers (any non-`message` trigger serves their purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change, and amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s consequence line naming cache hints as having a home, per [implemented/AGENTS.md](../../implemented/AGENTS.md). +Delete `CacheHint` with its three `cache?` fields, the `agent` message-source variant, and the `continuation` turn-trigger variant. Switch the llm-replay fixture to an `injection` trigger (any non-`message` trigger serves its purpose). Update the type-equiv pastes in [core.md](../../../core-data-structures/core.md) and [session.md](../../../core-data-structures/session.md) (and `scripts/type-equiv.manifest.json` where block identity shifts) in the same change, and amend the [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-content-block-vocabulary.md)'s consequence line naming cache hints as having a home, per [implemented/AGENTS.md](../../implemented/AGENTS.md). Each variant returns the day it gains a real producer, exactly as the maps are designed to grow: a caching feature re-adds `cache` together with the adapter that transmits it; subagent attribution re-adds `agent` together with the backend that stamps it and a consumer that routes on it; an auto-continue feature that genuinely starts new turns re-adds `continuation` with the plugin that emits it. @@ -24,8 +24,8 @@ The [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-con - `rg` for `CacheHint`, the `agent` message-source spelling, and the `continuation` trigger spelling returns only this RFC. - The core-data-structures pastes and the type-equiv manifest are in sync (`pnpm run doc-sync` green). -- The two fixtures assert the same replay behavior with `injection` triggers; the suite is green. +- The fixture asserts the same replay behavior with an `injection` trigger; the suite is green. ## Risks -None operational — nothing can construct these values today. The in-flight event-taxonomy work reworks the transient `agent/*` mirror events, not the durable vocabulary declarations, so there is no collision. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order. +None operational — nothing can construct these values today. The event-taxonomy rework that removed the transient `agent/*` mirrors left the durable vocabulary declarations untouched, and the vocabulary the loop and the hook bridges DID add — the `rejected` turn-end reason, the `prompt/blocked` session event, `hook/invoked`/`hook/result` — all arrived together with their producers: live demonstrations of the admission policy this RFC applies retroactively. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md index f5c8c424d3..7291020df7 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md @@ -21,7 +21,7 @@ This is the seam-vocabulary echo of [prune dead methods from the persistence sea ## Why not keep it? -The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/execute` veto, not schema filtering) — re-adding against a real implementing provider will pin a better contract than the current speculative one. +The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/pre-execute` deny in the child's context, not schema filtering) — that deny primitive now exists on the interception seams, so re-adding against a real implementing provider will pin a better contract than the current speculative one. ## Acceptance criteria @@ -30,4 +30,4 @@ The two-kinds-of-capability design is the seam RFC's headline, and re-adding `ou ## Risks -The in-flight hooks stack enriches subagent lifecycle event payloads (agent type, last assistant message) — adjacent files, no field overlap; coordinate landing order mechanically. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it. +The subagent lifecycle events carry `lastAssistantMessage` on the end payload (the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)) — that enrichment lives in the service module, not the seam vocabulary this RFC shrinks, and the same review that shipped it dropped an `agentType` sibling for lacking a consumer: the judgment this RFC extends. The CC hooks bridge, the first outside consumer of those lifecycle events, reads only the event payloads and touches none of the surface removed here; the observe-enrich RFC's deferred control-flow redesign names implementing `resume` as its own future work — exactly the re-add trigger this RFC's pattern anticipates. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it. diff --git a/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md b/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md new file mode 100644 index 0000000000..e8c04e6900 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md @@ -0,0 +1,28 @@ +# RFC: Remove the `agent/steering` mirror emit + +Status: proposed + +## Problem + +`agent/steering` is the last remaining transient mirror of a durable session event. The loop's steering drain appends the durable `steering/message { turn, content, source }` and, on the very next line, emits `agent/steering(agent, turn, content, source)` — the identical fact as a fire-and-forget event (`packages/core/agent-loop/src/loop.ts`, `drainSteering`). It has zero production listeners: the only subscriber anywhere is a loop regression test asserting the emit carries `source` — the same fact the durable event already records one line above. + +Both mirror-removal RFCs retained it while explicitly deferring the decision this RFC now makes. The [boundary-mirror removal](../../implemented/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) kept it as "a live control signal, not a boundary"; the [stream-chunk removal](../../implemented/simplification/2026-07-02-remove-stream-chunk-mirror.md) kept it as "a live control signal with no durable twin, retained (its fate is a separate future decision)". The second rationale does not survive the code: the durable twin is `steering/message`, appended immediately before the emit with the same payload. The mirrored-vs-live-only line the taxonomy actually draws puts it on the mirror side: `agent/queued` is genuinely live-only (it fires at enqueue time, before any durable event exists, and already carries a `steering: boolean` flag — cancelled queued work never enters the log), while `agent/steering` fires at the exact moment its durable twin lands, carrying nothing the log does not. + +Steering itself is busier than ever — the hook bridges' turn-continuation decisions inject their reasons through `inbox.steer()`, landing as durable `steering/message` events that the hook-matrix goldens pin — and every one of those consumers observes the durable event. Nothing observes the mirror. + +## Proposal + +Remove the `agent/steering` declaration from `packages/core/agent/src/types.ts` (and its mention in the live-events JSDoc list there), the emit in `drainSteering` (whose `ctx` parameter becomes unused and goes too), the row in `packages/core/agent/README.md`, and the emit line in the loop-pseudocode blocks (`packages/core/agent-loop/src/loop.ts` module doc and [architecture.md](../../../architecture.md)); run `pnpm run gen-cordis-catalog`. Retarget the one regression test at the durable `steering/message` event — the source-preservation fact it pins lives on the log. The implementing PR amends the two retaining RFCs' scope lines per [implemented/AGENTS.md](../../implemented/AGENTS.md): the boundary RFC's retained-list entry and the stream-chunk RFC's "no durable twin" clause. + +## Why not keep it? + +"It is a control signal, not a boundary" — but the taxonomy's operative distinction is mirrored-vs-live-only, not control-vs-boundary, and this event mirrors. A consumer that wants enqueue-time notification has `agent/queued` (with its steering flag); a consumer that wants drain-time notification is by definition asking for the moment `steering/message` is appended, which `session/event` delivers with the same payload plus durability. The rejected [retire-mid-turn-steering RFC](../../rejected/simplification/2026-06-20-retire-mid-turn-steering.md) defended the steering *capability* — `steer()`, the durable event, continuation forcing — all of which this removal keeps untouched. + +## Acceptance criteria + +- No `agent/steering` spelling outside this RFC and the two amended RFCs; the catalog is regenerated and fresh. +- The retargeted test pins source preservation on `steering/message`; the suite is green. + +## Risks + +None known: zero production listeners exist to migrate, and both live-notification needs (enqueue, drain) have surviving homes (`agent/queued`, `session/event`). diff --git a/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md b/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md new file mode 100644 index 0000000000..2636bd82e9 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md @@ -0,0 +1,29 @@ +# RFC: Tighten the hook-protocol contract — the `native` dialect, `suppressOutput`, and lib-owned `hook/result` semantics + +Status: proposed + +## Problem + +Three pieces of the freshly-landed `dsh-hook-protocol` contract miss the discipline the hooks stack itself applied elsewhere (its review dropped a `subagent/end` `agentType` field for lacking a consumer — the same test these fail): + +1. **`HookDialect`'s `'native'` variant** (`packages/hooks/hook-protocol/src/types.ts`) has zero producers — the bridges stamp `'claude'` and `'codex'`; the only `'native'` constructor anywhere is the lib's own unit test. The field's own JSDoc defines `dialect` as "the bridge that ran it", and native is not a bridge: the [interception-seams RFC](../../implemented/feature/2026-06-30-interception-seams.md) records that native hooks are not a package and that "a native plugin can already use the typed Decisions" without the durable hook log, and the flagship native-plugin worked example asserts exactly that (no `hook/*` events at all). +2. **`HookOutput.suppressOutput`** (same file) is parsed by the codec and discarded on every path: no bridge branch, no merge fold, no warn, no deferred-list row — uniquely among its parsed-but-unhonored siblings, each of which carries a stated deferral (`updatedInput` → a logged warn plus the [pre-tool-input-rewrite proposal](../feature/2026-06-30-pre-tool-input-rewrite.md); `systemMessage` → a logged warn plus a README deferred row; `continue`/`stopReason` → a `TODO(hook-continue-false)` anchor plus the `'stop'` decision record). Structurally there is nothing to suppress: hook stdout never enters any transcript (context flows only via `additionalContext`; the log records only `decision`/`stderrSummary`), so a hook author setting `suppressOutput: true` gets silent nothing with no warn. +3. **The `hook/result` semantics live in the bridges, twice, not in the lib that owns the event.** `summarize()` — the 500-character stderr truncation rule — is byte-identical in `packages/hooks/hooks-claude/src/index.ts` and `packages/hooks/hooks-codex/src/index.ts`, and so is the decision-string rule `output.decision ?? (output.continue === false ? 'stop' : 'pass')`; yet `dsh-hook-protocol` declares `hook/result`, documents `stderrSummary` as "truncated" without owning the truncation, and documents the decision values without owning the mapping. If one bridge drifts (a different cap, a different fallback), the shared durable event's semantics fork silently. + +## Proposal + +Narrow `HookDialect` to `'claude' | 'codex'` and fix its JSDoc; retarget the lib's one `'native'` test. Drop `suppressOutput` from `HookOutput`, the codec's parse lines, its codec-test assertions, and the parsed-superset lists in the lib README and [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) (amended per [implemented/AGENTS.md](../../implemented/AGENTS.md)). Move the `hook/result` semantics into the lib: `appendHookResult` (or a helper it exposes) derives `stderrSummary` and the decision string from the `HookOutput` + exit outcome, and both bridges delete their private copies. Rider: un-export `BLOCKING_EXIT_CODE` (zero importers; even the codec tests spell the literal `2`). + +## Why not keep them? + +The hook-protocol-lib RFC deliberately records "parses the full CC superset", and `'native'`/`suppressOutput` are days old — the strongest counterargument is that this re-litigates fresh decisions. But parsing a field whose value can never influence anything is not protocol faithfulness, it is a reader trap; and a dialect variant that the design's own thesis says will never be stamped is vocabulary without an interpreter — the exact bar the stack's own review enforced when it dropped `agentType`. Both return trivially with their first real producer (a transcript surface that has hook stdout to suppress; a native-provenance feature that logs hook events). On item 3, the lib RFC chose per-bridge explicitness over a parameterized engine — but that choice governed payload construction and Decision mapping; the semantics of the SHARED durable event are precisely the "primitives where duplication would actually be dangerous" that the same RFC assigns to the lib. + +## Acceptance criteria + +- `HookDialect` is two-valued; `rg "'native'"` in the hooks packages returns only this RFC's amended references. +- `suppressOutput` appears nowhere in source, tests, or parsed-field doc lists. +- One definition each of the truncation rule and the decision-string rule, in `dsh-hook-protocol`, exercised by both bridges' suites; the hook-matrix snapshot goldens are byte-identical. + +## Risks + +All three changes are invisible on the wire and in the goldens (`dialect` values emitted today are `claude`/`codex`; `suppressOutput` influences nothing; the folded semantics are the same rules). The cost is touching a week-old package — cheap now, per the pre-release stance, and cheaper than letting two copies of a durable event's semantics age apart. diff --git a/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md b/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md index 3e5fccb60a..a4bfbbf896 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md @@ -6,7 +6,7 @@ Status: proposed Two pieces of `dsh-acp` surface are unreachable from any shipped configuration: -1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model, systemPrompt }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — can set the knobs at all; they are settable solely by direct-mounting the bridge, which only a unit test does. Every snapshot golden pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carries a live `TODO(double-default)`: the literals exist twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home. +1. **`AcpConfig.agentName` / `agentVersion`** (`packages/ui/acp/src/index.ts`). The shipped app package hands the bridge only `{ model, systemPrompt }` (`packages/ui/acp-agent/src/index.ts`), so no leaf `cordis.yml` — the only production config surface — can set the knobs at all; they are settable solely by direct-mounting the bridge, which only a unit test does. Every snapshot golden — the hook-matrix scenarios included — pins the schema defaults (`deepseek-harness-acp` / `0.0.1`). The pair also carries a live `TODO(double-default)`: the literals exist twice (schema `.default(...)` plus `??` fallbacks), with the TODO asking to pick one home. 2. **The `toolKindFor` name heuristic** (same file) special-cases `bash*`/`read*`/`write`/`edit*` tool names in the generic-fallback path. Since the [render-intent union](../../implemented/architecture/2026-07-02-tool-render-intent-union.md), every first-party tool those arms match ships its own `presentCall` carrying its kind, and the presenter-less production tools (`subagent`, `subagent_fork`) fall through to `other` anyway. The arms are production-reachable only when a tool's `presentCall` THROWS (the containment fallback) — and the bridge's own module doc states the design rule the heuristic violates: "the bridge never special-cases tool names". ## Proposal diff --git a/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md b/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md new file mode 100644 index 0000000000..36c033cfb8 --- /dev/null +++ b/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md @@ -0,0 +1,26 @@ +# RFC: Single-source the acp-agent replay config + +Status: proposed + +## Problem + +`examples/acp-agent` ships two hand-maintained configs: `cordis.yml` (the live tree) and `cordis.snapshot.yml` (the keyless replay tree). Stripped of comments and blanks, their entire difference is ONE plugin entry — the eight-line `llm-deepseek` stanza (with its `!!js` env keys and model list) versus the two-line `llm-replay` stanza. Every other entry is byte-identical, including the multi-line system prompt and both hook-bridge stanzas. Every app-shape change must therefore be made twice, and the [hook-snapshot-matrix RFC](../../implemented/testing/2026-07-04-hook-snapshot-matrix.md) records paying exactly that tax: "hence the symmetric edit to both configs". + +Nothing gates the symmetry. If the copies drift, the snapshot tier silently exercises a different app than the one that ships — the ["green units, broken product" class of gap](../../../postmortem/0001-acp-default-export-drops-inject.md) the snapshot tier exists to close, reintroduced one level up, with reviewer vigilance as the only defense. + +## Proposal + +Make the replay tree derive from the live tree instead of mirroring it. Preferred endpoint: a single source — either `cordis.snapshot.yml` becomes a thin overlay that includes `cordis.yml` and swaps only the llm entry (if the vendored loader/include config supports entry-level override), or the acp-agent bin's existing `DSH_SNAPSHOT=replay` branch performs the one-entry swap on the parsed config and `cordis.snapshot.yml` is deleted. Fallback endpoint, if single-sourcing is judged too magical for a teaching example: keep both files and add a boring verify gate (in the `doc-sync`/`hygiene` family) asserting the two configs' entry sets are equal modulo the llm entry. The implementing PR picks after checking the loader's include/override capability, updates the recording docs, and amends the snapshot RFCs' facts per [implemented/AGENTS.md](../../implemented/AGENTS.md). + +## Why not keep the twin? + +An explicit replay file is transparently readable and teaches replay semantics — the strongest counterargument, and the reason the fallback keeps the file and adds only the gate. YAML surgery inside the published bin is real complexity in a shipping artifact, and an include-overlay depends on loader capability that may not exist. But the status quo — a 125-line hand-maintained near-copy of a 141-line file whose one meaningful difference is two lines, defended by nothing — is the one option with a silent failure mode, and it grows with every plugin the app gains (the hooks stack just added twin stanzas to both). + +## Acceptance criteria + +- Either one config file plus a mechanical llm-entry swap exercised by the snapshot suite itself, or two files plus a symmetry gate that fails CI on any non-llm divergence. +- All snapshot scenarios (hook matrix included) pass unchanged; `pnpm run test:snapshot:record` still boots the live tree. + +## Risks + +The include-overlay shape may be unsupported by the vendored loader — then the bin-side swap or the gate. `echo-agent`/`coding-agent` are unaffected (no snapshot twin). If the gate route is chosen, it is one more bespoke verify script — the cost the repo's gate-friendly policy explicitly accepts for encoding an invariant no human reliably remembers. From e6418398ff40d74891d4cfd61cb77ed751f97063 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 11:47:12 +0800 Subject: [PATCH 07/10] =?UTF-8?q?docs(rfc):=20address=20Codex=20round=20fi?= =?UTF-8?q?ndings=20=E2=80=94=20timeless=20framing=20+=20narrowed=20fs=20a?= =?UTF-8?q?cceptance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite the hook-protocol RFC's process-relative wording (freshly-landed / days-old / week-old) as timeless evidence anchored to the recorded RFCs, and sweep the same class from the steering, replay-config, subagent-vocabulary, and vocabulary RFCs. Narrow the fs RFC's acceptance criterion: replaceAll survives on the request spec and version on other outcome types by design — name the exact removed surfaces instead of claiming the spellings vanish. --- .../2026-07-04-prune-producerless-vocabulary-variants.md | 2 +- .../2026-07-04-prune-unimplemented-subagent-vocabulary.md | 4 ++-- .../2026-07-04-prune-write-only-fs-surface.md | 2 +- .../2026-07-04-remove-agent-steering-mirror.md | 2 +- .../2026-07-04-tighten-hook-protocol-contract.md | 6 +++--- .../testing/2026-07-04-single-source-acp-replay-config.md | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md index aa0465eb3b..d967e1d3b2 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md @@ -28,4 +28,4 @@ The [content-block vocabulary RFC](../../implemented/architecture/2026-06-11-con ## Risks -None operational — nothing can construct these values today. The event-taxonomy rework that removed the transient `agent/*` mirrors left the durable vocabulary declarations untouched, and the vocabulary the loop and the hook bridges DID add — the `rejected` turn-end reason, the `prompt/blocked` session event, `hook/invoked`/`hook/result` — all arrived together with their producers: live demonstrations of the admission policy this RFC applies retroactively. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order. +None operational — nothing can construct these values today. The mirror-event removals (recorded in [the boundary-mirror RFC](../../implemented/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) and [the stream-chunk RFC](../../implemented/simplification/2026-07-02-remove-stream-chunk-mirror.md)) touch only transient `agent/*` events, never the durable vocabulary, so there is no collision. Elsewhere in the vocabulary the admission policy already holds: `rejected`, `prompt/blocked`, and `hook/invoked`/`hook/result` each have live producers — this RFC extends the same bar to the three variants that lack one. If the [image-block RFC](2026-07-04-drop-image-content-block.md) ships first, one of the three `cache?` fields leaves with it; the two proposals are independent and compose in either order. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md index 7291020df7..10b8a45a38 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md @@ -21,7 +21,7 @@ This is the seam-vocabulary echo of [prune dead methods from the persistence sea ## Why not keep it? -The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/pre-execute` deny in the child's context, not schema filtering) — that deny primitive now exists on the interception seams, so re-adding against a real implementing provider will pin a better contract than the current speculative one. +The two-kinds-of-capability design is the seam RFC's headline, and re-adding `outputSchema` later touches several files. But the design survives with `depthLimit` as its live example and the RFCs as its record, and the seam RFC itself concedes the shipped `toolFilter` shape is wrong (real enforcement needs a `tools/pre-execute` deny in the child's context, not schema filtering) — that deny primitive exists on the interception seams, so re-adding against a real implementing provider will pin a better contract than the current speculative one. ## Acceptance criteria @@ -30,4 +30,4 @@ The two-kinds-of-capability design is the seam RFC's headline, and re-adding `ou ## Risks -The subagent lifecycle events carry `lastAssistantMessage` on the end payload (the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)) — that enrichment lives in the service module, not the seam vocabulary this RFC shrinks, and the same review that shipped it dropped an `agentType` sibling for lacking a consumer: the judgment this RFC extends. The CC hooks bridge, the first outside consumer of those lifecycle events, reads only the event payloads and touches none of the surface removed here; the observe-enrich RFC's deferred control-flow redesign names implementing `resume` as its own future work — exactly the re-add trigger this RFC's pattern anticipates. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it. +The subagent lifecycle events carry `lastAssistantMessage` on the end payload (the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)) — that enrichment lives in the service module, not the seam vocabulary this RFC shrinks, and the same RFC records dropping an `agentType` sibling for lacking a consumer: the judgment this RFC extends. The CC hooks bridge, the first outside consumer of those lifecycle events, reads only the event payloads and touches none of the surface removed here; the observe-enrich RFC's deferred control-flow redesign names implementing `resume` as its own future work — exactly the re-add trigger this RFC's pattern anticipates. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md b/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md index 91fa0ab739..0a4bc14d89 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-write-only-fs-surface.md @@ -21,7 +21,7 @@ A future permission/containment layer might want the pre-resolution path for err ## Acceptance criteria -- The removed spellings appear only in this RFC; doc pastes and the manifest in sync; the suite is green with the shrunk fakes. +- The removed surfaces are gone — `STREAM_MIN_SIZE`/`streamMinSize` in `dsh-fs-local`, `FsTarget.inputPath`, `FsEditOutcome.replacements`/`.replaceAll`, and `FileReadOutcome.limit`/`.version` — while the request-side `replaceAll` (`FsEditSpec`) and the version fields on the other outcome types are untouched; doc pastes and the manifest in sync; the suite is green with the shrunk fakes. - `formatEditOutput`'s emitted text is unchanged for both `replace_all` branches, so no snapshot golden churns. ## Risks diff --git a/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md b/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md index e8c04e6900..579401cb75 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md +++ b/docs/rfc/proposed/simplification/2026-07-04-remove-agent-steering-mirror.md @@ -8,7 +8,7 @@ Status: proposed Both mirror-removal RFCs retained it while explicitly deferring the decision this RFC now makes. The [boundary-mirror removal](../../implemented/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) kept it as "a live control signal, not a boundary"; the [stream-chunk removal](../../implemented/simplification/2026-07-02-remove-stream-chunk-mirror.md) kept it as "a live control signal with no durable twin, retained (its fate is a separate future decision)". The second rationale does not survive the code: the durable twin is `steering/message`, appended immediately before the emit with the same payload. The mirrored-vs-live-only line the taxonomy actually draws puts it on the mirror side: `agent/queued` is genuinely live-only (it fires at enqueue time, before any durable event exists, and already carries a `steering: boolean` flag — cancelled queued work never enters the log), while `agent/steering` fires at the exact moment its durable twin lands, carrying nothing the log does not. -Steering itself is busier than ever — the hook bridges' turn-continuation decisions inject their reasons through `inbox.steer()`, landing as durable `steering/message` events that the hook-matrix goldens pin — and every one of those consumers observes the durable event. Nothing observes the mirror. +Steering carries real production traffic — the hook bridges' turn-continuation decisions inject their reasons through `inbox.steer()`, landing as durable `steering/message` events that the hook-matrix goldens pin — and every one of those consumers observes the durable event. Nothing observes the mirror. ## Proposal diff --git a/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md b/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md index 2636bd82e9..4fe0813c9c 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md +++ b/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -Three pieces of the freshly-landed `dsh-hook-protocol` contract miss the discipline the hooks stack itself applied elsewhere (its review dropped a `subagent/end` `agentType` field for lacking a consumer — the same test these fail): +Three pieces of the `dsh-hook-protocol` contract miss the discipline the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md) records — it dropped an `agentType` lifecycle field for lacking a consumer, and these fail the same test: 1. **`HookDialect`'s `'native'` variant** (`packages/hooks/hook-protocol/src/types.ts`) has zero producers — the bridges stamp `'claude'` and `'codex'`; the only `'native'` constructor anywhere is the lib's own unit test. The field's own JSDoc defines `dialect` as "the bridge that ran it", and native is not a bridge: the [interception-seams RFC](../../implemented/feature/2026-06-30-interception-seams.md) records that native hooks are not a package and that "a native plugin can already use the typed Decisions" without the durable hook log, and the flagship native-plugin worked example asserts exactly that (no `hook/*` events at all). 2. **`HookOutput.suppressOutput`** (same file) is parsed by the codec and discarded on every path: no bridge branch, no merge fold, no warn, no deferred-list row — uniquely among its parsed-but-unhonored siblings, each of which carries a stated deferral (`updatedInput` → a logged warn plus the [pre-tool-input-rewrite proposal](../feature/2026-06-30-pre-tool-input-rewrite.md); `systemMessage` → a logged warn plus a README deferred row; `continue`/`stopReason` → a `TODO(hook-continue-false)` anchor plus the `'stop'` decision record). Structurally there is nothing to suppress: hook stdout never enters any transcript (context flows only via `additionalContext`; the log records only `decision`/`stderrSummary`), so a hook author setting `suppressOutput: true` gets silent nothing with no warn. @@ -16,7 +16,7 @@ Narrow `HookDialect` to `'claude' | 'codex'` and fix its JSDoc; retarget the lib ## Why not keep them? -The hook-protocol-lib RFC deliberately records "parses the full CC superset", and `'native'`/`suppressOutput` are days old — the strongest counterargument is that this re-litigates fresh decisions. But parsing a field whose value can never influence anything is not protocol faithfulness, it is a reader trap; and a dialect variant that the design's own thesis says will never be stamped is vocabulary without an interpreter — the exact bar the stack's own review enforced when it dropped `agentType`. Both return trivially with their first real producer (a transcript surface that has hook stdout to suppress; a native-provenance feature that logs hook events). On item 3, the lib RFC chose per-bridge explicitness over a parameterized engine — but that choice governed payload construction and Decision mapping; the semantics of the SHARED durable event are precisely the "primitives where duplication would actually be dangerous" that the same RFC assigns to the lib. +The [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) deliberately records "parses the full CC superset" — the strongest counterargument is that this proposal re-litigates decisions that RFC records. But parsing a field whose value can never influence anything is not protocol faithfulness, it is a reader trap; and a dialect variant that the design's own thesis says will never be stamped is vocabulary without an interpreter — the bar the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)'s `agentType` drop records. Both return trivially with their first real producer (a transcript surface that has hook stdout to suppress; a native-provenance feature that logs hook events). On item 3, the lib RFC chose per-bridge explicitness over a parameterized engine — but that choice governed payload construction and Decision mapping; the semantics of the SHARED durable event are precisely the "primitives where duplication would actually be dangerous" that the same RFC assigns to the lib. ## Acceptance criteria @@ -26,4 +26,4 @@ The hook-protocol-lib RFC deliberately records "parses the full CC superset", an ## Risks -All three changes are invisible on the wire and in the goldens (`dialect` values emitted today are `claude`/`codex`; `suppressOutput` influences nothing; the folded semantics are the same rules). The cost is touching a week-old package — cheap now, per the pre-release stance, and cheaper than letting two copies of a durable event's semantics age apart. +All three changes are invisible on the wire and in the goldens (`dialect` values emitted in practice are `claude`/`codex`; `suppressOutput` influences nothing; the folded semantics are the same rules). The cost is churn in `dsh-hook-protocol` and both bridges — cheap under the pre-release stance, and cheaper than letting two copies of a durable event's semantics age apart. diff --git a/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md b/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md index 36c033cfb8..3cb986a3e7 100644 --- a/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md +++ b/docs/rfc/proposed/testing/2026-07-04-single-source-acp-replay-config.md @@ -14,7 +14,7 @@ Make the replay tree derive from the live tree instead of mirroring it. Preferre ## Why not keep the twin? -An explicit replay file is transparently readable and teaches replay semantics — the strongest counterargument, and the reason the fallback keeps the file and adds only the gate. YAML surgery inside the published bin is real complexity in a shipping artifact, and an include-overlay depends on loader capability that may not exist. But the status quo — a 125-line hand-maintained near-copy of a 141-line file whose one meaningful difference is two lines, defended by nothing — is the one option with a silent failure mode, and it grows with every plugin the app gains (the hooks stack just added twin stanzas to both). +An explicit replay file is transparently readable and teaches replay semantics — the strongest counterargument, and the reason the fallback keeps the file and adds only the gate. YAML surgery inside the published bin is real complexity in a shipping artifact, and an include-overlay depends on loader capability that may not exist. But the status quo — a 125-line hand-maintained near-copy of a 141-line file whose one meaningful difference is two lines, defended by nothing — is the one option with a silent failure mode, and it grows with every plugin the app gains (the hook-bridge stanzas are twins in both files). ## Acceptance criteria From 09f131f78ee7108dc6def395c45c326bf089ac34 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 12:53:21 +0800 Subject: [PATCH 08/10] docs(rfc): fold PR #139's simplification sweep into this set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deduplicate the parallel sweep from codex/exhaustive-simplification-rfcs (merged in the parent commit) against the twelve RFCs already here, on the merits per item: Ported (rewritten to this set's evidence standard): - fold-stdio-ui-helper — verified: dsh-stdio-agent is the only runtime importer; the fold resolves the documented product-app-depends-on- support-package tension. The false acp-agent analogy is replaced with the real distinction (protocol product surface vs one app's front-door scaffolding). - generate-rfc-index-tables — verified: the classification RFC records rejecting generation; the index tables are the repo's only systematically conflicting docs region. Process framing made timeless. Consolidated into existing owners: - web searchStatus()/fetchStatus()/WebCapabilityStatus (verified: zero production callers; the tool-web README and architecture.md claims are drift) → drop-web-providers-change-event, renamed drop-unconsumed-web-observation-surface. - hook/result.durationMs (unread, nondeterministic, normalizer-scrubbed) and the double-defaulted defaultTimeoutMs knob → tighten-hook-protocol-contract. - the exercised-but-unadvertised exec.arguments mutation path (a tool-bash integration shim rewrites through it) → a sanction-or-seal note in the pre-tool-input-rewrite proposal. - the dormant-guard critique of subagent depth machinery → recorded in prune-unimplemented-subagent-vocabulary as the considered-and-rejected alternative, with the keep sharpened (uncapped-today acknowledged; wiring the cap is the completion, not deletion). getProvider()/list() and lastAssistantMessage recorded as examined-and-kept (bash-revert precedent; observe-enrich recorded keep). Not ported (with reasons): - tools/change + system-prompt/change removal — recorded keeps in the adapter-change RFC, unengaged by the sweep; no new facts. - LlmService.models() — flagged by both surveys, but two lines with a plausible consumer: TODO-or-drive-by territory per the RFC bar, not a proposal. - SchemaProp.default RFC — already XXX(unused-default)-tagged; the RFC bar excludes TODO-tracked provisional cleanups. - PreToolDecision 'ask' removal — FIXME(permissions)-anchored deferral with the permission system as its named consumer. - Codex bridge Config.model, merged systemMessages — wire-faithful tested surface / README-documented deferral. Their in-code TODO notes (acp-test-harness, hook-snapshot-noise, catalog-type-links) and the stale hook-prompt-block name fixes ride the merge unchanged. --- docs/rfc/README.md | 9 +-- .../2026-06-30-pre-tool-input-rewrite.md | 2 +- .../2026-07-04-generate-rfc-index-tables.md | 34 +++-------- ...drop-idle-registry-observation-surfaces.md | 55 ----------------- ...drop-unconsumed-web-observation-surface.md | 32 ++++++++++ ...6-07-04-drop-web-providers-change-event.md | 28 --------- .../2026-07-04-fold-stdio-ui-helper.md | 33 +++------- .../2026-07-04-narrow-pre-tool-gate.md | 45 -------------- ...-04-narrow-subagent-synchronous-collect.md | 61 ------------------- ...prune-unimplemented-subagent-vocabulary.md | 6 +- .../2026-07-04-remove-tool-schema-defaults.md | 42 ------------- ...26-07-04-tighten-hook-protocol-contract.md | 19 +++--- .../2026-07-04-trim-hook-protocol-surface.md | 46 -------------- 13 files changed, 68 insertions(+), 344 deletions(-) delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md create mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md delete mode 100644 docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 4d1f6aecdb..f427e03a4a 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -53,7 +53,7 @@ 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 | | [Prune producer-less vocabulary variants (block cache hints, the `agent` message source, the `continuation` turn trigger)](proposed/simplification/2026-07-04-prune-producerless-vocabulary-variants.md) | 2026-07-04 | | [Drop `GenerateOptions.prefill` and `ToolSchema.strict` — request knobs with no working end-to-end path](proposed/simplification/2026-07-04-drop-inert-request-knobs.md) | 2026-07-04 | -| [Drop the unconsumed `web/providers-change` event](proposed/simplification/2026-07-04-drop-web-providers-change-event.md) | 2026-07-04 | +| [Drop the unconsumed web observation surface — the `providers-change` event and the status methods](proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md) | 2026-07-04 | | [Drop the `image` content block until a path can honor it](proposed/simplification/2026-07-04-drop-image-content-block.md) | 2026-07-04 | | [Prune write-only fields and a dead routing knob from the fs seam](proposed/simplification/2026-07-04-prune-write-only-fs-surface.md) | 2026-07-04 | | [Prune the unimplemented subagent seam vocabulary](proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md) | 2026-07-04 | @@ -61,12 +61,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Prune dead core-spine surface — `SurfaceManager.invalidate()`, the loop-internal exports, `ToolExecutionResult.callId`](proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md) | 2026-07-04 | | [Share the app bins' boot glue instead of maintaining twin copies](proposed/simplification/2026-07-04-share-app-bin-boot-glue.md) | 2026-07-04 | | [Remove the `agent/steering` mirror emit](proposed/simplification/2026-07-04-remove-agent-steering-mirror.md) | 2026-07-04 | -| [Tighten the hook-protocol contract — the `native` dialect, `suppressOutput`, and lib-owned `hook/result` semantics](proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md) | 2026-07-04 | -| [Narrow the subagent seam to synchronous collect](proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md) | 2026-07-04 | -| [Drop idle registry and status observation surfaces](proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md) | 2026-07-04 | -| [Remove defaults from the tool-schema DSL](proposed/simplification/2026-07-04-remove-tool-schema-defaults.md) | 2026-07-04 | -| [Trim unused hook protocol and bridge surface](proposed/simplification/2026-07-04-trim-hook-protocol-surface.md) | 2026-07-04 | -| [Narrow the pre-tool gate to shipped behavior](proposed/simplification/2026-07-04-narrow-pre-tool-gate.md) | 2026-07-04 | +| [Tighten the hook-protocol contract — dialect, discarded fields, double defaults, and lib-owned `hook/result` semantics](proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md) | 2026-07-04 | | [Fold the stdio UI helper into the stdio app](proposed/simplification/2026-07-04-fold-stdio-ui-helper.md) | 2026-07-04 | ### Architecture diff --git a/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md b/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md index 4c91b0c4a4..3e731af4b9 100644 --- a/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md +++ b/docs/rfc/proposed/feature/2026-06-30-pre-tool-input-rewrite.md @@ -16,7 +16,7 @@ In the loop, a tool call's arguments are committed to the log and read by live c 2. **`tool/call`** is the durable AUDIT record, appended before `ctx.tools.execute()`. 3. **Live presentation reads `tool/call.arguments`**: the ACP bridge remembers them and passes them to `presentResult`; `dsh-tool-bash` derives the card title, the rawInput, the cwd, and the terminal-vs-background treatment from them. -So an "input rewrite" that changes ONLY what executes would make the UI show one command while another RAN, and render result state against the wrong arguments — a real inconsistency, not a documentable gap. (The existing low-level capability to mutate `exec.arguments` in a listener has exactly this latent inconsistency; it is unadvertised precisely because of this.) +So an "input rewrite" that changes ONLY what executes would make the UI show one command while another RAN, and render result state against the wrong arguments — a real inconsistency, not a documentable gap. (The existing low-level capability to mutate `exec.arguments` in a listener has exactly this latent inconsistency; it is unadvertised precisely because of this — yet not unused: a tool-bash integration test rewrites a scripted call's arguments through it (`packages/bash/tool-bash/tests/integration.spec.ts`), so this design must either sanction that path with the consistency unit below or seal it — `readonly` arguments at the seam, with the test shim moved onto a behavior-level helper.) ## Proposed design (sketch — to validate against the code when built) diff --git a/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md b/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md index f68d34f2eb..b3c88c47ad 100644 --- a/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md +++ b/docs/rfc/proposed/process/2026-07-04-generate-rfc-index-tables.md @@ -4,40 +4,24 @@ Status: proposed ## Problem -`docs/rfc/README.md` is hand-maintained even though the repo already has a machine-readable RFC layout: every RFC lives at `docs/rfc/{lifecycle}/{class}/yyyy-mm-dd-topic.md`, and `scripts/verify-rfc-classification.ts` walks that tree to verify structure and index completeness. The current gate prevents drift, but every new RFC still edits the same README tables by hand. +`docs/rfc/README.md`'s per-lifecycle/per-class tables are hand-maintained even though every fact in them is derivable: an RFC's path encodes lifecycle and class, its filename encodes the first-proposed date, and its H1 carries the title. `scripts/verify-rfc-classification.ts` already walks the tree and cross-checks the index — the expensive parsing exists; it reports instead of writing. -The stacked hook work made the cost visible. PR #138 added implemented feature/testing/process rows while this simplification sweep added proposed simplification rows, and the only merge conflict when retargeting the sweep onto #138 was the RFC index table. That is predictable: high-churn proposal waves all touch the same few lines even though the truth is already in filenames and H1 titles. - -[The classification RFC](../../implemented/process/2026-06-20-rfc-classification.md) explicitly rejected auto-generating the README index so the file could stay curated. That was a reasonable first cut, but the repo now has enough RFC volume and stacked-PR churn that the hand-written table is the unstable part, not the curated prose. The verifier already does the expensive parsing; it just reports instead of writing. +The tables are also the repo's highest-contention docs hotspot: every proposal wave appends rows to the same few lines, so concurrent RFC branches conflict precisely there while agreeing everywhere else, and each conflict is resolved by hand-merging rows whose content the filesystem already knows. [The classification RFC](../../implemented/process/2026-06-20-rfc-classification.md) records rejecting auto-generation to keep the file curated — but the curated part of the README is the prose, and the prose never conflicts; only the mechanical tables do. ## Proposal -Keep the curated prose in `docs/rfc/README.md`, but generate the per-lifecycle/per-class tables from the filesystem. +Keep the curated prose; generate the tables. Add a `gen-rfc-index` mode (a `--write` flag on `verify-rfc-classification.ts`, or a sibling script sharing its walker) that scans the RFC tree, reads each H1, derives the date from the filename, and rewrites the table rows under stable generated markers per `## {Lifecycle}` / `### {Class}` section; `verify-rfc-classification` asserts freshness — the `gen-cordis-catalog`/`verify-cordis-catalog` pattern. The class and lifecycle sets stay closed in the script. The implementing PR amends the classification RFC's rejected-alternatives record per [implemented/AGENTS.md](../../implemented/AGENTS.md), since this supersedes that recorded choice. -- Add a `gen-rfc-index` script (or extend `verify-rfc-classification.ts` with `--write`) that scans RFC files, reads each H1, derives the first-proposed date from the filename, and writes the table rows under stable generated markers for each `## {Lifecycle}` / `### {Class}` section. -- Keep the class set and lifecycle set closed in one script-owned source of truth. -- Make `verify-rfc-classification` check that the generated sections are fresh, analogous to `verify-cordis-catalog`. -- Preserve manually curated prose, classification descriptions, and "when to write one" guidance outside the generated table blocks. -- Update [the classification RFC](../../implemented/process/2026-06-20-rfc-classification.md) to say the earlier "verify, do not generate" choice was superseded after stacked-PR conflicts made the tradeoff worse. +## Why not keep the verifier-only model? -The generated output should stay boring Markdown: the same tables reviewers read today, just mechanically produced from the path + title source of truth. - -## Why not keep the current verifier-only model? - -The current model catches mistakes but still forces every proposal to edit a shared hotspot. A failed verifier is also more annoying than a generator for a purely mechanical row: the author has already named and placed the file correctly, then has to copy the same facts into the index. That is exactly the kind of hand-maintained inventory the repo already proposes removing elsewhere. - -This does not turn the whole README into a build artifact. The prose remains curated. Only the parts whose content is derivable from RFC files become generated. +It catches mistakes but still makes every proposal edit a shared hotspot, and a failed verifier is strictly more annoying than a generator for a purely mechanical row: the author has already named and placed the file; the index copy adds no information. This is the same hand-list-versus-derivation judgment the [package-inventory proposal](2026-06-20-discover-package-inventory.md) applies to tsconfig references and knip stanzas — applied to the one list that demonstrably conflicts. ## Acceptance criteria -- `pnpm run gen-rfc-index` (or the chosen command) rewrites only the generated RFC table regions. -- `pnpm run verify-rfc-classification` fails when those generated regions are stale and passes after regeneration. -- Adding, moving, or deleting an RFC requires editing the RFC file itself; the README rows are produced mechanically. -- The generated rows use each RFC's H1 title and filename date, and preserve the existing lifecycle/class grouping. -- `pnpm run doc-sync` passes after implementation. +- `pnpm run gen-rfc-index` (or the chosen spelling) rewrites only the generated table regions; `verify-rfc-classification` fails when they are stale and passes after regeneration. +- Adding, moving, or deleting an RFC requires editing only the RFC file itself; the rows are produced from path + H1 + filename date. +- The prose outside the generated markers is untouched by the generator; `pnpm run doc-sync` passes. ## Risks -- Generated regions inside a curated README can be jarring. Use explicit markers and keep the table output minimal so reviewers know what is owned by the script. -- Reading H1 titles makes malformed RFC headers a generator concern. That is useful pressure: a missing or nonstandard H1 should fail clearly. -- This supersedes an implemented process decision. The implementing PR must amend the old classification RFC so the historical record explains why the tradeoff changed. +Generated regions inside a curated file need explicit markers so ownership is obvious to reviewers. Reading H1s makes a malformed header a generator error — useful pressure, and it should fail clearly. This supersedes an implemented process decision; amending that RFC's record is part of the change, not optional. diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md b/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md deleted file mode 100644 index 142972f3c7..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-idle-registry-observation-surfaces.md +++ /dev/null @@ -1,55 +0,0 @@ -# RFC: Drop idle registry and status 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. - -The same "status without observer" pattern now shows up in the web seam. `ctx.web.searchStatus()` and `ctx.web.fetchStatus()` are documented as diagnostics for `dsh-tool-web`, but the current tools execute directly through `ctx.web.search()` and `ctx.web.fetch()` ([packages/web/tool-web/src/search.ts](../../../../packages/web/tool-web/src/search.ts), [packages/web/tool-web/src/fetch.ts](../../../../packages/web/tool-web/src/fetch.ts)). The execution path already resolves the selected provider at call time and throws a structured `WebError` (`WEB_PROVIDER_UNAVAILABLE`, `WEB_PROVIDER_AMBIGUOUS`, `WEB_PROVIDER_CONFIGURED_MISSING`, `WEB_PROVIDER_CONFIGURED_UNAVAILABLE`) when the capability cannot run. The status methods duplicate that selection logic for tests and stale docs, not for a live product surface. - -## 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. -- Delete `WebService.searchStatus()` / `fetchStatus()` and the `WebCapabilityStatus` contract if no other live type needs it. Web provider `status()` stays internal to provider resolution; callers observe availability by attempting `search()` / `fetch()` and handling `WebError`. - -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.search()` and `ctx.web.fetch()` stay because they are the model-facing web tools' execution path and they already carry the provider-selection error taxonomy. `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 or status queries. But none exists today, and the current event payloads/status shapes 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. -- `rg "searchStatus|fetchStatus|WebCapabilityStatus" packages/web docs --glob '!docs/rfc/**'` finds no remaining public web status surface, docs entry, generated-catalog entry, or tests except provider-private status concepts that still feed execution. -- 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 and status 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 execute or throw the expected `WebError`, and an adapter can stream for its model. -- Web tests lose a cheap status assertion. They should assert the behavior a real caller observes: successful `search()` / `fetch()` for a usable provider and structured `WebError` codes for unavailable, ambiguous, or misconfigured provider sets. -- A future UI may need observer hooks or status queries. That is fine; the hook/query should return with that UI, not ahead of it. diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md b/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md new file mode 100644 index 0000000000..30d7f565d1 --- /dev/null +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md @@ -0,0 +1,32 @@ +# RFC: Drop the unconsumed web observation surface — the `providers-change` event and the status methods + +Status: proposed + +## Problem + +`WebService` exposes an observation surface no production code observes: + +- **`web/providers-change`** (`packages/web/web/src/index.ts`) is declared and emitted on every provider registration and disposal, and each registration effect's rollback yield is ordered BEFORE the emit solely so a throwing change listener unwinds the registration. No listener exists outside the package's own two unit tests (one of which exists to pin that rollback ordering). +- **`searchStatus()` / `fetchStatus()` and the `WebCapabilityStatus` union** (same package) have zero production callers: `dsh-tool-web` executes directly through `ctx.web.search()`/`fetch()` and surfaces unavailability as the structured `WebError` codes the seam throws at execution time (`packages/web/tool-web/src/search.ts`, `packages/web/tool-web/src/fetch.ts`); the only status callers are provider unit tests. The prose in `packages/web/tool-web/README.md` and [architecture.md](../../../architecture.md) still claims the tool "reads only the aggregated `searchStatus()`/`fetchStatus()`" — drift that survives only because nothing checks prose against call sites. + +The seam's own design starves both surfaces of consumers: tool registration follows product ENABLEMENT, not provider availability (`packages/web/tool-web/src/index.ts`), and provider selection resolves at execution time, never cached — so there is no cache to invalidate, no registration set to recompute, and no caller that needs an availability probe distinct from executing and routing the structured error. HMR cleanup is carried by the effect disposers themselves. + +This mirrors [drop the unconsumed `llm/adapter-change` event](../../implemented/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md), which removed the same notification shape, the same rollback-before-emit machinery, and the same listener-throw test from `LlmService`. That RFC's keep/cut criterion — keep `tools/change` for its plausible user-facing tool-list consumer, cut the boot-time backend-registry signal — puts a web-provider registry squarely on the cut side; the status methods are the same judgment applied to a pull surface instead of a push one. + +## Proposal + +Delete the event declaration, both emits, and the rollback-before-emit ordering (the plain `ctx.effect` disposer keeps HMR cleanup). Delete `searchStatus()`/`fetchStatus()`/`WebCapabilityStatus` — the provider-private `status()` stays, since it feeds execution-time selection. Delete the two event tests and rewrite the status-based test assertions onto the behavior a real caller observes (a successful `search()`/`fetch()`, or the structured `WebError` codes for unavailable/ambiguous/misconfigured provider sets). Run `pnpm run gen-cordis-catalog`; update `packages/web/web/README.md`, `packages/web/tool-web/README.md` (the drifted reads-status sentence), [web.md](../../../core-data-structures/web.md), and the web paragraph in [architecture.md](../../../architecture.md). The implementing PR amends the [web capability seam RFC](../../implemented/architecture/2026-06-24-web-capability-seam.md)'s facts (it specifies the event and the status aggregation) per [implemented/AGENTS.md](../../implemented/AGENTS.md). + +## Why not keep it? + +The web seam RFC specified both deliberately — the event as a minimal HMR-visibility signal, the status methods as the tool's aggregated diagnostics — and a future provider-status panel is imaginable. But the same RFC's other choices starved them: derived-on-call selection and enablement-based registration leave no consumer that CAN need either, the shipped tool demonstrates the real pattern (execute and route the structured error), and the drifted README sentence shows the promised consumer never materialized. Per AGENTS.md "RFCs are proposals, not golden truth", these are the parts of that proposal the code has since shown to over-reach; a future observer reintroduces the smallest signal or query it actually consumes, shaped by that consumer. + +## Acceptance criteria + +- No `providers-change`, `searchStatus`, `fetchStatus`, or `WebCapabilityStatus` spelling outside RFC history; the catalog is regenerated and fresh (`verify-cordis-catalog` green). +- Registration/disposal HMR-safety tests prove cleanup through execution behavior rather than the removed surfaces. +- `packages/web/tool-web/README.md` and the architecture paragraph describe the execution-time error-routing contract the tool actually has. + +## Risks + +A future provider-picker UI or diagnostics panel wants change notifications or a status query — it re-adds the smallest surface it consumes; the identical judgment, and its reversal condition, is already recorded on the llm precedent. diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md b/docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md deleted file mode 100644 index ed40e54afe..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-web-providers-change-event.md +++ /dev/null @@ -1,28 +0,0 @@ -# RFC: Drop the unconsumed `web/providers-change` event - -Status: proposed - -## Problem - -`WebService` declares and emits `web/providers-change` (`packages/web/web/src/index.ts`) on every provider registration and disposal, and orders each registration effect's rollback yield BEFORE the emit solely so a throwing change listener unwinds the registration. No listener exists outside the package's own two unit tests (one of which exists to pin that rollback ordering). The remaining references are the generated catalog and README/doc prose. - -The seam's own design removed the natural consumer. `dsh-tool-web` registers tools by product ENABLEMENT, deliberately not by provider availability (`packages/web/tool-web/src/index.ts`), and `searchStatus()`/`fetchStatus()` are derived per call, never cached — so there is no cache to invalidate and no registration set to recompute when providers come and go. HMR cleanup is already carried by the effect disposers themselves. - -This is shape-for-shape the surface the repo already cut once: [drop the unconsumed `llm/adapter-change` event](../../implemented/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) removed the same notification, the same rollback-before-emit machinery, and the same listener-throw test from `LlmService`. That RFC's keep/cut criterion — keep `tools/change` for its plausible user-facing tool-list consumer, cut the boot-time backend-registry signal — puts a web-provider registry squarely on the cut side. - -## Proposal - -Delete the event declaration, both emits, and the rollback-before-emit ordering (the plain `ctx.effect` disposer keeps HMR cleanup); delete the two event tests; run `pnpm run gen-cordis-catalog` and commit the regenerated catalog; update `packages/web/web/README.md` and the [web.md](../../../core-data-structures/web.md) prose. The implementing PR amends the [web capability seam RFC](../../implemented/architecture/2026-06-24-web-capability-seam.md)'s facts (it specifies the event in its interface sketch and test list), per [implemented/AGENTS.md](../../implemented/AGENTS.md). - -## Why not keep it? - -The web seam RFC specified the event deliberately — days after the adapter-change removal — as a minimal HMR-visibility signal. But the same RFC also made every status read derived-on-call and tool registration availability-independent, which is precisely why no consumer can need the signal: the design's other choices starved this one. Per AGENTS.md "RFCs are proposals, not golden truth", the event is the part of that proposal the code has since shown to over-reach; validating it against the repo's own precedent yields the verdict the precedent already recorded. - -## Acceptance criteria - -- No `providers-change` spelling outside this RFC and the amended seam RFC; the catalog is regenerated and fresh (`verify-cordis-catalog` green). -- Registration/disposal HMR-safety tests still prove cleanup via `searchStatus()`/`fetchStatus()` derivation rather than via the event. - -## Risks - -A future provider-picker UI or diagnostics panel that wants live change notifications re-adds the event with that consumer — the identical judgment, and its reversal condition, is already recorded on the llm precedent. diff --git a/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md b/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md index 4ef1a264b0..4af1ae9397 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md +++ b/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md @@ -4,39 +4,24 @@ Status: proposed ## Problem -`@deepseek-ai/dsh-ui-stdio` lives under `packages/support/`, but its only runtime importer is the product app package `@deepseek-ai/dsh-stdio-agent` ([packages/ui/stdio-agent/src/index.ts](../../../../packages/ui/stdio-agent/src/index.ts)). Direct `createStdioChat()` uses are package-local tests and the production wrapper inside the same support package. The examples reach it by loading `dsh-stdio-agent`, not by composing the UI helper themselves. +`@deepseek-ai/dsh-ui-stdio` is a whole package whose only runtime importer is the app package `@deepseek-ai/dsh-stdio-agent` (`packages/ui/stdio-agent/src/index.ts`). The examples reach the readline UI by loading the app, never by composing the helper themselves; the only other repo references are doc comments in two example e2e module docs and the dependency-graph rows in `packages/README.md`. [The ui group README](../../../../packages/ui/README.md) records the placement rationale — the helper "exists chiefly for the examples and the coverage gate — `ui/` is reserved for surfaces shipped as product" — which leaves a standing tension: a shipped product app depends on a support package documented as NOT product surface. -That leaves an awkward package boundary. `support/` is documented as lower-compat dev/test/example infrastructure, and the `ui-stdio` README says it is a convenience REPL, not a product surface. But `dsh-stdio-agent` is a shipped app package whose front-door cluster always includes the readline UI, console logger, JSONL persistence, and a pre-created `main` agent. In practice the helper is not an independent swappable capability; it is an implementation detail of the stdio app. - -The boundary adds package metadata, workspace references, generated module-graph rows, README entries, publish lint surface, and a cross-group dependency from `packages/ui/stdio-agent` to `packages/support/ui-stdio`. It also creates a policy mismatch: a product UI app depends on a support package whose docs say it should not be treated as load-bearing product surface. +The boundary buys package metadata, workspace and tsconfig references, module-graph rows, README entries, and publint surface for a helper that is not independently swappable: the stdio app's front-door cluster always includes the readline UI, and nothing else can meaningfully consume it. ## Proposal -Fold the stdio UI helper into `@deepseek-ai/dsh-stdio-agent`. +Fold the helper into `@deepseek-ai/dsh-stdio-agent`: move `createStdioChat`, its `StdioRuntime` test seam, and its unit tests into `packages/ui/stdio-agent`; delete the `packages/support/ui-stdio` package with its manifest, references, module-graph rows, and README rows; update the doc comments that name the package (the two example e2e module docs, `packages/README.md`, the ui group README). Keep the runtime seam so EOF handling, rendering, disposal, and piped-vs-TTY behavior stay unit-covered without hijacking process globals; the keyless Loader-path smokes keep guarding the export shape end-to-end. -- Move the `createStdioChat` implementation, its `StdioRuntime` test seam, and its unit tests into `packages/ui/stdio-agent`. -- Delete the `packages/support/ui-stdio` package, package references, path aliases, dependency entries, module-graph rows, and support README row. -- Keep the testable runtime seam inside `dsh-stdio-agent` so EOF handling, rendering, disposal, and piped-vs-TTY behavior remain covered without hijacking process globals. -- Update docs that currently point at `../support/ui-stdio` to describe stdio rendering as part of the stdio app. +## Why not promote it to `ui/` instead? -After the fold, the stdio app owns its front door the same way `dsh-acp-agent` owns its ACP bridge cluster. The examples still load one app package; no leaf config has to learn a new plugin. - -## Why not promote it to `packages/ui/` instead? - -Promotion would fix the support/product mismatch but keep the extra package boundary. That would make sense if more than one product app composed `createStdioChat()` directly, or if the readline UI were a swappable UI integration in its own right. The current consumer audit says neither is true. The stdio app is the consumer and the owner. - -Re-extraction stays cheap while the repo is unreleased. If a second product app needs the same readline UI independently, split it back out then, with that consumer shaping the package contract. +Promotion would resolve the support-vs-product mismatch while keeping the boundary — the right call only if the readline UI were an independently swappable integration or had a second composer, and the consumer census says neither. The structured ACP bridge stays its own package because it is the product protocol surface with its own contract and snapshot tiers; the readline helper is scaffolding for one app's front door. Re-extraction stays cheap pre-release: if a second product app wants the readline UI, split it back out then, with that consumer shaping the package contract. ## Acceptance criteria -- `rg "@deepseek-ai/dsh-ui-stdio|support/ui-stdio|createStdioChat" packages examples docs scripts --glob '!docs/rfc/**' --glob '!**/lib/**'` finds no deleted package dependency or docs reference; `createStdioChat` remains only as an internal/tested helper under `packages/ui/stdio-agent` if the name survives. -- The stdio app still prints transcript events, handles stdin lines/EOF, renders todo updates, and disposes readline listeners under HMR. -- Echo/coding-agent keyless smoke tests still boot through the real Loader path and guard the named-export shape. -- Package manifests, tsconfig project references, generated module graph, and docs are updated. -- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, `pnpm run build`, and `pnpm run hygiene` pass after implementation. +- `packages/support/ui-stdio` no longer exists; the helper and its tests live in `packages/ui/stdio-agent`; no reference to the deleted package remains outside RFC history. +- The stdio app still renders transcript events, handles stdin lines and EOF, renders todo checklists, and disposes readline listeners under HMR; the echo/coding keyless smokes still boot through the real Loader path and guard the export shape. +- Manifests, tsconfig references, the generated module graph, and docs are updated; `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, `pnpm run build`, and `pnpm run hygiene` pass. ## Risks -- `dsh-ui-stdio` currently has focused tests with a small package-local setup. Moving them risks blurring app composition tests with UI rendering tests; keep the helper test seam and colocated unit tests to avoid that. -- A future standalone terminal UI may want the helper as a package. Reintroduce it when a second product consumer exists rather than keeping a boundary for hypothetical reuse. -- Docs that mention the stdio UI as a support example need careful wording so they still distinguish the non-product terminal demo from the ACP product surface. +A future standalone terminal UI may want the helper as a package again — reintroduce it with that second consumer rather than keeping the boundary for hypothetical reuse. Moving tests risks blurring app-composition tests with UI-rendering tests; keeping the runtime seam and the colocated unit tests avoids that. diff --git a/docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md b/docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md deleted file mode 100644 index ee17588235..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-narrow-pre-tool-gate.md +++ /dev/null @@ -1,45 +0,0 @@ -# RFC: Narrow the pre-tool gate to shipped behavior - -Status: proposed - -## Problem - -The `tools/pre-execute` seam advertises two pieces of deferred capability that are not actually supported end to end: interactive `ask` permission and pre-tool argument rewrite. - -`PreToolDecision` includes `{ kind: 'ask' }`, but `ToolRegistry.execute()` treats every non-`allow` decision as a denied tool result because no permission UI exists yet ([packages/core/tools/src/index.ts](../../../../packages/core/tools/src/index.ts)). The only production producer is `dsh-hooks-claude`, which maps Claude Code `permissionDecision: "ask"` into that variant; Codex has no allow/ask path. The durable hook log can still record that an external hook asked, but the canonical typed seam cannot do anything distinct with it. The public union therefore has a third branch whose runtime semantics are "deny with a different default string." - -The same seam also has an unadvertised argument-rewrite escape hatch. The docs correctly say input rewrite is not offered because `assistant/message`, `tool/call`, and live presentation all see the model's original arguments before execution; changing only `exec.arguments` would make the UI/audit/history disagree with what ran. Yet `ToolExecution.arguments` is mutable, and dispatch reads `exec.arguments` after `tools/pre-execute`, so a listener can rewrite it anyway. A test shim does exactly that to thread a generated bash task id ([packages/bash/tool-bash/tests/integration.spec.ts](../../../../packages/bash/tool-bash/tests/integration.spec.ts)). The proposed [pre-tool input rewrite RFC](../feature/2026-06-30-pre-tool-input-rewrite.md) exists because doing this consistently is a design unit, not a hidden mutation. - -Both shapes are honest feature deferrals, but the public seam currently encodes them as if they were ready. That makes bridge code, docs, generated catalogs, and tests explain behavior whose only shipped result is "deny" or "mutate at your own risk." - -## Proposal - -Make `tools/pre-execute` express the behavior it can actually provide today: allow or deny a pending tool call, without argument mutation. - -- Remove `{ kind: 'ask' }` from `PreToolDecision`. The Claude bridge should still parse and log hook `ask` decisions, but map them to `deny` at the typed seam with an approval-not-supported reason until a real permission prompt exists. -- Update docs, generated catalogs, hook bridge README tables, and tests so `tools/pre-execute` is an allow/deny gate, not an allow/deny/ask gate. -- Make `ToolExecution.arguments` immutable by contract. At minimum mark it `readonly` and stop relying on a listener-mutated `exec.arguments` for dispatch; if a defensive runtime copy/freeze is needed to make the contract true, add it at the `ToolRegistry.execute()` boundary. -- Rewrite the one test shim that mutates `exec.arguments` to use a behavior-level helper instead of the hidden rewrite path. - -When permission prompts or consistent input rewrite lands, reintroduce the smallest explicit decision shape those features need. `ask` belongs with a real user approval loop; argument rewrite belongs with the audit/history/presentation update described by the proposed rewrite RFC. - -## What we give up - -Claude `permissionDecision: "ask"` no longer has a distinct typed-decision branch inside `dsh-tools`. The bridge can still preserve the external fact in `hook/result.decision` and still deny the call conservatively. That matches current product behavior without requiring every native plugin to handle an unusable branch. - -Internal tests lose a convenient mutable-object trick. That is a good loss: public tests should not depend on an unadvertised inconsistency that production docs warn against. - -## Acceptance criteria - -- `PreToolDecision` contains only `allow` and `deny`. -- `dsh-hooks-claude` still records hook `ask` in hook provenance, but returns a `deny` decision to `tools/pre-execute`. -- `rg "kind: 'ask'|PreToolDecision.*ask|ask.*degrades" packages docs --glob '!docs/rfc/**'` finds no remaining public pre-tool ask contract outside historical RFC text. -- `ToolExecution.arguments` is no longer a writable rewrite path, and `rg "exec\\.arguments\\s*=" packages examples --glob '!docs/rfc/**' --glob '!**/lib/**'` finds no mutation. -- The proposed pre-tool input rewrite RFC remains the future home for a consistent rewrite design. -- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation. - -## Risks - -- A native plugin author may already have experimented with `ask`. The repo is unreleased, and the branch currently cannot prompt a user; collapsing it now avoids shipping a promise that cannot be honored. -- Making arguments immutable may reveal more test helpers that were relying on mutation. Those helpers should move closer to the behavior they actually need instead of preserving a public inconsistency. -- Future permission and rewrite work will add back surface area. That is fine; the new surface should land with the product workflow and consistency guarantees that make it real. diff --git a/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md b/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md deleted file mode 100644 index 6f68dceb59..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-narrow-subagent-synchronous-collect.md +++ /dev/null @@ -1,61 +0,0 @@ -# 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 #138 hook stack made one earlier simplification idea too broad: `subagent/start` and `subagent/end` are now live. `dsh-hooks-claude` listens to `subagent/start` to run a `SubagentStart` hook and inject any returned `additionalContext` into the live child, and listens to `subagent/end` to run `SubagentStop` ([packages/hooks/hooks-claude/src/index.ts](../../../../packages/hooks/hooks-claude/src/index.ts)). Those lifecycle emits should stay. What remains idle is the registry-observation surface around the provider map: `ctx.subagents.getProvider()` and `ctx.subagents.list()` still have declarations, docs, generated-catalog entries, and tests, but no production caller. - -The new hook stack also exposes an overreach inside the lifecycle payload. [The subagent observe-enrichment RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md) added `lastAssistantMessage` so a hooks bridge could forward the child output to a `SubagentStop` handler, but the current `SubagentStop` payload builder does not read it; it emits only `agent_id`, `agent_type`, and `stop_hook_active`. The field therefore buys a `structuredClone` branch, clone-failure containment, docs, and tests without changing any shipped hook behavior. If `SubagentStop` should carry the final child message, that should be implemented end to end; until then the payload should be honest. - -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 final-output lifecycle cloning even though the real model-facing behavior is "start a named child, await its final result, cancel or dispose it," plus observe-only lifecycle emits the Claude hook bridge actually consumes. - -## Proposal - -Make the subagent seam describe the behavior the harness actually uses today: synchronous collect plus the two live observe-only lifecycle emits. - -- 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)`. -- Keep `subagent/start` and `subagent/end`, but narrow their payloads to the fields the live bridge can use: `provider`, `id`, and on end `stopReason`. Remove `SubagentRunEndInfo.lastAssistantMessage`, the `structuredClone(result.output)` branch, and the clone-failure tests/docs. -- 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. The service still emits `subagent/start` / `subagent/end` around that run because the hook bridge now consumes them. - -## 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 final-output 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. - -The Claude bridge would no longer be able to forward a child final message to `SubagentStop` without a later payload change. That is also honest: the current bridge does not forward it now. If that behavior becomes product-owned, reintroduce the field with the bridge payload and snapshot/unit coverage that prove the hook sees it. - -## 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 "getProvider\\(|ctx\\.subagents\\.list\\(" packages examples docs --glob '!docs/rfc/**'` finds no production API surface. -- `rg "lastAssistantMessage" packages docs --glob '!docs/rfc/**'` finds no live contract, clone branch, test, or generated-catalog entry. -- `subagent/start` and `subagent/end` still exist, and `dsh-hooks-claude` still handles `SubagentStart` / `SubagentStop`. -- 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 richer lifecycle payloads. Keep the live emits now, but reintroduce extra fields only with that UI and a payload it actually consumes. -- 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. diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md index 10b8a45a38..a68cb015c2 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md +++ b/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md @@ -15,7 +15,9 @@ The only reason `dsh-subagent` depends on `dsh-tools` at all is `outputSchema`'s Remove `outputSchema`/`structured`, `toolFilter`, `sendMessage`, and `resume` from the seam; shrink `SubagentCapabilities` to `{ depthLimit }`; drop the two capability-assert rows, the all-false flags on the three providers, the mock's structured branch and its `capabilities`/`structured` config knobs, and the tests that exist to pin the removed surface (the two rejection rows, the spawn absence test, the mock structured specs). Drop the `dsh-tools` peer/dev dependency from `packages/subagent/subagent/package.json`. Update the [subagent.md](../../../core-data-structures/subagent.md) pastes and the type-equiv manifest, and the README rows in `packages/subagent/subagent`, `packages/subagent/subagent-spawn`, `packages/subagent/subagent-fork`, and `packages/support/subagent-mock`. The implementing PR amends the seam RFC's capability catalog per [implemented/AGENTS.md](../../implemented/AGENTS.md). -**Keep** `depthLimit`/`maxDepth` and the capability-check mechanism itself: the in-process backend genuinely enforces the cap (`SubagentDepthError` in `packages/subagent/subagent-inprocess/src/index.ts`), recursion is the seam RFC's named risk, and one live capability row keeps the two-tier design demonstrated rather than merely remembered. +**Keep** `depthLimit`/`maxDepth` and the capability-check mechanism itself — with eyes open about its current reach. The in-process backend genuinely enforces the cap (`SubagentDepthError` in `packages/subagent/subagent-inprocess/src/index.ts`), but no production request sets `maxDepth` (`tool-subagent` exposes no knob for it), so on the shipped tool path the guard is dormant and recursion is uncapped. The alternative — remove the depth machinery too, on the argument that a dormant guard reads like a safety property while providing none — was considered and rejected: recursion is the seam RFC's named risk, the enforcement is real working code rather than vocabulary awaiting an implementation, and the honest completion is wiring a default cap through `tool-subagent` (a few-line feature) rather than deleting the only existing guard. One live capability row also keeps the two-tier design demonstrated rather than merely remembered. + +Adjacent surface examined and deliberately left alone: `SubagentService.getProvider()`/`list()` have test-harness consumers only, but the [prune-dead-seam-methods implementation note](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md) records precisely this shape being removed from the bash executor and reverted — a test harness IS a consumer for a one-line accessor over an already-tracked map. `SubagentRunEndInfo.lastAssistantMessage` is a recorded keep (the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)'s review dropped `agentType` and kept it deliberately, as the only final-message channel for out-of-process children); its currently-unwired bridge forwarding is a gap to close or a consumer to document, not surface for this RFC to cut. This is the seam-vocabulary echo of [prune dead methods from the persistence seam](../../implemented/simplification/2026-06-20-prune-dead-seam-methods.md): members every implementation must declare for nobody — weaker even, since here zero implementations exist. @@ -30,4 +32,4 @@ The two-kinds-of-capability design is the seam RFC's headline, and re-adding `ou ## Risks -The subagent lifecycle events carry `lastAssistantMessage` on the end payload (the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)) — that enrichment lives in the service module, not the seam vocabulary this RFC shrinks, and the same RFC records dropping an `agentType` sibling for lacking a consumer: the judgment this RFC extends. The CC hooks bridge, the first outside consumer of those lifecycle events, reads only the event payloads and touches none of the surface removed here; the observe-enrich RFC's deferred control-flow redesign names implementing `resume` as its own future work — exactly the re-add trigger this RFC's pattern anticipates. Worth recording while here: nothing production sets `maxDepth` today (`tool-subagent` exposes no knob for it), so in-process recursion is uncapped — wiring the depth machinery this RFC keeps is a small feature gap, and an argument for keeping it, not for cutting it. +The subagent lifecycle events carry `lastAssistantMessage` on the end payload — that enrichment lives in the service module, not the seam vocabulary this RFC shrinks, and the observe-enrich RFC records dropping an `agentType` sibling for lacking a consumer: the judgment this RFC extends. The CC hooks bridge, the first outside consumer of those lifecycle events, reads only the event payloads and touches none of the surface removed here; the observe-enrich RFC's deferred control-flow redesign names implementing `resume` as its own future work — exactly the re-add trigger this RFC's pattern anticipates. diff --git a/docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md b/docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md deleted file mode 100644 index 4d2428a2aa..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-remove-tool-schema-defaults.md +++ /dev/null @@ -1,42 +0,0 @@ -# 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." diff --git a/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md b/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md index 4fe0813c9c..42d222c6be 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md +++ b/docs/rfc/proposed/simplification/2026-07-04-tighten-hook-protocol-contract.md @@ -1,29 +1,32 @@ -# RFC: Tighten the hook-protocol contract — the `native` dialect, `suppressOutput`, and lib-owned `hook/result` semantics +# RFC: Tighten the hook-protocol contract — dialect, discarded fields, double defaults, and lib-owned `hook/result` semantics Status: proposed ## Problem -Three pieces of the `dsh-hook-protocol` contract miss the discipline the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md) records — it dropped an `agentType` lifecycle field for lacking a consumer, and these fail the same test: +Five pieces of the `dsh-hook-protocol`/bridge contract miss the discipline the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md) records — it dropped an `agentType` lifecycle field for lacking a consumer, and these fail the same test: 1. **`HookDialect`'s `'native'` variant** (`packages/hooks/hook-protocol/src/types.ts`) has zero producers — the bridges stamp `'claude'` and `'codex'`; the only `'native'` constructor anywhere is the lib's own unit test. The field's own JSDoc defines `dialect` as "the bridge that ran it", and native is not a bridge: the [interception-seams RFC](../../implemented/feature/2026-06-30-interception-seams.md) records that native hooks are not a package and that "a native plugin can already use the typed Decisions" without the durable hook log, and the flagship native-plugin worked example asserts exactly that (no `hook/*` events at all). 2. **`HookOutput.suppressOutput`** (same file) is parsed by the codec and discarded on every path: no bridge branch, no merge fold, no warn, no deferred-list row — uniquely among its parsed-but-unhonored siblings, each of which carries a stated deferral (`updatedInput` → a logged warn plus the [pre-tool-input-rewrite proposal](../feature/2026-06-30-pre-tool-input-rewrite.md); `systemMessage` → a logged warn plus a README deferred row; `continue`/`stopReason` → a `TODO(hook-continue-false)` anchor plus the `'stop'` decision record). Structurally there is nothing to suppress: hook stdout never enters any transcript (context flows only via `additionalContext`; the log records only `decision`/`stderrSummary`), so a hook author setting `suppressOutput: true` gets silent nothing with no warn. -3. **The `hook/result` semantics live in the bridges, twice, not in the lib that owns the event.** `summarize()` — the 500-character stderr truncation rule — is byte-identical in `packages/hooks/hooks-claude/src/index.ts` and `packages/hooks/hooks-codex/src/index.ts`, and so is the decision-string rule `output.decision ?? (output.continue === false ? 'stop' : 'pass')`; yet `dsh-hook-protocol` declares `hook/result`, documents `stderrSummary` as "truncated" without owning the truncation, and documents the decision values without owning the mapping. If one bridge drifts (a different cap, a different fallback), the shared durable event's semantics fork silently. +3. **`hook/result.durationMs`** is durable timing telemetry with no reader. Both bridges write it, and the ACP snapshot normalizer scrubs it to `0` because wall-clock hook runtime is replay noise (`examples/acp-agent/tests/snapshot-normalize.ts`); the remaining consumers are tests and the goldens that exist because the field exists. Deterministic provenance fields (`point`, `matcher`, `turn`, `handlerId`) earn their durability as audit facts; a nondeterministic field that replay must erase and nothing reads earns neither its bytes nor its special-case scrub. +4. **`defaultTimeoutMs` is double-defaulted in both bridge configs** — a schema `.default(600_000)` AND a `?? 600_000` fallback (`packages/hooks/hooks-claude/src/index.ts`, `packages/hooks/hooks-codex/src/index.ts`) — the same two-homes-for-one-literal shape the ACP bridge's `TODO(double-default)` flags, for a knob no shipped config sets; the per-hook `timeoutSec` is the real timeout surface. +5. **The `hook/result` semantics live in the bridges, twice, not in the lib that owns the event.** `summarize()` — the 500-character stderr truncation rule — is byte-identical in `packages/hooks/hooks-claude/src/index.ts` and `packages/hooks/hooks-codex/src/index.ts`, and so is the decision-string rule `output.decision ?? (output.continue === false ? 'stop' : 'pass')`; yet `dsh-hook-protocol` declares `hook/result`, documents `stderrSummary` as "truncated" without owning the truncation, and documents the decision values without owning the mapping. If one bridge drifts (a different cap, a different fallback), the shared durable event's semantics fork silently. ## Proposal -Narrow `HookDialect` to `'claude' | 'codex'` and fix its JSDoc; retarget the lib's one `'native'` test. Drop `suppressOutput` from `HookOutput`, the codec's parse lines, its codec-test assertions, and the parsed-superset lists in the lib README and [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) (amended per [implemented/AGENTS.md](../../implemented/AGENTS.md)). Move the `hook/result` semantics into the lib: `appendHookResult` (or a helper it exposes) derives `stderrSummary` and the decision string from the `HookOutput` + exit outcome, and both bridges delete their private copies. Rider: un-export `BLOCKING_EXIT_CODE` (zero importers; even the codec tests spell the literal `2`). +Narrow `HookDialect` to `'claude' | 'codex'` and fix its JSDoc; retarget the lib's one `'native'` test. Drop `suppressOutput` from `HookOutput`, the codec's parse lines, its codec-test assertions, and the parsed-superset lists in the lib README and [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) (amended per [implemented/AGENTS.md](../../implemented/AGENTS.md)). Drop `durationMs` from `HookResultRecord`, `RunHookResult`, the `hook/result` event, the bridge appends, the docs/catalog, and the snapshot normalizer's special-case scrub (retiring `runHook`'s injected clock if nothing else needs it); the hook goldens refresh mechanically as the scrubbed field disappears. Replace the bridges' `defaultTimeoutMs` config knob with one shared reference-default constant in `dsh-hook-protocol` (per-hook `timeoutSec` stays the override surface). Move the `hook/result` semantics into the lib: `appendHookResult` (or a helper it exposes) derives `stderrSummary` and the decision string from the `HookOutput` + exit outcome, and both bridges delete their private copies. Rider: un-export `BLOCKING_EXIT_CODE` (zero importers; even the codec tests spell the literal `2`). ## Why not keep them? -The [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) deliberately records "parses the full CC superset" — the strongest counterargument is that this proposal re-litigates decisions that RFC records. But parsing a field whose value can never influence anything is not protocol faithfulness, it is a reader trap; and a dialect variant that the design's own thesis says will never be stamped is vocabulary without an interpreter — the bar the [subagent-observe-enrich RFC](../../implemented/feature/2026-06-30-subagent-observe-enrich.md)'s `agentType` drop records. Both return trivially with their first real producer (a transcript surface that has hook stdout to suppress; a native-provenance feature that logs hook events). On item 3, the lib RFC chose per-bridge explicitness over a parameterized engine — but that choice governed payload construction and Decision mapping; the semantics of the SHARED durable event are precisely the "primitives where duplication would actually be dangerous" that the same RFC assigns to the lib. +The [hook-protocol-lib RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) deliberately records "parses the full CC superset" — the strongest counterargument is that this proposal re-litigates decisions that RFC records. But parsing a field whose value can never influence anything is not protocol faithfulness, it is a reader trap; a dialect variant that the design's own thesis says will never be stamped is vocabulary without an interpreter; and durable telemetry that replay must scrub is a cost with no buyer. Each returns trivially with its first real consumer (a transcript surface with hook stdout to suppress; a native-provenance feature that logs hook events; a trace viewer that reads timings — as live diagnostics or a deliberately durable telemetry event designed for it). On item 5, the lib RFC chose per-bridge explicitness over a parameterized engine — but that choice governed payload construction and Decision mapping; the semantics of the SHARED durable event are precisely the "primitives where duplication would actually be dangerous" that the same RFC assigns to the lib. ## Acceptance criteria - `HookDialect` is two-valued; `rg "'native'"` in the hooks packages returns only this RFC's amended references. -- `suppressOutput` appears nowhere in source, tests, or parsed-field doc lists. -- One definition each of the truncation rule and the decision-string rule, in `dsh-hook-protocol`, exercised by both bridges' suites; the hook-matrix snapshot goldens are byte-identical. +- `suppressOutput` and `durationMs` appear nowhere in source, parsed-field doc lists, the catalog, or the normalizer; the hook goldens are re-recorded or refreshed without the field. +- Both bridge configs lose `defaultTimeoutMs`; the reference default lives once, in the lib; per-hook `timeoutSec` still overrides it. +- One definition each of the truncation rule and the decision-string rule, in `dsh-hook-protocol`, exercised by both bridges' suites. ## Risks -All three changes are invisible on the wire and in the goldens (`dialect` values emitted in practice are `claude`/`codex`; `suppressOutput` influences nothing; the folded semantics are the same rules). The cost is churn in `dsh-hook-protocol` and both bridges — cheap under the pre-release stance, and cheaper than letting two copies of a durable event's semantics age apart. +The `dialect`, `suppressOutput`, `defaultTimeoutMs`, and semantics changes are invisible on the wire and in the goldens; the `durationMs` removal churns the hook goldens once (a mechanical refresh — the field was already normalized to a constant). The cost is churn in `dsh-hook-protocol` and both bridges — cheap under the pre-release stance, and cheaper than letting two copies of a durable event's semantics age apart. diff --git a/docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md b/docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md deleted file mode 100644 index d7de6d5d66..0000000000 --- a/docs/rfc/proposed/simplification/2026-07-04-trim-hook-protocol-surface.md +++ /dev/null @@ -1,46 +0,0 @@ -# RFC: Trim unused hook protocol and bridge surface - -Status: proposed - -## Problem - -The #138 hook stack added a useful bridge layer, but the current public protocol still exposes a few fields and knobs that no shipped writer or reader uses. They are small individually; together they widen the durable hook log, the shared hook-protocol API, and both bridge configs. - -`HookDialect` includes `'native'`, but real `hook/invoked` writers are the Claude and Codex bridges only. The worked native-plugin test explicitly proves the opposite: a native plugin uses typed Cordis decisions and emits no `hook/*` session events. Grepping `dialect: 'native'` finds a hook-protocol unit test, docs, and type text, not production code. - -`hook/result.durationMs` is durable timing telemetry with no production reader. Both bridges write it; the ACP snapshot normalizer immediately scrubs it to `0` because wall-clock hook runtime is replay noise ([examples/acp-agent/tests/snapshot-normalize.ts](../../../../examples/acp-agent/tests/snapshot-normalize.ts)). The only remaining consumers are tests and generated goldens that exist because the field exists. Persisting a value that replay must erase is a smell: it is neither product behavior nor useful audit state. - -`MergedHookOutcome.systemMessages` is also unused. The codec should still parse `HookOutput.systemMessage` because the external protocols can emit it and both bridges warn when it appears, but the merged aggregate is never read; `rg "systemMessages|\\.systemMessages"` finds the merge helper, README prose, and merge tests only. The bridge already handles warnings per raw output before merge. - -Finally, both bridge configs carry optional process-level defaults that shipped configs do not set. `defaultTimeoutMs` duplicates the reference default (`600_000`) even though each command hook already has its own `timeout`; tests mostly cover schema-bypass fallback. `dsh-hooks-codex` also exposes `Config.model`, but the ACP configs load the Codex bridge with only `configPath`, and every hook payload already has an `Agent` whose `options.model` is the actual model for that run. - -## Proposal - -Remove the unused protocol and config surface while keeping the live external-hook behavior: - -- Change `HookDialect` to `'claude' | 'codex'` until a real native `hook/*` producer exists. Native plugins keep using the typed interception seams directly. -- Remove `durationMs` from the `hook/result` session event, `HookResultRecord`, `RunHookResult`, bridge append calls, docs, generated catalog, snapshots, and the snapshot normalizer's special-case scrub. Remove the injected `now` clock from `runHook()` if it becomes unnecessary after the field disappears. -- Remove `MergedHookOutcome.systemMessages` and its tests/docs. Keep `HookOutput.systemMessage` parsing and the bridge warnings. -- Remove `defaultTimeoutMs` from both bridge configs. Keep per-command `timeoutSec`; when absent, `runHook()` uses a single shared protocol constant for the reference default. -- Remove `dsh-hooks-codex` `Config.model`; stamp Codex payloads from `agent.options.model ?? ''` at the point that has an agent, with `''` only for no-agent fallback paths. - -## What stays - -This RFC does not remove `hook/invoked` / `hook/result` themselves. They are live provenance: bridges append them around actual hook execution and ACP snapshots persist them. It also does not remove parsing/warning for `updatedInput`, `systemMessage`, `continue:false`, or `suppressOutput`; those are deliberate faithful-but-degraded external-protocol fields documented by [the hook bridge RFC](../../implemented/feature/2026-06-30-hook-bridges.md). - -This RFC does not collapse the shared `dsh-hook-protocol` package into the bridges or build a single parameterized bridge engine. [The protocol-library RFC](../../implemented/feature/2026-06-30-hook-protocol-lib.md) explicitly keeps only the identical wire primitives shared and leaves per-dialect payload/config mapping in each bridge. - -## Acceptance criteria - -- `rg "HookDialect.*native|dialect: 'native'|claude.*/.*codex.*/.*native|claude.*codex.*native" packages/hooks docs/core-data-structures/session.md docs/cordis-catalog/events-and-services.md --glob '!docs/rfc/**'` finds no `HookDialect` branch, test writer, or `hook/*` docs claiming a native durable writer. -- `rg "durationMs" packages/hooks examples/acp-agent/tests docs/core-data-structures/session.md docs/cordis-catalog/events-and-services.md --glob '!docs/rfc/**'` finds no hook-result field, snapshot scrub, or generated-golden requirement outside unrelated timing concepts. -- `rg "systemMessages|\\.systemMessages" packages/hooks docs --glob '!docs/rfc/**'` finds no merged aggregate surface, while `systemMessage` parsing and bridge warnings remain covered. -- `rg "defaultTimeoutMs|Config\\.model|model\\?: string" packages/hooks docs --glob '!docs/rfc/**'` finds no bridge config knob for the removed defaults, while per-hook timeout support and Codex payload model stamping still work. -- Hook bridge unit tests and ACP hook snapshots still prove prompt-submit, pre-tool, post-tool, and stop behavior for both dialects. -- `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, and `pnpm run hygiene` pass after implementation. - -## Risks - -- Durable hook timing can be useful diagnostics. If a product UI or trace viewer wants it, add live diagnostics or an intentionally durable telemetry event then; do not keep replay-noisy timing in the base hook-result record without a reader. -- A future native hook provenance logger might want `dialect: 'native'`. Add it with that logger. Until then, documenting native hooks as `hook/*` writers blurs the important design point that native plugins do not need the shell-hook log. -- A deployment could want a process-level Codex model override for hook payloads. The agent already knows its actual model, which is less surprising than a bridge-level default that can drift from the run being observed. From 3395d463fdc0bb4475308f1097f8bb39586e20ef Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 13:13:03 +0800 Subject: [PATCH 09/10] =?UTF-8?q?docs(rfc):=20fix=20fold-round=20review=20?= =?UTF-8?q?findings=20=E2=80=94=20reference=20census=20scope,=20test-calle?= =?UTF-8?q?r=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fold-stdio RFC claimed the two e2e doc comments and packages/README were the only non-runtime references; the package name also appears in the generated module graph, sibling READMEs, and tsconfig references — rescope the sentence to the runtime-importer census (the load-bearing claim) and fold the mechanical references into the update list. The web RFC's status-caller census now says 'the web packages' own tests' (the seam's tests use the methods too, not only provider tests). --- .../2026-07-04-drop-unconsumed-web-observation-surface.md | 2 +- .../simplification/2026-07-04-fold-stdio-ui-helper.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md b/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md index 30d7f565d1..e3516d974f 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md +++ b/docs/rfc/proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md @@ -7,7 +7,7 @@ Status: proposed `WebService` exposes an observation surface no production code observes: - **`web/providers-change`** (`packages/web/web/src/index.ts`) is declared and emitted on every provider registration and disposal, and each registration effect's rollback yield is ordered BEFORE the emit solely so a throwing change listener unwinds the registration. No listener exists outside the package's own two unit tests (one of which exists to pin that rollback ordering). -- **`searchStatus()` / `fetchStatus()` and the `WebCapabilityStatus` union** (same package) have zero production callers: `dsh-tool-web` executes directly through `ctx.web.search()`/`fetch()` and surfaces unavailability as the structured `WebError` codes the seam throws at execution time (`packages/web/tool-web/src/search.ts`, `packages/web/tool-web/src/fetch.ts`); the only status callers are provider unit tests. The prose in `packages/web/tool-web/README.md` and [architecture.md](../../../architecture.md) still claims the tool "reads only the aggregated `searchStatus()`/`fetchStatus()`" — drift that survives only because nothing checks prose against call sites. +- **`searchStatus()` / `fetchStatus()` and the `WebCapabilityStatus` union** (same package) have zero production callers: `dsh-tool-web` executes directly through `ctx.web.search()`/`fetch()` and surfaces unavailability as the structured `WebError` codes the seam throws at execution time (`packages/web/tool-web/src/search.ts`, `packages/web/tool-web/src/fetch.ts`); the only status callers are the web packages' own tests. The prose in `packages/web/tool-web/README.md` and [architecture.md](../../../architecture.md) still claims the tool "reads only the aggregated `searchStatus()`/`fetchStatus()`" — drift that survives only because nothing checks prose against call sites. The seam's own design starves both surfaces of consumers: tool registration follows product ENABLEMENT, not provider availability (`packages/web/tool-web/src/index.ts`), and provider selection resolves at execution time, never cached — so there is no cache to invalidate, no registration set to recompute, and no caller that needs an availability probe distinct from executing and routing the structured error. HMR cleanup is carried by the effect disposers themselves. diff --git a/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md b/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md index 4af1ae9397..aba2faf4ee 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md +++ b/docs/rfc/proposed/simplification/2026-07-04-fold-stdio-ui-helper.md @@ -4,13 +4,13 @@ Status: proposed ## Problem -`@deepseek-ai/dsh-ui-stdio` is a whole package whose only runtime importer is the app package `@deepseek-ai/dsh-stdio-agent` (`packages/ui/stdio-agent/src/index.ts`). The examples reach the readline UI by loading the app, never by composing the helper themselves; the only other repo references are doc comments in two example e2e module docs and the dependency-graph rows in `packages/README.md`. [The ui group README](../../../../packages/ui/README.md) records the placement rationale — the helper "exists chiefly for the examples and the coverage gate — `ui/` is reserved for surfaces shipped as product" — which leaves a standing tension: a shipped product app depends on a support package documented as NOT product surface. +`@deepseek-ai/dsh-ui-stdio` is a whole package whose only runtime importer is the app package `@deepseek-ai/dsh-stdio-agent` (`packages/ui/stdio-agent/src/index.ts`). The examples reach the readline UI by loading the app, never by composing the helper themselves; every other repo reference is mechanical or descriptive surface that exists BECAUSE the package boundary exists — manifest and tsconfig entries, generated module-graph rows, dependency-graph and README rows, and doc comments naming the package. [The ui group README](../../../../packages/ui/README.md) records the placement rationale — the helper "exists chiefly for the examples and the coverage gate — `ui/` is reserved for surfaces shipped as product" — which leaves a standing tension: a shipped product app depends on a support package documented as NOT product surface. The boundary buys package metadata, workspace and tsconfig references, module-graph rows, README entries, and publint surface for a helper that is not independently swappable: the stdio app's front-door cluster always includes the readline UI, and nothing else can meaningfully consume it. ## Proposal -Fold the helper into `@deepseek-ai/dsh-stdio-agent`: move `createStdioChat`, its `StdioRuntime` test seam, and its unit tests into `packages/ui/stdio-agent`; delete the `packages/support/ui-stdio` package with its manifest, references, module-graph rows, and README rows; update the doc comments that name the package (the two example e2e module docs, `packages/README.md`, the ui group README). Keep the runtime seam so EOF handling, rendering, disposal, and piped-vs-TTY behavior stay unit-covered without hijacking process globals; the keyless Loader-path smokes keep guarding the export shape end-to-end. +Fold the helper into `@deepseek-ai/dsh-stdio-agent`: move `createStdioChat`, its `StdioRuntime` test seam, and its unit tests into `packages/ui/stdio-agent`; delete the `packages/support/ui-stdio` package with its manifest, references, module-graph rows, and README rows; update every reference that names the package (the example e2e module docs, `packages/README.md`, the support and todo README rows, the stdio-agent README, the ui group README, tsconfig references, the generated module graph). Keep the runtime seam so EOF handling, rendering, disposal, and piped-vs-TTY behavior stay unit-covered without hijacking process globals; the keyless Loader-path smokes keep guarding the export shape end-to-end. ## Why not promote it to `ui/` instead? From fa139c88b75ad3f5854ec9ce9c2abdeedc1c66fb Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 4 Jul 2026 14:06:26 +0800 Subject: [PATCH 10/10] =?UTF-8?q?docs(rfc):=20reject=20prune-unimplemented?= =?UTF-8?q?-subagent-vocabulary=20=E2=80=94=20reserved=20surface?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Maintainer decision: the subagent seam's deferred capability vocabulary (outputSchema/structured, toolFilter, sendMessage/resume) is intentionally reserved — the seam advertises the full intended contract ahead of its implementations so providers and consumers grow into a stable shape. Moved to rejected/ with the rationale on the status line; the consumer-evidence analysis stays as the record of what is currently unimplemented. --- docs/rfc/README.md | 2 +- .../2026-07-04-prune-unimplemented-subagent-vocabulary.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename docs/rfc/{proposed => rejected}/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md (93%) diff --git a/docs/rfc/README.md b/docs/rfc/README.md index f427e03a4a..6124c94ffc 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -56,7 +56,6 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Drop the unconsumed web observation surface — the `providers-change` event and the status methods](proposed/simplification/2026-07-04-drop-unconsumed-web-observation-surface.md) | 2026-07-04 | | [Drop the `image` content block until a path can honor it](proposed/simplification/2026-07-04-drop-image-content-block.md) | 2026-07-04 | | [Prune write-only fields and a dead routing knob from the fs seam](proposed/simplification/2026-07-04-prune-write-only-fs-surface.md) | 2026-07-04 | -| [Prune the unimplemented subagent seam vocabulary](proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md) | 2026-07-04 | | [Trim unreachable ACP bridge surface — the branding knobs and the kind-sniffing fallback](proposed/simplification/2026-07-04-trim-acp-bridge-unreachable-surface.md) | 2026-07-04 | | [Prune dead core-spine surface — `SurfaceManager.invalidate()`, the loop-internal exports, `ToolExecutionResult.callId`](proposed/simplification/2026-07-04-prune-dead-core-spine-surface.md) | 2026-07-04 | | [Share the app bins' boot glue instead of maintaining twin copies](proposed/simplification/2026-07-04-share-app-bin-boot-glue.md) | 2026-07-04 | @@ -199,6 +198,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Retire mid-turn steering](rejected/simplification/2026-06-20-retire-mid-turn-steering.md) | 2026-06-20 | | [Return the ACP bridge to one live session per connection](rejected/simplification/2026-06-20-single-session-acp-bridge.md) | 2026-06-20 | | [Truncate interrupted final turns on load](rejected/simplification/2026-06-20-truncate-interrupted-turns.md) | 2026-06-20 | +| [Prune the unimplemented subagent seam vocabulary](rejected/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md) | 2026-07-04 | ### Architecture diff --git a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md b/docs/rfc/rejected/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md similarity index 93% rename from docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md rename to docs/rfc/rejected/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md index a68cb015c2..9eaa1a5a8d 100644 --- a/docs/rfc/proposed/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md +++ b/docs/rfc/rejected/simplification/2026-07-04-prune-unimplemented-subagent-vocabulary.md @@ -1,6 +1,6 @@ # RFC: Prune the unimplemented subagent seam vocabulary -Status: proposed +Status: rejected — the deferred capability vocabulary (`outputSchema`/`structured`, `toolFilter`, `sendMessage`/`resume`) is intentionally reserved surface: the seam advertises the full intended contract ahead of its implementations by design, so providers and consumers grow into a stable shape rather than re-negotiating it per capability. The consumer-evidence analysis below stands as the record of what is currently unimplemented. ## Problem