diff --git a/.agents/skills/dsh-find-simplifications/agents/openai.yaml b/.agents/skills/dsh-find-simplifications/agents/openai.yaml deleted file mode 100644 index 4016526f85..0000000000 --- a/.agents/skills/dsh-find-simplifications/agents/openai.yaml +++ /dev/null @@ -1,4 +0,0 @@ -interface: - display_name: "DSH Find Simplifications" - short_description: "Find dead or overbuilt harness surfaces" - default_prompt: "Use $dsh-find-simplifications to find simplification candidates and write proposed RFCs." diff --git a/docs/rfc/README.md b/docs/rfc/README.md index b75f95d0bf..84677bda8a 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -32,27 +32,16 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [Optional Code Mode — model writes TypeScript against an SDK of all tools](proposed/2026-06-15-optional-code-mode.md) | 2026-06-15 | | [Runtime schemas for the event vocabulary (Zod vs the merge-extensible-map pattern)](proposed/2026-06-16-typed-event-schemas.md) | 2026-06-16 | | [Unify the agent id and the session id](proposed/2026-06-20-unify-agent-and-session-id.md) | 2026-06-20 | -| [Retire mid-turn steering](proposed/2026-06-20-retire-mid-turn-steering.md) | 2026-06-20 | | [Stop mirroring durable boundaries as agent events](proposed/2026-06-20-remove-agent-boundary-mirror-events.md) | 2026-06-20 | | [Keep one public stop primitive](proposed/2026-06-20-public-agent-stop-surface.md) | 2026-06-20 | -| [Drop the unconsumed `streamBlocks()` assembled-view surface](proposed/2026-06-20-drop-unconsumed-llm-block-views.md) | 2026-06-20 | -| [Drop the unconsumed registry `*/change` events](proposed/2026-06-20-drop-unconsumed-registry-change-events.md) | 2026-06-20 | +| [Drop unconsumed assembled LLM convenience surfaces](proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md) | 2026-06-20 | +| [Drop the unconsumed `llm/adapter-change` event](proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md) | 2026-06-20 | | [Prune dead methods from the persistence and bash seams](proposed/2026-06-20-prune-dead-seam-methods.md) | 2026-06-20 | | [Drop durable step boundary events](proposed/2026-06-20-drop-durable-step-boundaries.md) | 2026-06-20 | -| [Truncate interrupted final turns on load](proposed/2026-06-20-truncate-interrupted-turns.md) | 2026-06-20 | -| [Persist assembled assistant messages, not stream chunks](proposed/2026-06-20-assembled-assistant-messages-only.md) | 2026-06-20 | -| [Collapse trace-only session events](proposed/2026-06-20-collapse-trace-only-session-events.md) | 2026-06-20 | -| [Drop unused session lineage metadata](proposed/2026-06-20-drop-unused-session-lineage.md) | 2026-06-20 | -| [Make the bash tool foreground-only](proposed/2026-06-20-foreground-only-bash.md) | 2026-06-20 | -| [Drop bash full-output spill files](proposed/2026-06-20-drop-bash-output-spill-files.md) | 2026-06-20 | -| [Collapse tool-owned UI presentation](proposed/2026-06-20-generic-tool-rendering.md) | 2026-06-20 | -| [Drop ACP terminal `_meta` rendering](proposed/2026-06-20-drop-acp-terminal-meta.md) | 2026-06-20 | -| [Return the ACP bridge to one live session per connection](proposed/2026-06-20-single-session-acp-bridge.md) | 2026-06-20 | -| [Drop ACP session/load until resume has a product shape](proposed/2026-06-20-drop-acp-session-load.md) | 2026-06-20 | +| [Fold trace-only session facts into load-bearing events](proposed/2026-06-20-collapse-trace-only-session-events.md) | 2026-06-20 | +| [Extract a generic long-running tool runtime](proposed/2026-06-20-generic-long-running-tool-runtime.md) | 2026-06-20 | | [Make the shared example base providerless](proposed/2026-06-20-providerless-example-base.md) | 2026-06-20 | -| [Classify product, integration, and support packages](proposed/2026-06-20-classify-support-packages.md) | 2026-06-20 | -| [Fold the persistence interface into dsh-session](proposed/2026-06-20-fold-session-persistence-interface.md) | 2026-06-20 | -| [Remove redundant recorded snapshot log goldens](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | +| [Use the recorded session fixture as the snapshot log golden](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | | [Discover package inventories instead of maintaining static lists](proposed/2026-06-20-discover-package-inventory.md) | 2026-06-20 | ## Implemented @@ -90,3 +79,14 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | Title | First proposed | |---|---| | [Deep-readonly public surfaces](rejected/2026-06-11-immutable-public-surfaces.md) | 2026-06-11 | +| [Persist assembled assistant messages, not stream chunks](rejected/2026-06-20-assembled-assistant-messages-only.md) | 2026-06-20 | +| [Classify product, integration, and support packages](rejected/2026-06-20-classify-support-packages.md) | 2026-06-20 | +| [Drop ACP session/load until resume has a product shape](rejected/2026-06-20-drop-acp-session-load.md) | 2026-06-20 | +| [Drop ACP terminal `_meta` rendering](rejected/2026-06-20-drop-acp-terminal-meta.md) | 2026-06-20 | +| [Drop bash full-output spill files](rejected/2026-06-20-drop-bash-output-spill-files.md) | 2026-06-20 | +| [Drop unused session lineage metadata](rejected/2026-06-20-drop-unused-session-lineage.md) | 2026-06-20 | +| [Fold the persistence interface into dsh-session](rejected/2026-06-20-fold-session-persistence-interface.md) | 2026-06-20 | +| [Collapse tool-owned UI presentation](rejected/2026-06-20-generic-tool-rendering.md) | 2026-06-20 | +| [Retire mid-turn steering](rejected/2026-06-20-retire-mid-turn-steering.md) | 2026-06-20 | +| [Return the ACP bridge to one live session per connection](rejected/2026-06-20-single-session-acp-bridge.md) | 2026-06-20 | +| [Truncate interrupted final turns on load](rejected/2026-06-20-truncate-interrupted-turns.md) | 2026-06-20 | diff --git a/docs/rfc/proposed/2026-06-14-acp-multi-session.md b/docs/rfc/proposed/2026-06-14-acp-multi-session.md index 33d4a7c177..3b73726607 100644 --- a/docs/rfc/proposed/2026-06-14-acp-multi-session.md +++ b/docs/rfc/proposed/2026-06-14-acp-multi-session.md @@ -5,7 +5,7 @@ Status: proposed > **Implementation status:** the multi-session bridge (steps 1, 3, 4) and the bash task-ownership isolation are implemented in `packages/acp` + `packages/tool-bash`. **Per-session *permission* ownership is deferred** — it depends on [the ACP support permission gate](2026-06-14-acp-agent-client-protocol.md) (`TODO(rfc010-permission-gate)`), which is itself deferred; the `agent→sessionId` reverse map the gate will route through is in place. Step 2's per-session disposer scope is now implemented (see [agent lifecycle & ownership seams](../implemented/2026-06-18-agent-lifecycle-and-ownership-seams.md)): the factory returns a per-agent `AgentHandle` whose `dispose()` stops the loop, awaits quiescence, unregisters the agent, and removes its session, so a bare client disconnect leaves no registered agent or session-store entry. Status stays `proposed` until per-session permission ownership lands. -> **Competing simplification:** [Return the ACP bridge to one live session per connection](2026-06-20-single-session-acp-bridge.md) proposes reversing the multiplexing scope until the product has a concrete multi-session UX and permission model. While both RFCs remain proposed, this one represents the "finish multiplexing" path and the newer RFC represents the "remove multiplexing" path. +> **Target-client note:** Zed is the current target ACP client, and its ACP client maintains a `HashMap` plus `pending_sessions` for concurrent `session/load` calls. The competing simplification to return to one live session per connection was rejected after checking that target-client shape; this RFC remains the path for finishing multiplexing and per-session permission ownership. See [the rejected simplification](../rejected/2026-06-20-single-session-acp-bridge.md). ## Problem diff --git a/docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md b/docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md index 143f3e822a..ecba30bcbd 100644 --- a/docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md +++ b/docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md @@ -1,4 +1,4 @@ -# RFC: Collapse trace-only session events +# RFC: Fold trace-only session facts into load-bearing events Status: proposed @@ -6,23 +6,28 @@ Status: proposed The session event vocabulary includes first-class events that are not part of replayable conversation history and have little or no production consumption. `usage` is already present as a model stream chunk before the loop also appends a separate `usage` event. `error` duplicates the `turn/end { kind: 'error', message, code }` reason for loop failures; ACP settlement reads the turn-end reason, ACP rendering ignores the `error` event, and `deriveMessages()` skips it. -These events make the canonical transcript look more useful as telemetry than it currently is. They add event variants, invariants, tests, snapshots, and persistence cases, but they are not load-bearing for resume. The implemented [turn enclosure](../implemented/2026-06-15-turn-enclosure-invariant.md) already says post-turn operational diagnostics do not belong in the replayable session log. +These events make the canonical transcript look more useful as telemetry than it currently is. They add event variants, invariants, tests, snapshots, and persistence cases, but they are not load-bearing as separate records. The facts they carry can still be useful: token usage should remain available for accounting, and an error's step number should not silently disappear. The simplification is to fold those facts into nearby events consumers already must understand, not to record less information. ## Proposal -Remove trace-only events from the canonical session log unless a production consumer needs them. Model usage can be derived from retained stream chunks, attached to `assistant/message`, or emitted on a separate telemetry channel. Loop errors should be represented by `turn/end.reason` for durable transcript semantics and `agent/error` or logging for operational diagnostics. Do not keep a parallel `error` event that consumers must reconcile with the final turn reason. +Remove standalone trace-only events only where their information can be preserved without a parallel record: -If analytics become real, add a projection helper or a dedicated telemetry store with its own retention policy. The user conversation log should contain what is needed to render, resume, and audit the interaction, not every metric-shaped detail the loop happened to observe. +- Fold successful-step usage into the matching `assistant/message`, e.g. `assistant/message { turn, step, content, usage? }`, so the assembled model output and its accounting travel together. +- For a failed or aborted step that has usage but no `assistant/message`, carry the usage on the terminal turn reason or another load-bearing failure record in the same turn. The implementing design must prove no usage chunk that is currently persisted becomes unrepresented. +- Fold the step number from the standalone `error` event into `turn/end.reason` for `kind: 'error'`, e.g. `{ kind: 'error', step, message, code? }`. `turn/end` is the durable turn outcome ACP and resume already consume. +- Keep `agent/error` and logging for live diagnostics; do not add a second session-log error record after `turn/end`. + +If analytics become real, add a projection helper or a dedicated telemetry store with its own retention policy. The user conversation log should contain what is needed to render, resume, audit, and account for the interaction without requiring consumers to reconcile duplicate trace rows. ## Acceptance criteria -- `SessionEventMap` drops `usage` and `error`, or folds their fields into nearby load-bearing events. +- `SessionEventMap` drops standalone `usage` and `error` only after their fields are represented on load-bearing session events. - The loop no longer appends a separate `usage` event for a usage chunk. -- The loop records durable failures only as `turn/end { kind: 'error' }` and reports live diagnostics through `agent/error`. +- The loop records durable failures through `turn/end { kind: 'error', step, message, code? }` or an equivalent no-information-loss shape and reports live diagnostics through `agent/error`. - ACP snapshots and persistence tests stop asserting trace-only lines. -- Documentation explains where token usage and operational errors are observed if they remain available. +- Documentation explains exactly where token usage and operational errors are observed. - The session format version and recorded fixtures are refreshed; non-current stored logs are rejected per the pre-release format policy. ## What we give up -A consumer can no longer filter the canonical log for `usage` or step-level `error` events. That is a real loss for future analytics and debugging, but there is no current production analytics consumer. Keeping a telemetry-shaped event in the replay log because it might matter later repeats the dead-summary pattern from [drop the mutable session summary](../implemented/2026-06-19-drop-mutable-session-summary.md). +A consumer can no longer filter the canonical log for standalone `usage` or step-level `error` rows. It must read those facts from the assistant/failure events that carry them. That is a reasonable simplification only if the implementing PR proves the same facts remain present; otherwise the standalone events should stay. diff --git a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md index f346b8bb91..c75f93fd99 100644 --- a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md @@ -10,13 +10,13 @@ Static lists are appropriate when they encode policy; they are needless friction ## Proposal -Make package/gate inventories discoverable. Publishability should come from explicit package classification metadata, not from a static array in a script or the npm `private` flag. Module graph generation should read package manifests. `doc-sync` should be the one command that defines and prints its sub-gates, with docs linking to that command rather than restating a second list. +Make package/gate inventories discoverable. Publishability should come from explicit package aspect metadata, not from a static array in a script or the npm `private` flag. Module graph generation should read package manifests. `doc-sync` should be the one command that defines and prints its sub-gates, with docs linking to that command rather than restating a second list. -This pairs well with [classifying support packages](2026-06-20-classify-support-packages.md), because discovery needs to know which packages are product-publishable, support-only, private, or examples. +The metadata should be aspect-oriented rather than a single support/product bucket: a package may be core, bash-related, filesystem-related, persistence-related, provider-facing, example-facing, testing-only, and/or publishable. Discovery needs enough explicit facts to drive gates without baking a fragile hierarchy into every script. ## Acceptance criteria -- `publint-all` discovers publishable packages from manifests plus a single classification source. +- `publint-all` discovers publishable packages from manifests plus explicit aspect metadata. - Adding a package does not require editing a static package list for every 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. diff --git a/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md b/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md index 4bb5938b59..ad424cb06e 100644 --- a/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md +++ b/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md @@ -12,7 +12,7 @@ The boundary events make the log more ceremonial than informative. The loop trac Make the turn the only durable boundary. Remove `step/start` and `step/end` from `SessionEventMap`; keep the numeric `step` field on events that need grouping. The loop increments the step counter and records step-scoped events with that number, but it no longer appends open/close boundary events. Consumers infer step groups from contiguous events sharing `(turn, step)`. -The invariants plugin should enforce that step-scoped events have valid positive step numbers within an open turn, not that separate boundary records surround them. Crash repair should not synthesize `step/end`; if [interrupted turns are truncated](2026-06-20-truncate-interrupted-turns.md), the repair path disappears entirely. +The invariants plugin should enforce that step-scoped events have valid positive step numbers within an open turn, not that separate boundary records surround them. Crash repair should not synthesize `step/end`; if an interrupted turn is preserved, the repair path can still close the turn without inventing step boundary records. ## Acceptance criteria diff --git a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md b/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md new file mode 100644 index 0000000000..ab7f23296b --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md @@ -0,0 +1,43 @@ +# RFC: Drop the unconsumed `llm/adapter-change` event + +Status: proposed + +## Problem + +`LlmService.registerAdapter()` emits `llm/adapter-change` on registration and disposal ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)). Grepping `llm/adapter-change` across `packages/*/src` and `examples/*/src` finds only the declaration, emit sites, docs, and tests; no production listener subscribes to it. + +This differs from `tools/change` and `system-prompt/change`. Those two events are also unconsumed today, but they are plausible registry-change signals for future live tool/prompt UIs. LLM adapter registration is more of a boot-time implementation detail: adapters are not a user-visible palette and the real model-call interception seam is `llm/stream`. Keeping an adapter-change event with no listener repeats the [drop-the-dead-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern at a smaller scale. + +The event is not free. `registerAdapter()` yields its rollback disposer before emitting `llm/adapter-change` so a throwing listener unwinds the mutation instead of leaking an adapter entry, and the package carries tests for that listener-throw path. That defensive ordering protects a failure mode only tests can trigger. + +## Proposal + +Remove only `llm/adapter-change`: + +- Delete the `llm/adapter-change` declaration from `dsh-llm`'s `interface Events`. +- Delete the `ctx.emit('llm/adapter-change')` calls. +- Simplify `registerAdapter()`'s effect generator: keep the mutation and rollback disposer for HMR/disposal, but drop the listener-throw rollback ordering that exists only for the removed event. +- Remove the "Emits `llm/adapter-change` on registration and disposal" sentence from `LlmService.registerAdapter`'s JSDoc. +- Rewrite the adapter-disposer test to assert the returned disposer removes the adapter without subscribing to `llm/adapter-change`; delete the listener-throw rollback test that exists solely for the removed event. +- Update the event taxonomy table in [docs/architecture.md](../../../docs/architecture.md) and [packages/llm/README.md](../../../packages/llm/README.md). The [doc-sync-enforcement RFC](../implemented/2026-06-11-doc-sync-enforcement.md) should avoid using `llm/adapter-change` as an example once the event is gone. + +## Why not remove every registry change event? + +A microkernel where registries announce mutations is a coherent convention. `tools/change` and `system-prompt/change` may become useful when a UI can live-refresh available tools or prompt sections. This RFC leaves that convention intact where it has a plausible user-facing consumer and cuts only the adapter-change event whose current and likely future consumer is unclear. + +If an LLM adapter browser or dynamic model-picker needs this signal later, reintroduce it with that consumer and a clearer payload than "something changed." + +## Acceptance criteria + +- `llm/adapter-change` and its emits are gone; `pnpm run verify-event-taxonomy` passes against the updated table. +- HMR-safety tests still pass: disposing a contributing fiber still removes the adapter. +- `tools/change` and `system-prompt/change` remain documented and tested. +- `pnpm run test:coverage` stays 100% per-file. +- No production code path changes observable behavior (verified by unchanged ACP snapshot goldens and the echo-agent smoke test). + +## Risks + +- **Removing a documented emit event is a public-surface change.** It is in the taxonomy table, so it reads as deliberate API. But "declared and emitted" is not "consumed" — the same distinction that justified dropping the mutable summary. The taxonomy table is updated in the same change, so the docs do not drift. +- **The registry-change convention becomes uneven.** That is acceptable because LLM adapter registration is not the same user-facing concept as tools or prompt sections. Uneven but honest beats uniform but dead. + +This is a small cut, but it retires a standing correctness invariant that guards a consumer that does not exist. diff --git a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md b/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md new file mode 100644 index 0000000000..85198e7d84 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md @@ -0,0 +1,45 @@ +# RFC: Drop unconsumed assembled LLM convenience surfaces + +Status: proposed + +## Problem + +`LlmService` ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)) exposes three call surfaces over a model: + +- `stream()` — raw `StreamChunk`s, dispatched through the `llm/stream` waterfall. +- `streamBlocks()` — a "convenience view" that runs the chunks through a `BlockAssembler` and yields completed `ContentBlock`s in stream order ([index.ts:137-144](../../../packages/llm/src/index.ts)). +- `generate()` — one fully-assembled `GenerateResult`, dispatched through a second `llm/generate` waterfall ([index.ts:151-157](../../../packages/llm/src/index.ts)). + +The only production consumer of the LLM service is the agent loop, and it uses `stream()` exclusively — feeding raw chunks through its own `BlockAssembler` so it can log chunks for replay fidelity while assembling in parallel ([packages/agent-loop/src/loop.ts](../../../packages/agent-loop/src/loop.ts), the `ctx.llm.stream(req)` step). Grepping `streamBlocks` and `ctx.llm.generate` across `packages/*/src` and `examples/*/src` finds no production callers. The references are the service methods, docs, and tests; adapter tests use `generate()` as a convenient driver, but they can hand-drain `stream()` through the same assembler helper without preserving a public production API. + +This is the [drop-mutable-session-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern: assembled-view APIs with tested contracts, consumed by tests rather than production. They were built speculatively for consumers that do not care about token-level deltas, but the one real consumer cares about deltas precisely so it can persist high-fidelity replay data. + +`streamBlocks()` drags a dedicated slice of `BlockAssembler` behind it: `flushReady()` and `flushRemaining()` ([packages/llm/src/assembler.ts:138-168](../../../packages/llm/src/assembler.ts)) plus the `flushed` cursor field exist only to support incremental in-order yield. `generate()` drags `GenerateResult`, `BlockAssembler.result()`, and the `llm/generate` waterfall as a second interception surface over the same underlying stream. The loop's assembler usage is `push()` / `message()` / `usage` / `finish` — not streaming flush or one-shot service assembly. + +## Proposal + +Make `stream()` the only public LLM call surface: + +- Remove `LlmService.streamBlocks()` and its JSDoc. +- Remove `LlmService.generate()`, the `llm/generate` waterfall event, and `GenerateResult` if no surviving API needs that named result shape. +- Remove `BlockAssembler.flushReady()`, `BlockAssembler.flushRemaining()`, and the `flushed` cursor field. +- Remove `BlockAssembler.result()` if it is only a helper for the deleted `generate()` service path and tests. +- Replace adapter-test use of `ctx.llm.generate()` with a small test helper that calls `ctx.llm.stream()`, pushes chunks into `BlockAssembler`, and returns the assembled message, usage, and finish reason needed by that test. That keeps the [twin-adapter design](../implemented/2026-06-13-twin-llm-adapters.md) intact while avoiding a public method whose only callers are tests. +- Remove or rework the `flushReady`/`flushRemaining`-dependent tests. Keep assembler invariants that still apply to `push()` / `blocks()` / `message()`; delete behavior that only pins the removed flush API. +- Update every doc/comment reference to `streamBlocks`, `generate`, `GenerateResult`, and `llm/generate` across `docs/`, package READMEs, and source comments. The `ctx.llm` service-map row in [docs/architecture.md](../../../docs/architecture.md) becomes `stream()` only, the event taxonomy drops `llm/generate`, and the [property-based-testing RFC](../implemented/2026-06-11-property-based-testing.md) names block-assembly invariants without referring to removed convenience methods. + +## Acceptance criteria + +- `streamBlocks`, `generate`, `llm/generate`, and the assembler helpers they alone require are gone; `pnpm run knip` reports no new dead exports. +- `pnpm run test:coverage` stays at 100% per-file (the deleted methods take their dedicated tests with them; no remaining line goes uncovered). +- Adapter tests still exercise both real adapters through `stream()` and the shared assembler, not through a test-only public shortcut. +- The loop behaves identically — verified by unchanged ACP snapshot goldens. +- `packages/llm/README.md`, [docs/architecture.md](../../../docs/architecture.md), and module docs no longer mention the removed convenience surfaces. + +## Risks + +- **It removes public methods from a core vocabulary package.** A future plugin that wants assembled blocks without deltas would need to call `stream()` and use `BlockAssembler` directly or reintroduce a focused helper with a real consumer. Given the pre-release "foundation over speculative future" stance ([AGENTS.md](../../../AGENTS.md)), this is the right time to cut test-only public shape. +- **Adapter tests get a little more explicit.** They lose the ergonomic `generate()` wrapper, but that is useful pressure: tests exercise the same streaming path production uses. +- **Waterfall users lose `llm/generate`.** No production listener exists. Any future caching/retry/logging plugin should wrap `llm/stream`, which remains the single provider call path. + +The size is modest, but it is a clean removal of speculative surface area from the LLM package, leaving one model-call contract for both production and tests. diff --git a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-block-views.md b/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-block-views.md deleted file mode 100644 index a0fb71d40b..0000000000 --- a/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-block-views.md +++ /dev/null @@ -1,44 +0,0 @@ -# RFC: Drop the unconsumed `streamBlocks()` assembled-view surface on `dsh-llm` - -Status: proposed - -## Problem - -`LlmService` ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)) exposes three call surfaces over a model: - -- `stream()` — raw `StreamChunk`s, dispatched through the `llm/stream` waterfall. -- `streamBlocks()` — a "convenience view" that runs the chunks through a `BlockAssembler` and yields completed `ContentBlock`s in stream order ([index.ts:137-144](../../../packages/llm/src/index.ts)). -- `generate()` — one fully-assembled `GenerateResult`, dispatched through a second `llm/generate` waterfall ([index.ts:151-157](../../../packages/llm/src/index.ts)). - -The only production consumer of the LLM service is the agent loop, and it uses `stream()` exclusively — feeding the raw chunks through its own `BlockAssembler` so it can log raw chunks for replay fidelity while assembling in parallel ([packages/agent-loop/src/loop.ts](../../../packages/agent-loop/src/loop.ts), the `ctx.llm.stream(req)` step). Grepping `streamBlocks` across `packages/*/src` and `examples/*/src` finds zero callers; the only references are the method itself, two doc comments, and two test files (`llm/tests/properties.spec.ts`, `agent-loop/tests/review-fixes.spec.ts`). - -This is the [drop-mutable-session-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern: an entire assembled-view API with a property-tested contract, consumed by nothing but its own tests. It was built speculatively for "consumers that don't care about token-level deltas" that never materialized — the one real consumer cares about deltas precisely so it can log them. - -`streamBlocks()` also drags a dedicated slice of `BlockAssembler` behind it: `flushReady()` and `flushRemaining()` ([packages/llm/src/assembler.ts:138-168](../../../packages/llm/src/assembler.ts)) plus the `flushed` cursor field exist only to support the incremental in-order yield. The loop's assembler usage is `push()` / `message()` / `usage` / `finish` — never the streaming flush. With `streamBlocks()` gone, `flushReady`/`flushRemaining`/`flushed` are dead too. - -## Proposal - -Delete `streamBlocks()` and the assembler's streaming-flush machinery it alone drives: - -- Remove `LlmService.streamBlocks()` and its JSDoc. -- Remove `BlockAssembler.flushReady()`, `BlockAssembler.flushRemaining()`, and the `flushed` cursor field. -- Remove or rework the `flushReady`/`flushRemaining`-dependent tests: in `llm/tests/properties.spec.ts` the `flushReady() ++ flushRemaining() === blocks()` property, the strict-order property, and the "streaming and one-shot assembly agree on usage and finish" property (which pushes-then-flushes incrementally) all exercise the streaming-flush path; the `flushRemaining` cases in `llm/tests/assembler.spec.ts` and the three `streamBlocks` edge-case tests in `agent-loop/tests/review-fixes.spec.ts` likewise. Each is either deleted or, where it also asserts a non-flush invariant worth keeping (e.g. streaming vs one-shot agreeing on usage/finish), rewritten to use `push()` + `message()`/`result()` without the removed flush methods — the behavior pinned to the deleted methods goes, per AGENTS.md "tests document behavior, not golden truth". -- Update every doc/comment reference to `streamBlocks` — grep it across `docs/`, `packages/llm/README.md`, and source comments. `packages/llm/README.md` mentions it twice (the API-list row and the `BlockAssembler` "used by `streamBlocks()`/`generate()`" line); the `assembler.ts` module doc references it; and the retained `generate()` JSDoc currently reads "Same completion guarantees as `streamBlocks()`" — reword it to state the guarantee directly. The `ctx.llm` service-map row in [docs/architecture.md](../../../docs/architecture.md) (`stream()` / `streamBlocks()` / `generate()`) drops `streamBlocks()` too. The [property-based-testing RFC](../implemented/2026-06-11-property-based-testing.md) needs two edits: its motivating anecdote ("a `streamBlocks` ordering bug") is reworded to name the bug class (a block-assembly ordering bug) rather than a removed method, and its dsh-llm invariant list — which names `flushReady()+flushRemaining() ≡ blocks()` as a checked property — is updated to drop the removed-method invariant and keep only the ones the surviving assembler API (`push`/`blocks`/`message`/`result`) still supports. - -## Scope: why `generate()` and `llm/generate` stay - -`generate()` is not dead the same way: the twin-adapter e2e/unit suites (`llm-deepseek`, `llm-pi-ai`) use `ctx.llm.generate({...})` as a convenient one-shot driver to assert provider behavior, and `GenerateResult` / `assembler.result()` back it. Those adapter tests are the [twin-adapter design](../implemented/2026-06-13-twin-llm-adapters.md), explicitly out of scope for a simplification pass. Removing `generate()` would force adapter-test call sites to hand-drain `stream()`, which is churn in protected territory for a method that is at least a legitimate ergonomic driver. So this RFC deliberately stops at the surface that nothing — not even an out-of-scope test — consumes. If a later pass wants to also collapse `generate()`/`llm/generate`/`result()`, that is a separate decision with a real caller to migrate. - -## Acceptance criteria - -- `streamBlocks` and the assembler streaming-flush methods are gone; `pnpm run knip` reports no new dead exports. -- `pnpm run test:coverage` stays at 100% per-file (the deleted methods take their dedicated tests with them; no remaining line goes uncovered). -- `generate()`, `stream()`, `result()`, `blocks()`, `message()` are untouched and the loop behaves identically — verified by the unchanged ACP snapshot goldens. -- `packages/llm/README.md` and the module docs no longer mention `streamBlocks`. - -## Risks - -- **It removes a public method from a core vocabulary package.** A future plugin that wants "assembled blocks without the deltas" would have to re-add it (or call `generate()` and read `.message.content`). Given the pre-release "foundation over speculative future" stance ([AGENTS.md](../../../AGENTS.md)) and that the obvious assembled-view need is already served by `generate()`, this is the right time to cut — re-adding a thin assembler wrapper later is trivial if a real consumer appears. -- **Low blast radius.** The change is confined to `dsh-llm`; no other package imports `streamBlocks` or the flush methods, so there is no cross-package ripple. - -The size is modest, but it is a clean, zero-production-impact removal of a speculative surface — the cheapest kind of correctness. diff --git a/docs/rfc/proposed/2026-06-20-drop-unconsumed-registry-change-events.md b/docs/rfc/proposed/2026-06-20-drop-unconsumed-registry-change-events.md deleted file mode 100644 index 16c1b8faf2..0000000000 --- a/docs/rfc/proposed/2026-06-20-drop-unconsumed-registry-change-events.md +++ /dev/null @@ -1,50 +0,0 @@ -# RFC: Drop the unconsumed registry `*/change` notification events - -Status: proposed - -## Problem - -Three registries each emit a "something changed" notification event that no production listener subscribes to: - -- `tools/change` — emitted by `ToolRegistry.register()` on register and disposal ([packages/tools/src/index.ts:302-304](../../../packages/tools/src/index.ts)). -- `system-prompt/change` — emitted by `SystemPrompt.section()` and `.tools()` ([packages/system-prompt/src/index.ts:86-110](../../../packages/system-prompt/src/index.ts)). -- `llm/adapter-change` — emitted by `LlmService.registerAdapter()` ([packages/llm/src/index.ts:98-100](../../../packages/llm/src/index.ts)). - -Grepping the three event names across `packages/*/src` and `examples/*/src` finds only the emit sites and their declarations — zero `ctx.on('.../change')` listeners in production. The only subscribers are each package's own spec file, and they subscribe purely to test that the emit fires. They are listed in the event taxonomy table ([docs/architecture.md](../../../docs/architecture.md)) as `emit` events, but nothing reacts to them. - -These events are speculative generality for a hypothetical reactive consumer (a UI that live-refreshes its tool palette, say) that does not exist. That alone would be a mild [drop-the-dead-summary](../implemented/2026-06-19-drop-mutable-session-summary.md)-style cut. What makes it worth an RFC is the machinery the events drag along: to emit `.../change` safely, each registry orders its generator effect so the rollback disposer is `yield`ed before the change-emit, specifically so a throwing change-listener unwinds the mutation instead of leaking a registry entry. Every one of the three carries a multi-line comment justifying this ordering, plus a dedicated "rollback when a change listener throws" test. That is a non-trivial correctness burden guarding a failure mode that only the tests' own injected listeners can trigger, because there are no real listeners. - -## Proposal - -Remove the three `*/change` events and the defensive machinery that exists only to make them safe: - -- Delete the `tools/change`, `system-prompt/change`, `llm/adapter-change` declarations from each package's `interface Events`. -- Delete the `ctx.emit('.../change')` calls. -- Simplify each `ctx.effect` generator: the mutation and its rollback disposer remain (HMR/disposal still need them), but the "yield rollback before the emit so a throwing listener rolls back" ordering comment and any emit-after-yield collapse to a plain `set`/`push` plus a `yield () => delete`/`splice`. No behavior an external observer can see changes, because nothing observes the events. -- Remove the "Emits `.../change` on register/unregister" sentence from the surviving registration-method JSDocs — these sit on methods that stay, so they go stale rather than vanish with the deleted code: `LlmService.registerAdapter` ([packages/llm/src/index.ts](../../../packages/llm/src/index.ts)), `ToolRegistry.register` ([packages/tools/src/index.ts](../../../packages/tools/src/index.ts)), and both `SystemPrompt.section` and `SystemPrompt.tools` ([packages/system-prompt/src/index.ts](../../../packages/system-prompt/src/index.ts)). -- Delete or rewrite the tests that exist to exercise the events. The change-listener-rollback tests are deleted outright (the rollback behavior goes with the event). The positive emission-subscriber tests are handled case by case: `system-prompt/tests/system-prompt.spec.ts`'s "emits system-prompt/change ..." is deleted (its disposal coverage is duplicated by the separate "cleans up tool providers on fiber dispose" / "removes section when returned disposer is called directly" tests), but `llm/tests/service.spec.ts`'s "disposes adapter registration on adapter-change event emission" is the only test that calls the `registerAdapter()` returned disposer and asserts the adapter is removed (the HMR test at "unregisters adapters when the owning fiber is disposed" covers fiber disposal, a different path) — so it is rewritten to drop the event subscription while keeping the returned-disposer assertion, not deleted. Per AGENTS.md "tests document behavior, not golden truth". -- Update the event taxonomy table in [docs/architecture.md](../../../docs/architecture.md) (remove the three rows) and re-run `pnpm run verify-event-taxonomy`, which mechanically checks the table against source. Also remove the per-package README event rows that list them: [packages/tools/README.md](../../../packages/tools/README.md) (`tools/change`), [packages/system-prompt/README.md](../../../packages/system-prompt/README.md) (`system-prompt/change`), and [packages/llm/README.md](../../../packages/llm/README.md) (`llm/adapter-change`). The [doc-sync-enforcement RFC](../implemented/2026-06-11-doc-sync-enforcement.md), whose `verify-event-taxonomy` description names these three as the events that surfaced when the check landed, is reworded so its example does not point at removed events. - -## Why not keep them as a "registries announce changes" convention? - -That is the honest counter-argument: a microkernel where every registry announces its mutations is a clean, uniform reactive substrate, and a future live UI would want exactly this. Three considerations push the other way: - -1. **The harness already has a finer-grained feed for the one realistic consumer.** A UI live-renders from `session/event` and `agent/*`, not from registry mutations — tools/sections/adapters are registered at plugin-load time and effectively static during a session. The `.../change` events fire almost exclusively during boot and HMR, when nothing is watching. -2. **Pre-release stance.** [AGENTS.md](../../../AGENTS.md) says optimize for the correct foundation, not a speculative future; add the seam when a real consumer needs it. Re-adding an emit is one line; the cost today is the standing rollback-ordering burden on three hot registration paths. -3. **The events are not free — they shape the registration code.** Keeping them means keeping the throwing-change-listener invariant and its tests forever, for a listener that cannot exist until someone adds one. - -If a reactive consumer is later built, it should be reintroduced deliberately, as one coherent decision about which registries announce what (and possibly a single `registry/change` shape), not as three independently-grown emits nothing reads. - -## Acceptance criteria - -- The three events and their emits are gone; `pnpm run verify-event-taxonomy` passes against the updated table. -- HMR-safety tests still pass: disposing a contributing fiber still removes the tool/section/adapter (the rollback disposer is retained; only the change-emit and its throwing-listener guard are removed). -- `pnpm run test:coverage` stays 100% per-file. -- No production code path changes observable behavior (verified by unchanged ACP snapshot goldens and the echo-agent smoke test). - -## Risks - -- **Removing a documented emit event is a public-surface change.** It is in the taxonomy table, so it reads as deliberate API. But "declared and emitted" is not "consumed" — the same distinction that justified dropping the mutable summary. The taxonomy table is updated in the same change, so the docs do not drift. -- **A registry that genuinely wants change-notification later pays a small reintroduction cost.** Judged acceptable per the pre-release stance; the reintroduction is mechanical. - -This is a small-to-medium cut across three packages and, more valuably, it retires a standing correctness invariant that guards a consumer that does not exist. diff --git a/docs/rfc/proposed/2026-06-20-foreground-only-bash.md b/docs/rfc/proposed/2026-06-20-foreground-only-bash.md deleted file mode 100644 index 3197e916ca..0000000000 --- a/docs/rfc/proposed/2026-06-20-foreground-only-bash.md +++ /dev/null @@ -1,27 +0,0 @@ -# RFC: Make the bash tool foreground-only - -Status: proposed - -## Problem - -The bash capability seam supports both foreground commands and long-running background tasks. Background support is large: the abstract executor exposes `start`, `get`, `ownerOf`, `list`, `readOutput`, `kill`, and `onTaskDone`; the local executor tracks tasks, incremental reads, owner tokens, process cleanup, and completion listeners; the model sees three tools (`bash`, `bash_output`, `bash_kill`); the tool plugin injects completion notices back into the owning agent's session. The local executor fences task access behind owner tokens because predictable global task ids are a cross-session read/kill hazard. - -The [tool cookbook](../../cookbook/adding-a-tool.md) already points at the real design smell: background bash is really generic long-running-tool infrastructure living inside one tool. If future tools need background execution, polling, kill, ownership, and completion notices, those semantics should not be hidden in `dsh-bash`. - -## Proposal - -Temporarily collapse `bash` to foreground-only execution. Remove the model-facing `run_in_background` schema field, the `bash_output` and `bash_kill` tools, background task ownership, incremental task reads, completion injection, and task-listener APIs from the bash executor seam. The `BashExecRequest` request type is already foreground-shaped; the removal surface is the tool schema plus the executor's background-task methods. Long commands can still run with an explicit timeout; a command that needs to outlive a model step is not supported until a generic task service exists. - -If long-running tasks return later, implement them once as a capability-agnostic task layer that owns ids, authorization, polling, cancellation, completion notifications, and any UI affordances. Bash can then opt into that layer like any other tool. - -## Acceptance criteria - -- `@deepseek-ai/dsh-tool-bash` registers only the `bash` tool. -- `BashExecutor` exposes `resolve()` and foreground `run()` only. -- `@deepseek-ai/dsh-bash-local` no longer tracks background task maps, owner tokens, task listeners, or incremental output cursors. -- ACP and snapshot fixtures no longer mention `bash_output` or `bash_kill`. -- The [tool cookbook](../../cookbook/adding-a-tool.md) either removes the background example or redirects long-running work to a future generic task proposal. - -## What we give up - -The model loses the ability to start a server or long-running command, continue other work, and poll later. That is a real capability regression, but the current design makes one tool carry infrastructure that belongs above all tools. Foreground-only bash is smaller, safer, and easier to sandbox while the generic long-running-tool design is still absent. diff --git a/docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md b/docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md new file mode 100644 index 0000000000..825fb133fe --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md @@ -0,0 +1,35 @@ +# RFC: Extract a generic long-running tool runtime + +Status: proposed + +## Problem + +The bash capability seam supports both foreground commands and long-running background tasks. Background support is large: the abstract executor exposes `start`, `get`, `ownerOf`, `list`, `readOutput`, `kill`, and `onTaskDone`; the local executor tracks tasks, incremental reads, owner tokens, process cleanup, and completion listeners; the model sees three tools (`bash`, `bash_output`, `bash_kill`); the tool plugin injects completion notices back into the owning agent's session. The local executor fences task access behind owner tokens because predictable global task ids are a cross-session read/kill hazard. + +The [tool cookbook](../../cookbook/adding-a-tool.md) already points at the real design smell: background bash is really generic long-running-tool infrastructure living inside one tool. If future tools need background execution, polling, kill, ownership, and completion notices, those semantics should not be hidden in `dsh-bash`. + +## Proposal + +Move long-running task semantics above bash into a tool-agnostic runtime. Bash remains able to run background commands, but it stops owning the general concepts of task ids, ownership tokens, polling, cancellation, completion notifications, and model-facing "read/kill this task" commands. + +The runtime should own: + +- Stable task ids and owner tokens keyed to the calling session/agent. +- Registration of a long-running task with a producer for incremental output and a completion promise. +- Generic read/cancel/list operations with the same cross-session authorization rule for every tool. +- Completion notification injection into the owning session. +- Presentation hooks for pending/running/completed task state, with bash supplying only command-specific labels and output formatting. + +`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. + +## Acceptance criteria + +- The bash-specific packages no longer define the generic task registry, owner-token authorization, polling, cancellation, or completion-notification machinery. +- A shared long-running-task service or tool layer owns those semantics and is documented as the path for any future background-capable tool. +- Bash background behavior remains available through the shared layer, with tests proving cross-session isolation still holds. +- ACP and snapshot fixtures render background bash through the shared task vocabulary, not through bash-only lifecycle semantics. +- The [tool cookbook](../../cookbook/adding-a-tool.md) points long-running tools at the shared runtime instead of telling each tool to invent its own task protocol. + +## What we give up + +The bash package loses local ownership of an already-working background-task implementation, and the implementing PR may temporarily churn model-facing tool names or transcript presentation. That churn is worthwhile if it leaves one background-task contract instead of making every future long-running tool clone bash's private protocol. diff --git a/docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md b/docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md index 8ff4ebb46b..6c67413a49 100644 --- a/docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md +++ b/docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md @@ -18,7 +18,7 @@ Delete public `abort()` and `whenIdle()`, the tests that exercise them as standa ## Acceptance criteria -- `Agent` exposes no public `abort()` or `whenIdle()`; if [retiring mid-turn steering](2026-06-20-retire-mid-turn-steering.md) has not landed, `steer()` remains part of the message surface. +- `Agent` exposes no public `abort()` or `whenIdle()`; `steer()` remains part of the message surface. - ACP cancellation continues to call `cancel()`. - Agent teardown continues to await quiescence through handle disposal. - Tests cover cancellation and disposal as the two supported stop paths. @@ -29,4 +29,4 @@ A future plugin cannot abort only the current model/tool step while preserving q ## Related -This RFC only removes the stop/quiescence methods. If it lands before [retiring mid-turn steering](2026-06-20-retire-mid-turn-steering.md), `steer()` remains part of the `Agent` message surface; if the steering RFC lands first, the resulting surface is `send()`, `inject()`, `cancel()`, status, options, session, and identity. +This RFC only removes the stop/quiescence methods. Mid-turn steering remains an intentional message path; the resulting public surface is `send()`, `steer()`, `inject()`, `cancel()`, status, options, session, and identity. diff --git a/docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md b/docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md index cbb299dfca..4b1cd75a56 100644 --- a/docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md +++ b/docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md @@ -28,4 +28,4 @@ A plugin can no longer observe turn/step boundaries from a convenient `Agent`-fi ## Related -This is compatible with [assembled assistant messages only](2026-06-20-assembled-assistant-messages-only.md), but the exact fate of `agent/stream-chunk` depends on that decision. If chunks leave the canonical log, `agent/stream-chunk` can remain as a deliberately live-only UI signal while the other mirror events disappear. +Because high-fidelity `assistant/chunk` persistence remains load-bearing, `agent/stream-chunk` can be evaluated as another mirror of durable session data rather than as the only token stream. If a future proposal moves chunks out of the canonical log, `agent/stream-chunk` would need a fresh decision as a deliberately live-only UI signal. diff --git a/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md b/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md index aedb1b9a40..cf853ce341 100644 --- a/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md +++ b/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md @@ -1,4 +1,4 @@ -# RFC: Remove redundant recorded snapshot log goldens +# RFC: Use the recorded session fixture as the snapshot log golden Status: proposed @@ -6,17 +6,17 @@ Status: proposed Recorded ACP snapshot scenarios ship both `session.jsonl` and `session.golden.jsonl`. For normal recorded scenarios, `session.jsonl` is the replay fixture harvested from a real run, and the replay test normalizes the newly persisted log and compares it to `session.golden.jsonl`. In the current fixtures, the normalized recorded log and normalized golden are identical for the ordinary recorded scenarios. -The duplicate file can help review by showing "expected persisted log" separately from "model replay input", but for recorded scenarios those are intentionally the same artifact. Keeping both means a re-record churns two files with the same semantic content. +The duplicate file can help review by showing "expected persisted log" separately from "model replay input", but for recorded scenarios those are intentionally the same artifact. Keeping both means a re-record churns two files with the same semantic content, when one committed session log can serve as both replay input and expected persisted output. ## Proposal -For recorded scenarios, compare the replay run's normalized session log directly against normalized `session.jsonl`. Keep explicit `session.golden.jsonl` only for authored scenarios where `replay.override.json` drives behavior that is not derivable from the fixture, or where the expected persisted log intentionally differs from the replay script. +For recorded scenarios, keep one session-log artifact: `session.jsonl`. The snapshot test compares the replay run's normalized persisted log directly against normalized `session.jsonl`. Keep explicit `session.golden.jsonl` only for authored scenarios where `replay.override.json` drives behavior that is not derivable from the fixture, or where the expected persisted log intentionally differs from the replay script. Stdout goldens remain unchanged; they are the editor-facing projection and are not redundant with the session fixture. ## Acceptance criteria -- Recorded scenarios stop committing `session.golden.jsonl`. +- Recorded scenarios commit `session.jsonl` as the single session-log fixture/golden and stop committing `session.golden.jsonl`. - The snapshot test derives the expected session log from `session.jsonl` for `recorded: true` scenarios. - Authored sidecar scenarios keep explicit session goldens when needed. - Orphan-fixture guards understand which files are required by scenario kind. diff --git a/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md b/docs/rfc/rejected/2026-06-20-assembled-assistant-messages-only.md similarity index 85% rename from docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md rename to docs/rfc/rejected/2026-06-20-assembled-assistant-messages-only.md index ecbd06a734..173b63e8aa 100644 --- a/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md +++ b/docs/rfc/rejected/2026-06-20-assembled-assistant-messages-only.md @@ -1,6 +1,6 @@ # RFC: Persist assembled assistant messages, not stream chunks -Status: proposed +Status: rejected — high-fidelity chunk replay, partial failed streams, and snapshot replay currently depend on persisted `assistant/chunk` events. Dropping chunks is only viable with a no-information-loss replay/artifact replacement. ## Problem @@ -10,7 +10,7 @@ For successful steps that assemble completed content, the loop already appends a ## Proposal -Stop storing `assistant/chunk` in the canonical session log. The durable log keeps `assistant/message`, `tool/call`, `tool/result`, `usage` if retained, and turn boundaries. Live UIs can still receive token deltas through a deliberately transient stream event. Snapshot replay should move its model script into an explicit fixture sidecar or derive it from a recorded adapter artifact, rather than treating the canonical user session as a token tape. Scenarios that need partial failed-stream output must record that output in the replay fixture or accept that it is not part of completed conversation history. +Stop storing `assistant/chunk` in the canonical session log. The durable log keeps `assistant/message`, `tool/call`, `tool/result`, `usage` if retained, and turn boundaries. Live UIs can still receive token deltas through a deliberately transient stream event. Snapshot replay should move its model script into an explicit fixture sidecar or derive it from a recorded adapter artifact, rather than treating the canonical user session as a token tape. Scenarios that need partial failed-stream output must record that output in the replay fixture. ACP `session/load` can replay prior assistant messages as complete content blocks instead of simulating the original token stream. A loaded transcript need not reproduce every historical delta; it must show the same completed assistant content and resume with a valid provider history. @@ -25,7 +25,7 @@ ACP `session/load` can replay prior assistant messages as complete content block ## What we give up -The canonical user session no longer reconstructs the exact token stream of an old turn. It also loses partial assistant output from failed or aborted streams unless another event or fixture records it. That is acceptable for resume and load, where completed message content is the user-visible state. Tests that need exact deterministic streams should own that fixture directly instead of smuggling it through the durable session format. +The canonical user session no longer reconstructs the exact token stream of an old turn. It also loses partial assistant output from failed or aborted streams unless another event or fixture records it. That is too much information loss for the current resume, load, and snapshot contracts. Tests that need exact deterministic streams should own that fixture directly only if the production session log keeps enough fidelity for user-visible recovery. ## Related diff --git a/docs/rfc/proposed/2026-06-20-classify-support-packages.md b/docs/rfc/rejected/2026-06-20-classify-support-packages.md similarity index 82% rename from docs/rfc/proposed/2026-06-20-classify-support-packages.md rename to docs/rfc/rejected/2026-06-20-classify-support-packages.md index c3947c1c61..3634974a5b 100644 --- a/docs/rfc/proposed/2026-06-20-classify-support-packages.md +++ b/docs/rfc/rejected/2026-06-20-classify-support-packages.md @@ -1,6 +1,6 @@ # RFC: Classify product, integration, and support packages -Status: proposed +Status: rejected — a single product/support taxonomy is too coarse. If package metadata changes, it should be aspect-oriented (`core`, `bash`, `fs`, `persistence`, `example`, `testing`, `publishable`, and similar facets) instead of forcing every package into one hierarchy. ## Problem @@ -12,6 +12,8 @@ This is not just cosmetic. A package's location currently says little about whet Introduce an explicit package classification and move packages accordingly, for example `packages/core/`, `packages/integrations/`, `packages/tools/`, `packages/testing/`, and `packages/examples/`, or an equivalent structure decided in the implementing PR. The important part is that example/test support packages are not indistinguishable from product core. +The rejected part is the one-dimensional taxonomy. The useful follow-up is explicit package aspect metadata that scripts can consume without pretending a package has only one role. + This proposal does not delete `llm-replay` or `ui-stdio` by itself. It makes their status honest: either they graduate into product packages with documented consumers, or they live under a support/testing/example classification where release and compatibility expectations are lower. ## Acceptance criteria diff --git a/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md b/docs/rfc/rejected/2026-06-20-drop-acp-session-load.md similarity index 85% rename from docs/rfc/proposed/2026-06-20-drop-acp-session-load.md rename to docs/rfc/rejected/2026-06-20-drop-acp-session-load.md index fe58449645..5714dc058f 100644 --- a/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md +++ b/docs/rfc/rejected/2026-06-20-drop-acp-session-load.md @@ -1,12 +1,12 @@ # RFC: Drop ACP session/load until resume has a product shape -Status: proposed +Status: rejected — Zed is the current target ACP client, advertises and exercises load-capable sessions, and keeps pending-load state for concurrent `session/load`. The bridge should keep `session/load` and make the resume contract solid. ## Problem ACP advertises `loadSession: true` and implements `session/load` by injecting persistence into the bridge, validating cwd against stored metadata, reconstructing an agent from the persisted log, and replaying prior transcript updates to the client. That path has its own race handling, loading-id guard, replay presenter logic, and tests. It also depends on the canonical log retaining enough UI data to reconstruct old chunks and tool presentations. -Durable persistence remains foundational, but editor-visible resume is not yet a designed product flow. There is no session picker, no title/preview metadata, and no clear UX for failed or partial loads. The bridge is paying complexity for a feature that is mostly exercised by tests and documentation. +Durable persistence remains foundational, but editor-visible resume is not yet a designed product flow. There is no session picker, no title/preview metadata, and no clear UX for failed or partial loads. The bridge is paying complexity for a feature that is exercised by tests, documentation, and the current target client's session model. ## Proposal diff --git a/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md b/docs/rfc/rejected/2026-06-20-drop-acp-terminal-meta.md similarity index 89% rename from docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md rename to docs/rfc/rejected/2026-06-20-drop-acp-terminal-meta.md index 05dacc100b..89b0335275 100644 --- a/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md +++ b/docs/rfc/rejected/2026-06-20-drop-acp-terminal-meta.md @@ -1,12 +1,12 @@ # RFC: Drop ACP terminal `_meta` rendering -Status: proposed +Status: rejected — Zed is the current target client, and the terminal `_meta` convention is intentional Zed UX with a plain ACP fallback for other clients. ## Problem The ACP bridge implements a Zed-specific terminal-card convention through `_meta.terminal_info`, `_meta.terminal_output`, and `_meta.terminal_exit`. The implemented [rich ACP bash rendering RFC](../implemented/2026-06-18-acp-terminal-and-tool-rendering.md) deliberately avoided ACP's client-side `terminal/create` because bash execution belongs in the harness, but still adopted the reference agents' display-only `_meta` convention. That gives a nicer Zed card at the cost of bridge state, capability negotiation, terminal ids, special update mapping, text fallback tests, and exit-pill parsing in `dsh-tool-bash`. -The fallback path already exists: render the tool call and completed output as normal ACP content blocks. Non-Zed clients rely on that path anyway. +The fallback path already exists: render the tool call and completed output as normal ACP content blocks. Non-Zed clients rely on that path anyway, but the Zed terminal card is a current target-client feature rather than speculative decoration. ## Proposal diff --git a/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md b/docs/rfc/rejected/2026-06-20-drop-bash-output-spill-files.md similarity index 79% rename from docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md rename to docs/rfc/rejected/2026-06-20-drop-bash-output-spill-files.md index 9f55e6e6f8..ba533bba6d 100644 --- a/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md +++ b/docs/rfc/rejected/2026-06-20-drop-bash-output-spill-files.md @@ -1,6 +1,6 @@ # RFC: Drop bash full-output spill files -Status: proposed +Status: rejected — full-output recovery is a real bash behavior. A future artifact/blob service may generalize it, but dropping spill files before that replacement would lose useful command output. ## Problem @@ -12,7 +12,7 @@ This solves a real problem, but in a narrow and leaky way. A spill path is a pro Keep tail truncation, drop full-output spill files. A bash result contains the bounded tail plus a clear truncation marker; no path is emitted. If users need full-output recovery, add a generic artifact/blob service with explicit ownership, cleanup, and UI rendering, then let bash attach large outputs to that service. -This proposal can land independently of [foreground-only bash](2026-06-20-foreground-only-bash.md). If background tasks stay, `bash_output` should still report that output was dropped, but without advertising a spill path. +This proposal can land independently of [a generic long-running tool runtime](../proposed/2026-06-20-generic-long-running-tool-runtime.md). If background tasks stay, `bash_output` should still report that output was dropped, but without advertising a spill path. ## Acceptance criteria diff --git a/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md b/docs/rfc/rejected/2026-06-20-drop-unused-session-lineage.md similarity index 79% rename from docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md rename to docs/rfc/rejected/2026-06-20-drop-unused-session-lineage.md index 0a5d1474f7..f32b0f3461 100644 --- a/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md +++ b/docs/rfc/rejected/2026-06-20-drop-unused-session-lineage.md @@ -1,12 +1,12 @@ # RFC: Drop unused session lineage metadata -Status: proposed +Status: rejected — `parentSession` is part of the documented fork/sub-agent seam and is already preserved by the agent/session resume path. The field is future-facing, but it is not accidental dead state. ## Problem `SessionHeader.parentSession` records the session a new session was forked from. It is defined in `dsh-session`, preserved by persistence backends, copied through resume, documented as lineage metadata, and covered by round-trip tests. The repo has no production fork UI or sub-agent flow that reads it. The planned sub-agent/fork seam is still a TODO, so the field is currently stored future shape. -The cost is small per file but broad across the format: every backend schema and metadata serializer preserves a value that no feature uses. Because the header is an on-disk contract, even a placeholder field becomes something future refactors must either maintain, migrate, or deliberately break. +The cost is small per file but broad across the format: every backend schema and metadata serializer preserves a value that no completed feature reads yet. Because the header is an on-disk contract, even a placeholder field becomes something future refactors must either maintain, migrate, or deliberately break. ## Proposal diff --git a/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md b/docs/rfc/rejected/2026-06-20-fold-session-persistence-interface.md similarity index 90% rename from docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md rename to docs/rfc/rejected/2026-06-20-fold-session-persistence-interface.md index bd760e7922..da19617793 100644 --- a/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md +++ b/docs/rfc/rejected/2026-06-20-fold-session-persistence-interface.md @@ -1,6 +1,6 @@ # RFC: Fold the persistence interface into dsh-session -Status: proposed +Status: rejected — the separate persistence interface package is the intended modular capability seam for durable backends. Folding it into `dsh-session` would reduce package count at the cost of a cleaner backend boundary. ## Problem diff --git a/docs/rfc/proposed/2026-06-20-generic-tool-rendering.md b/docs/rfc/rejected/2026-06-20-generic-tool-rendering.md similarity index 93% rename from docs/rfc/proposed/2026-06-20-generic-tool-rendering.md rename to docs/rfc/rejected/2026-06-20-generic-tool-rendering.md index e6bde8f434..5b80920a6a 100644 --- a/docs/rfc/proposed/2026-06-20-generic-tool-rendering.md +++ b/docs/rfc/rejected/2026-06-20-generic-tool-rendering.md @@ -1,6 +1,6 @@ # RFC: Collapse tool-owned UI presentation -Status: proposed +Status: rejected — tool-owned presentation should wait for more real tools before being generalized or deleted. Bash and ACP currently need the existing richer presentation path. ## Problem diff --git a/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md b/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md similarity index 87% rename from docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md rename to docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md index 45092fc1e6..cbfa238370 100644 --- a/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md +++ b/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md @@ -1,6 +1,6 @@ # RFC: Retire mid-turn steering -Status: proposed +Status: rejected — mid-turn steering is an intentional agent capability for between-step user/plugin input and future goal/loop workflows. It is complexity with a product direction, not an accidental duplicate of `send()`. ## Problem @@ -30,4 +30,4 @@ A user cannot add same-turn steering content while a model is between tool steps ## Related -This pairs naturally with [dropping durable step boundaries](2026-06-20-drop-durable-step-boundaries.md), because removing same-turn steering and `agent/turn-continuation` leaves tool calls as the only reason a turn contains multiple model steps. +This pairs naturally with [dropping durable step boundaries](../proposed/2026-06-20-drop-durable-step-boundaries.md), because removing same-turn steering and `agent/turn-continuation` leaves tool calls as the only reason a turn contains multiple model steps. diff --git a/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md b/docs/rfc/rejected/2026-06-20-single-session-acp-bridge.md similarity index 64% rename from docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md rename to docs/rfc/rejected/2026-06-20-single-session-acp-bridge.md index 0558975949..ab26b9c3c9 100644 --- a/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md +++ b/docs/rfc/rejected/2026-06-20-single-session-acp-bridge.md @@ -1,12 +1,12 @@ # RFC: Return the ACP bridge to one live session per connection -Status: proposed +Status: rejected — Zed is the current target ACP client and its ACP implementation is explicitly multi-session: it stores live sessions in a `HashMap`, tracks `pending_sessions`, joins concurrent loads for the same id, and tests close-during-load behavior. ## Problem -The ACP bridge now supports multiple live sessions on one JSON-RPC connection. That capability brings multi-entry session maps, reverse session/agent lookups, per-session prompt state, loading ids, demux for every event, cross-session teardown, and isolation concerns for future permission prompts and background tasks. The older [multi-session ACP proposal](2026-06-14-acp-multi-session.md) still tracks the unfinished permission-ownership piece; this RFC is the competing simplification path. +The ACP bridge now supports multiple live sessions on one JSON-RPC connection. That capability brings multi-entry session maps, reverse session/agent lookups, per-session prompt state, loading ids, demux for every event, cross-session teardown, and isolation concerns for future permission prompts and background tasks. The older [multi-session ACP proposal](../proposed/2026-06-14-acp-multi-session.md) still tracks the unfinished permission-ownership piece; this RFC is the competing simplification path. -The product has not yet proven it needs concurrent editor conversations over one harness process. The snapshot replay tier also avoids concurrent model streams because its replay entries are positional; concurrency would require keying replay by request instead of by stream order. +The product target has proven it needs concurrent editor conversations over one harness process: Zed's ACP connection owns multiple sessions and load states. The snapshot replay tier still avoids concurrent model streams because its replay entries are positional; that is a test-fixture limitation, not a reason to remove bridge multiplexing. ## Proposal @@ -20,7 +20,7 @@ Remove the multi-session maps and demux where a single `SessionRecord | undefine - `session/new` and `session/load` reject while that record exists. - Event handlers no longer demux across a `Map`. - Multi-session tests are removed or moved under the proposal that continues to defend multiplexing. -- The existing [multi-session ACP proposal](2026-06-14-acp-multi-session.md) is updated to link this RFC while both proposals remain live. +- The existing [multi-session ACP proposal](../proposed/2026-06-14-acp-multi-session.md) is updated to link this RFC and remains the live direction. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md b/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md similarity index 90% rename from docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md rename to docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md index 388af09716..410237daa4 100644 --- a/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md +++ b/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md @@ -1,6 +1,6 @@ # RFC: Truncate interrupted final turns on load -Status: proposed +Status: rejected — a single turn can contain substantial real work, including many steps and large tool output. Preserving interrupted turns is preferable to silently dropping that tail on load. ## Problem @@ -29,4 +29,4 @@ A crash can lose real work from the final turn: assistant text, tool calls, and ## Related -This is a direct simplification of [session persistence](../implemented/2026-06-14-session-persistence.md) and [turn enclosure](../implemented/2026-06-15-turn-enclosure-invariant.md). It also removes much of the motivation for durable step boundary events, making [drop durable step boundary events](2026-06-20-drop-durable-step-boundaries.md) smaller. +This is a direct simplification of [session persistence](../implemented/2026-06-14-session-persistence.md) and [turn enclosure](../implemented/2026-06-15-turn-enclosure-invariant.md). It also removes much of the motivation for durable step boundary events, making [drop durable step boundary events](../proposed/2026-06-20-drop-durable-step-boundaries.md) smaller. diff --git a/packages/acp/README.md b/packages/acp/README.md index 23f32b9bd4..2c6d531a60 100644 --- a/packages/acp/README.md +++ b/packages/acp/README.md @@ -1,6 +1,6 @@ # @deepseek-ai/dsh-acp -The **Agent Client Protocol (ACP)** bridge: exposes the DeepSeek Harness coding agent as an ACP server over JSON-RPC stdio, so editors (Zed and other ACP clients) can drive it — streaming render, tool-call display, and resumable sessions. **N concurrent sessions per connection** (see [ACP multi-session](../../docs/rfc/proposed/2026-06-14-acp-multi-session.md)): each maps to its own `ReactLoopAgent`, and every event is demuxed strictly by session id so two sessions streaming at once never interleave. +The **Agent Client Protocol (ACP)** bridge: exposes the DeepSeek Harness coding agent as an ACP server over JSON-RPC stdio, so editors (Zed and other ACP clients) can drive it — streaming render, tool-call display, and resumable sessions. Zed is the current target client: baseline ACP behavior should remain reasonable for other clients, but bridge capabilities and compatibility decisions are evaluated against Zed first. **N concurrent sessions per connection** (see [ACP multi-session](../../docs/rfc/proposed/2026-06-14-acp-multi-session.md)): each maps to its own `ReactLoopAgent`, and every event is demuxed strictly by session id so two sessions streaming at once never interleave. It is a **client-driver / UI plugin**, the structured analogue of the readline `stdio-chat` plugin — NOT a loop change and NOT a [capability seam](../../docs/rfc/implemented/2026-06-13-capability-seams.md). It consumes the existing `agent/*` event taxonomy, the `dsh-agent` create/resume factory, and `dsh-session-persistence`.