From cc47f76cea0f83f0381a1ea71f15101402c99f46 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 16:33:03 +0800 Subject: [PATCH 1/8] docs: propose simplification RFCs --- docs/rfc/README.md | 19 ++++++++++++ ...06-20-assembled-assistant-messages-only.md | 31 +++++++++++++++++++ .../2026-06-20-classify-support-packages.md | 26 ++++++++++++++++ ...6-20-collapse-trace-only-session-events.md | 27 ++++++++++++++++ .../2026-06-20-discover-package-inventory.md | 26 ++++++++++++++++ .../2026-06-20-drop-acp-session-load.md | 25 +++++++++++++++ .../2026-06-20-drop-acp-terminal-meta.md | 27 ++++++++++++++++ ...2026-06-20-drop-bash-output-spill-files.md | 27 ++++++++++++++++ ...2026-06-20-drop-durable-step-boundaries.md | 27 ++++++++++++++++ .../2026-06-20-drop-unused-session-lineage.md | 26 ++++++++++++++++ ...6-20-fold-session-persistence-interface.md | 27 ++++++++++++++++ .../2026-06-20-foreground-only-bash.md | 27 ++++++++++++++++ .../2026-06-20-generic-tool-rendering.md | 31 +++++++++++++++++++ .../2026-06-20-providerless-example-base.md | 27 ++++++++++++++++ .../2026-06-20-public-agent-stop-surface.md | 26 ++++++++++++++++ ...-20-remove-agent-boundary-mirror-events.md | 30 ++++++++++++++++++ ...0-remove-redundant-snapshot-log-goldens.md | 27 ++++++++++++++++ .../2026-06-20-retire-mid-turn-steering.md | 31 +++++++++++++++++++ .../2026-06-20-single-session-acp-bridge.md | 27 ++++++++++++++++ .../2026-06-20-truncate-interrupted-turns.md | 31 +++++++++++++++++++ scripts/publint-all.ts | 1 + 21 files changed, 546 insertions(+) create mode 100644 docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md create mode 100644 docs/rfc/proposed/2026-06-20-classify-support-packages.md create mode 100644 docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md create mode 100644 docs/rfc/proposed/2026-06-20-discover-package-inventory.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-acp-session-load.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md create mode 100644 docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md create mode 100644 docs/rfc/proposed/2026-06-20-foreground-only-bash.md create mode 100644 docs/rfc/proposed/2026-06-20-generic-tool-rendering.md create mode 100644 docs/rfc/proposed/2026-06-20-providerless-example-base.md create mode 100644 docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md create mode 100644 docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md create mode 100644 docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md create mode 100644 docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md create mode 100644 docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md create mode 100644 docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 0fe36dca75..5f2e25ae31 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -32,6 +32,25 @@ 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 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 | +| [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 | +| [Discover package inventories instead of maintaining static lists](proposed/2026-06-20-discover-package-inventory.md) | 2026-06-20 | ## Implemented diff --git a/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md b/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md new file mode 100644 index 0000000000..b7724c105c --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md @@ -0,0 +1,31 @@ +# RFC: Persist assembled assistant messages, not stream chunks + +Status: proposed + +## Problem + +The canonical session log currently persists every `assistant/chunk` exactly as streamed by the model. The persistence RFC chose this for token-level replay fidelity and contiguous `seq`, but the cost has grown: JSONL fixtures are dominated by tiny delta records, snapshot scenarios replay the model by grouping chunk events, ACP load reconstructs prior assistant output from chunks, and any future log reader must distinguish durable message history from token-level trace. + +The loop already appends an assembled `assistant/message` for each step. That is the event `deriveMessages()` uses for the next model request. In other words, the resumable conversation state is already present without the chunks; chunks are a live rendering and deterministic-test artifact, not required conversation history. + +## 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. + +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. + +## Acceptance criteria + +- `SessionEventMap` drops `assistant/chunk`, or marks it as non-persisted if a transitional live event is needed. +- Persistence docs no longer require every stream chunk to be stored verbatim. +- `llm-replay` and ACP snapshots use an explicit replay fixture format or sidecar for model chunks. +- `session/load` renders completed assistant messages from `assistant/message`. +- Stored logs get much smaller and remain `seq`-contiguous without chunk holes. + +## What we give up + +The canonical user session no longer reconstructs the exact token stream of an old turn. 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. + +## Related + +This supersedes the chunk-persistence choice in [session persistence](../implemented/2026-06-14-session-persistence.md) and affects [ACP snapshot tests](../implemented/2026-06-19-acp-snapshot-tests.md), whose current replay plugin derives its script from `assistant/chunk` events. diff --git a/docs/rfc/proposed/2026-06-20-classify-support-packages.md b/docs/rfc/proposed/2026-06-20-classify-support-packages.md new file mode 100644 index 0000000000..13f5a2b04a --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-classify-support-packages.md @@ -0,0 +1,26 @@ +# RFC: Classify product, integration, and support packages + +Status: proposed + +## Problem + +`packages/` is flat. Core product packages, provider integrations, tool implementations, example UI support, and snapshot-only replay support all sit at the same level and look equally publishable. `packages/README.md` already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more foundational than they are and forces publish/lint/doc scripts to special-case intent in prose or static lists. + +This is not just cosmetic. A package's location currently says little about whether it is core API, an integration, an example harness helper, or test infrastructure. That makes future removal harder because every top-level package looks like part of the same public surface. + +## Proposal + +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. + +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 + +- Each package has an explicit classification visible from path or package metadata. +- Scripts that publish, lint publishability, or generate module graphs use the classification instead of an ad hoc static list. +- Docs explain which package classes are part of the product API. +- YAML loader paths and TypeScript path aliases are updated in one coordinated move. + +## What we give up + +The restructure churns imports, workspace globs, docs links, and package paths. That churn is acceptable pre-release if it prevents the flat layout from fossilizing a support package as a product contract. 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 new file mode 100644 index 0000000000..464eb9a154 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-collapse-trace-only-session-events.md @@ -0,0 +1,27 @@ +# RFC: Collapse trace-only session events + +Status: proposed + +## Problem + +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. + +## 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. + +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. + +## Acceptance criteria + +- `SessionEventMap` drops `usage` and `error`, or folds their fields into nearby load-bearing 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`. +- ACP snapshots and persistence tests stop asserting trace-only lines. +- Documentation explains where token usage and operational errors are observed if they remain available. + +## 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). diff --git a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md new file mode 100644 index 0000000000..cafb70a948 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md @@ -0,0 +1,26 @@ +# RFC: Discover package inventories instead of maintaining static lists + +Status: proposed + +## Problem + +Package and gate inventories are repeated by hand. `scripts/publint-all.ts` has a static list of publishable packages. The package cookbook tells authors to update several files. The package README carries a hand-written dependency graph. CI and development docs can drift from the actual `doc-sync` subcommands when new gates are added. These lists are small today, but every new package or gate creates another manual synchronization point. + +Static lists are appropriate when they encode policy; they are needless friction when they duplicate manifest data that already exists in `package.json`, workspace globs, or package metadata. + +## Proposal + +Make package/gate inventories discoverable. Publishability should come from package metadata or classification, not from a static array in a script. 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. + +## Acceptance criteria + +- `publint-all` discovers publishable packages from manifests or a single classification source. +- 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. + +## What we give up + +Discovery scripts can become too clever. The implementation should stay boring: read manifests, filter on explicit fields, print the resolved list, and fail loud. The payoff is removing manual inventory drift, not inventing a build system. diff --git a/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md b/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md new file mode 100644 index 0000000000..f8a0853524 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md @@ -0,0 +1,25 @@ +# RFC: Drop ACP session/load until resume has a product shape + +Status: proposed + +## 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. + +## Proposal + +For now, ACP starts fresh sessions only. `initialize` advertises `loadSession: false` or omits the capability, and `session/load` is unsupported. Persistence remains available to the agent loop and tests; resume can still exist as a lower-level factory if another consumer needs it. The editor bridge should reintroduce `session/load` alongside a real session-selection UX and a stable load transcript contract. + +## Acceptance criteria + +- ACP no longer injects `sessionPersistence` solely for `session/load`. +- `initialize` does not advertise load support. +- The `session/load` handler, loading-id tracking, cwd preflight for loaded sessions, and load replay tests are removed. +- Snapshot fixtures no longer rely on load replay presentation. +- ACP docs describe fresh-session support only. + +## What we give up + +An editor cannot reopen a prior persisted session through ACP. That is a real product feature, but the current implementation is ahead of the UX and ties the bridge to token-level log replay. Keeping persistence while dropping editor load narrows the bridge to the workflow it can currently present cleanly. diff --git a/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md b/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md new file mode 100644 index 0000000000..897f23a2e8 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md @@ -0,0 +1,27 @@ +# RFC: Drop ACP terminal `_meta` rendering + +Status: proposed + +## Problem + +The ACP bridge implements a Zed-specific terminal-card convention through `_meta.terminal_info`, `_meta.terminal_output`, and `_meta.terminal_exit`. The implemented RFC 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. + +## Proposal + +Ignore `clientCapabilities._meta.terminal_output` and render bash results through the plain ACP content path. Keep execution agent-side through `dsh-bash`; only the display-specific terminal metadata is removed. A terminal card can return later if ACP standardizes agent-executed terminals or if the product decides Zed-specific display is worth the maintenance cost. + +This proposal is narrower than [collapsing tool-owned UI presentation](2026-06-20-generic-tool-rendering.md): it keeps generic `presentCall`/`presentResult` if those survive, but removes the terminal sub-shape and `_meta` mapping. + +## Acceptance criteria + +- ACP no longer reads or stores `_meta.terminal_output` capability state. +- `TerminalRendering`, terminal ids, terminal cwd resolution, and `_meta.terminal_*` update mapping disappear from `@deepseek-ai/dsh-acp`. +- `ToolTerminal` disappears from `@deepseek-ai/dsh-tools`, or is unused and deleted with the presentation cleanup. +- Bash result presentation no longer parses exit status for terminal pills. +- The implemented terminal-rendering RFC is superseded or moved to rejected with this proposal linked. + +## What we give up + +Zed users lose the dedicated terminal card: no cwd header, terminal display, or exit pill. They still see the command and output as plain content. That is a reasonable simplification while the ACP bridge is still unreleased and the `_meta` keys are a convention rather than a standard. diff --git a/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md b/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md new file mode 100644 index 0000000000..2df48cd3cc --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md @@ -0,0 +1,27 @@ +# RFC: Drop bash full-output spill files + +Status: proposed + +## Problem + +`dsh-bash-local` keeps bounded in-memory output and spills large stdout/stderr streams into private temp files. That requires a private directory, random owner-only file creation, close-failure handling, byte-offset incremental reads, lossy read reporting, path rendering in model-facing text, and cleanup discipline. The tool then tells the model to read a local spill path when output was truncated. + +This solves a real problem, but in a narrow and leaky way. A spill path is a process-local filesystem artifact exposed to model output, not a durable harness artifact with scoped access, retention, or UI affordances. It also complicates background-task reads because a lossy incremental read has to point at one or two spill files. + +## Proposal + +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. + +## Acceptance criteria + +- `CollectedOutput` no longer carries spill paths. +- `OutputCollector` keeps bounded buffers only and deletes the temp-file machinery. +- `renderResult()` reports truncation without a filesystem path. +- Tests cover tail truncation and no longer assert full-output file contents. +- Security docs stop treating private spill files as a model-visible interface. + +## What we give up + +A model or user cannot recover the omitted prefix of a huge command output from a temp file. That is acceptable until there is a real artifact service. The current spill path is too much bespoke machinery for a feature whose lifecycle and permissions are not designed. 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 new file mode 100644 index 0000000000..356fb20c31 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md @@ -0,0 +1,27 @@ +# RFC: Drop durable step boundary events + +Status: proposed + +## Problem + +The session log stores `step/start` and `step/end` events even though every step-scoped event already carries `{ turn, step }`: assistant chunks, assistant messages, tool calls, tool results, usage, and errors. `deriveMessages()` ignores step boundaries, ACP ignores them for UI, and the main consumers are invariants, tests, snapshot goldens, and crash repair. + +The boundary events make the log more ceremonial than informative. The loop tracks open steps solely to close them, repair synthesizes `step/end` when a crash leaves a step open, invariants track a second nesting stack inside the turn, and snapshots carry lines that do not affect replayed message history. A model request that crashes before producing any step-scoped event is the only information represented by a bare `step/start`, and that case has no useful resumable content. + +## Proposal + +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. + +## Acceptance criteria + +- `SessionEventMap` no longer includes `step/start` or `step/end`. +- The loop has no `closeStep()` finalization path. +- ACP snapshots and persistence contract fixtures stop expecting step-boundary lines. +- `deriveMessages()` and replay derive the same message history from step-scoped events. +- The event taxonomy docs describe turns as the durable boundary and steps as a field on step-scoped records. + +## What we give up + +The log no longer records "a model request started but produced no event before the process died" as a durable fact. That is acceptable: there is no assistant content, tool call, usage, or error to replay from that empty request. A live UI can still show an in-progress step from a transient event if it needs one; the durable log should not store an empty bracket. diff --git a/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md b/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md new file mode 100644 index 0000000000..35fcb3fc63 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md @@ -0,0 +1,26 @@ +# RFC: Drop unused session lineage metadata + +Status: proposed + +## 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. + +## Proposal + +Remove `parentSession` from `SessionHeader` until a real fork/resume feature needs lineage. Forking can still seed a new session with prior events if such an API exists, but the durable parent pointer should be introduced alongside the feature that reads it and the UX that explains it. + +If lineage returns, decide then whether it belongs in the immutable header, a session graph index, or a first-class event. The current field should not pre-commit that design. + +## Acceptance criteria + +- `SessionHeader` contains version, id, createdAt, and optional cwd only. +- JSONL and SQLite metadata schemas stop storing parent-session ids. +- Resume and list APIs no longer round-trip `parentSession`. +- Docs and tests remove fork-lineage claims that are not backed by a production consumer. + +## What we give up + +The codebase loses a ready-made lineage hook for future fork/sub-agent UX. That is intentional. The field is easy to reintroduce when the feature exists, and the unreleased stance lets the format change without migrations. diff --git a/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md b/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md new file mode 100644 index 0000000000..d2e8466d07 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md @@ -0,0 +1,27 @@ +# RFC: Fold the persistence interface into dsh-session + +Status: proposed + +## Problem + +`dsh-session-persistence` is an interface package whose main concepts are already owned by `dsh-session`: `SessionHeader`, `SessionEvent`, `SessionId`, `session/event`, and `session/flush`. The package adds the abstract `SessionPersistence` service, the shared write coordinator, and contract helpers. Backend packages depend on it, and `agent-loop` has to optionally find a sibling service for resume. + +The capability-seam split made sense when persistence was a new swappable backend design. After the mutable summary was removed, the interface package mostly wraps the session log's own storage concern. Keeping it separate may be more ceremony than clarity. + +## Proposal + +Move the abstract `SessionPersistence` service, the coordinator, and persistence contract helpers into `dsh-session`. Keep JSONL and SQLite as separate backend packages that register the session-owned service. This preserves backend swappability while deleting one support package and one cross-package seam. + +The implementing PR should update the [capability seams](../implemented/2026-06-13-capability-seams.md) guidance with the exception: persistence is not like bash or LLM because its vocabulary and lifecycle events are already the session package's core domain. + +## Acceptance criteria + +- `@deepseek-ai/dsh-session-persistence` is removed as a package. +- `dsh-session` exports the persistence service type, coordinator, and contract helpers. +- JSONL and SQLite backend packages depend on `dsh-session` directly. +- `agent-loop` resume uses the session-owned service key. +- Persistence RFCs and package docs explain why backend implementations remain separate. + +## What we give up + +`dsh-session` becomes heavier: it owns both the in-memory log and the persistence interface. That is the trade. If third-party persistence backends were already a public ecosystem, the separate interface package would be a cleaner SDK boundary; pre-release, the extra package looks like abstraction before there is an external consumer. diff --git a/docs/rfc/proposed/2026-06-20-foreground-only-bash.md b/docs/rfc/proposed/2026-06-20-foreground-only-bash.md new file mode 100644 index 0000000000..42401151a1 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-foreground-only-bash.md @@ -0,0 +1,27 @@ +# 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. Recent work added owner-token isolation because global predictable task ids become a cross-session read/kill hazard. + +The cookbook 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 `run_in_background`, `bash_output`, `bash_kill`, background task ownership, incremental task reads, completion injection, and task-listener APIs from the public bash executor seam. 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 cookbook either removes the background example or redirects long-running work to the future generic task RFC. + +## 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-tool-rendering.md b/docs/rfc/proposed/2026-06-20-generic-tool-rendering.md new file mode 100644 index 0000000000..e6bde8f434 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-generic-tool-rendering.md @@ -0,0 +1,31 @@ +# RFC: Collapse tool-owned UI presentation + +Status: proposed + +## Problem + +Tools can define `presentCall()` and `presentResult()` callbacks that return `ToolCallPresentation`, `ToolResultPresentation`, and optional `ToolTerminal` fields. The code itself flags the design as muddy: title, kind, raw input, content, terminal cwd, terminal output, exit code, and signal grew incrementally into a bag of optional fields. ACP then maintains pending call state to pair a result with the original args, creates replay-only presenters on `session/load`, and maps terminal subfields into Zed-specific `_meta`. `dsh-tool-bash` even parses exit status back out of rendered text because the pure replay-safe presenter no longer has the structured `BashRunResult`. + +The real first-party use is bash presentation for ACP. That is too little evidence to freeze a cross-package UI presentation API. + +## Proposal + +Remove tool-owned UI presentation callbacks for now. The canonical tool events already carry the tool name, raw argument string, result content, and error state. UIs render a generic tool card from those fields. Tool-specific rich rendering can return later as a tagged render-intent union after there are at least two real tools and two real consumers to validate the vocabulary. + +As a smaller alternative, replace the current optional-field bag with one explicit union in a single PR; but if the goal is simplification, the stronger move is to delete the callbacks and keep the generic path. + +## Acceptance criteria + +- `ToolDefinition` drops `presentCall` and `presentResult`. +- `ToolCallPresentation`, `ToolResultPresentation`, `ToolTerminal`, and `ToolCallKind` disappear unless a minimal generic UI type still needs one. +- ACP no longer keeps presenter pending state or calls tool callbacks during live streaming/load replay. +- `dsh-tool-bash` no longer parses rendered text to recover exit status for a UI pill. +- Snapshot goldens show generic tool cards and text results. + +## What we give up + +Bash loses its custom terminal-looking card and model-written description placement. The fallback remains reasonable: the command appears as tool input, and the output appears as text. Rich rendering should be designed when the product has enough UI/tool variety to justify a stable presentation contract. + +## Related + +This is the broad version of [dropping ACP terminal metadata](2026-06-20-drop-acp-terminal-meta.md). If this RFC is accepted, that narrower RFC becomes unnecessary. diff --git a/docs/rfc/proposed/2026-06-20-providerless-example-base.md b/docs/rfc/proposed/2026-06-20-providerless-example-base.md new file mode 100644 index 0000000000..cefcccea08 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-providerless-example-base.md @@ -0,0 +1,27 @@ +# RFC: Make the shared example base providerless + +Status: proposed + +## Problem + +The examples have two shared base files: `examples/base-core.yml` is providerless, while `examples/base.yml` includes that core plus the real `llm-deepseek` adapter. Snapshot replay needs the providerless core with `llm-replay`, because loading the real adapter without a key throws. The normal demos need the real adapter. The result is a naming inversion: the file named `base.yml` is not the reusable base for all examples, while the true base is `base-core.yml`. + +The split is understandable, but it makes every config explanation longer. It also leads to awkward test setup like a keyless smoke test carrying a dummy API key so an adapter can boot even though the model is not called. + +## Proposal + +Rename the providerless core to `examples/base.yml` and make adapter selection explicit in each concrete example. The coding and ACP real configs add a tiny `llm-deepseek` include or local block; snapshot config adds `llm-replay`. Delete `base-core.yml`. + +The shared base should contain only provider-neutral services and tools: `llm`, sessions, system prompt, tools, agents, invariants, bash executor, and bash tool schemas. Anything that chooses a model provider belongs at the leaf config. + +## Acceptance criteria + +- `examples/base.yml` is providerless. +- `examples/base-core.yml` is deleted. +- Real demo configs explicitly add the DeepSeek adapter. +- Snapshot replay config includes the same providerless base and its replay adapter. +- README and RFC references stop explaining "base = base-core plus adapter". + +## What we give up + +Real demos lose one layer of convenience: each must opt into the adapter. That is the right default for examples, because adapter choice is the variable part and providerless wiring is the shared product core. 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 new file mode 100644 index 0000000000..dfc2da2677 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-public-agent-stop-surface.md @@ -0,0 +1,26 @@ +# RFC: Keep one public stop primitive + +Status: proposed + +## Problem + +The public `Agent` handle exposes three ways to reason about stopping work: `abort(reason?)`, `cancel(reason?)`, and `whenIdle()`. `abort()` kills only the in-flight step and leaves queued work alone; `cancel()` clears queued and steering work, aborts the running step, and handles the pre-step race; `whenIdle()` exposes the loop's private quiescence waiter to any consumer. In production, ACP uses `cancel()` for `session/cancel`, while lifecycle owners tear down agents through `AgentHandle.dispose()`. No production caller needs bare `abort()` or `whenIdle()`. + +The extra surface area makes the loop carry public semantics that are mostly teardown internals. `whenIdle()` needs waiter state, special disposed-agent behavior, and a loop-exit promise so it resolves after quiescence rather than merely after a status flip. `abort()` has to be documented as distinct from queue-aware cancellation even though a UI cancellation almost always wants the broader operation. + +## Proposal + +Keep `cancel()` as the only public stop primitive on `Agent`. Lifecycle owners use `AgentHandle.dispose()` to stop and unregister an agent; non-owners use `cancel()` to abandon current and queued work. The implementation can keep private abort controllers and quiescence promises, but they are not part of the plugin-facing `Agent` contract. + +Delete public `abort()` and `whenIdle()`, the tests that exercise them as standalone API, and the docs that describe step-only abort as an embedding feature. The disposer remains async and still waits for the loop to stop; that guarantee moves entirely onto `AgentHandle.dispose()`. + +## Acceptance criteria + +- `Agent` exposes `send()`, `inject()`, `cancel()`, status, options, session, and identity, with no public `abort()` or `whenIdle()`. +- 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. + +## What we give up + +A future plugin cannot abort only the current model/tool step while preserving queued prompts through the public interface. If that use case becomes real, it should return with a named consumer and a narrower contract. Today it is latent generality that keeps private loop mechanics public. 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 new file mode 100644 index 0000000000..7b2aa2d748 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-remove-agent-boundary-mirror-events.md @@ -0,0 +1,30 @@ +# RFC: Stop mirroring durable boundaries as agent events + +Status: proposed + +## Problem + +The loop records the canonical transcript in `SessionEvent` and also emits a parallel set of live `agent/*` mirror events: `agent/turn-start`, `agent/turn-end`, `agent/step-start`, `agent/step-end`, `agent/queued`, and `agent/steering`. The mirrors make consumers choose between two sources of truth. ACP already chose the session log for the editor-facing transcript because a throwing peer listener can prevent later `agent/*` listeners from observing a boundary, while the session event was already appended. The stdio UI is the only production consumer that still renders primarily from the mirror stream. + +This duplication is not free. Every lifecycle change has to update the session event, the mirror event, docs, invariants, tests, and snapshot expectations. The duplicate boundary events also make failure ordering subtle: a turn can be durably closed before a live `agent/turn-end` listener runs, so a post-boundary listener failure has no valid in-log position left and must be reported out of band. + +## Proposal + +Make `session/event` the live transcript stream. Consumers that render turns, tool calls, tool results, assistant messages, and durable boundaries subscribe to `session/event` and derive their UI from the same event vocabulary persistence uses. Keep agent lifecycle/control events that are not transcript data: `agent/created`, `agent/disposed`, `agent/status`, and `agent/error`. Keep any live-only token stream only if the canonical log separately stops storing chunks; otherwise `assistant/chunk` session events cover that too. + +Remove the duplicate durable-boundary mirrors from the agent event taxonomy. If a UI wants an agent handle from a session event, it can keep a small map from session id to agent built from `agent/created`/`agent/disposed`, or the registry can offer an explicit lookup. The canonical record remains the event-sourced session log. + +## Acceptance criteria + +- ACP and stdio render transcript content from `session/event`. +- `agent/turn-start`, `agent/turn-end`, `agent/step-start`, `agent/step-end`, and `agent/steering` are removed or reduced to private implementation details. +- Tests assert the persisted event stream, not a second mirror stream, for turn and step ordering. +- Documentation presents `SessionEvent` as both the durable source and the live transcript feed. + +## What we give up + +A plugin can no longer observe turn/step boundaries from a convenient `Agent`-first event. It must either subscribe to `session/event` or maintain a session-to-agent association. That is an acceptable trade: transcript consumers should not depend on a second event feed that can drift from the durable log. + +## 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. 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 new file mode 100644 index 0000000000..e4dc94e2be --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md @@ -0,0 +1,27 @@ +# RFC: Remove redundant recorded snapshot log goldens + +Status: proposed + +## Problem + +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. + +## 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. + +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`. +- 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. +- The snapshot-test RFC is updated to describe the reduced fixture set. + +## What we give up + +Reviewers lose one redundant artifact that made the expected persisted log visually separate from the replay fixture. The stdout golden still protects the editor transcript, and comparing replay output to the recorded fixture preserves the loop/persistence regression check without duplicating files. diff --git a/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md b/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md new file mode 100644 index 0000000000..6b699eb771 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md @@ -0,0 +1,31 @@ +# RFC: Retire mid-turn steering + +Status: proposed + +## Problem + +The agent exposes two user-message paths that look close but have different lifecycle semantics: `send()` queues a normal user turn, while `steer()` injects a message between steps of the currently running turn and falls back to `send()` when idle. That distinction leaks through the whole stack: `Agent.steer()` is public API, the session log has a durable `steering/message` event, the agent event taxonomy has `agent/steering`, the loop maintains a steering FIFO beside the queued-message FIFO, cancellation clears both queues, and `deriveMessages()` has to render steering as a tagged synthetic user message rather than a normal prompt. + +The continuation seam amplifies the cost. `agent/turn-continuation` defaults to `hadToolCalls || steeringInjected`, so a same-turn steering message can force the loop to call the model again even if the model did not ask for tools. The comments name future `/goal`, `/loop`, and budget-guard uses, but the current repo has no production listener. The only production UI that mentions steering is the stdio demo; ACP already sends prompts through the ordinary queue while a turn is running. + +## Proposal + +Delete mid-turn user steering for now. `Agent.send()` becomes the single public way to submit user content; when the agent is running, the content waits for the next turn. The loop continues within a turn only for tool calls, not because a user typed while a step was running. A caller that wants to interrupt the current turn uses `cancel()` and then `send()`. + +Remove `Agent.steer()`, the steering FIFO, `steering/message`, `agent/steering`, steering-derived continuation, and the cancellation logic that distinguishes queued messages from steering messages. Revisit `agent/turn-continuation` at the same time: if there is still no production listener, remove the waterfall too and let the loop continue only on the closed set of reasons it owns. If a real budget or goal plugin later needs forced continuation, it should reintroduce a narrower seam with that plugin as the concrete consumer. + +## Acceptance criteria + +- `Agent` exposes one user-message entry point, `send()`. +- The durable session event vocabulary no longer contains `steering/message`. +- `deriveMessages()` renders normal user messages and context injections, with no steering tag path. +- The loop has one queued-message FIFO and no same-turn user-message continuation path. +- The stdio UI and docs describe input while running as queued next-turn input. + +## What we give up + +A user cannot add same-turn steering content while a model is between tool steps. That behavior is useful in theory for "while you are already working, also consider X", but it is not the behavior ACP exposes today and it makes the turn boundary much harder to reason about. The simpler behavior is reasonable: user input becomes the next prompt, and cancellation remains the explicit tool for replacing in-flight work. + +## Related + +This pairs naturally with [dropping durable step boundaries](2026-06-20-drop-durable-step-boundaries.md), because removing same-turn steering 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/proposed/2026-06-20-single-session-acp-bridge.md new file mode 100644 index 0000000000..6c3c88a2cc --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md @@ -0,0 +1,27 @@ +# RFC: Return the ACP bridge to one live session per connection + +Status: proposed + +## 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. A separate proposed RFC still tracks the unfinished permission-ownership piece. + +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. + +## Proposal + +Scope ACP back to one live session per connection. `session/new` or `session/load` creates the only session record; a second live session request is rejected until the existing session is disposed or the connection closes. If editors need multiple chat tabs, they can launch multiple agent subprocesses until the bridge has a concrete multi-session UX and permission model. + +Remove the multi-session maps and demux where a single `SessionRecord | undefined` is enough. The bridge can still keep the agent/session lifecycle seams that make disposal correct; the simplification is only about multiplexing more than one active session through the same transport. + +## Acceptance criteria + +- ACP has one active session record per connection. +- `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 to a rejected/superseded proposal. +- The existing [multi-session ACP proposal](2026-06-14-acp-multi-session.md) is updated to link this RFC if rejected. + +## What we give up + +An ACP client cannot host several concurrent conversations on one server process. That is a meaningful capability cut. The simpler model is still reasonable for an unreleased harness: one editor conversation maps to one agent process, and cross-session permission/background-task isolation stops being a live correctness burden. diff --git a/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md b/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md new file mode 100644 index 0000000000..a0a16031c5 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md @@ -0,0 +1,31 @@ +# RFC: Truncate interrupted final turns on load + +Status: proposed + +## Problem + +The current persistence contract preserves a final turn that was durably written but never closed. On load, `interruptedTurnClosers()` scans the tail, synthesizes error `tool/result` events for unanswered tool calls, appends a `step/end` when a step is open, appends `turn/end { kind: 'interrupted' }`, and asks the backend to durably commit that repair. The coordinator, JSONL backend, SQLite backend, session event vocabulary, invariants, docs, and tests all model this synthetic close path. + +This is a lot of machinery to preserve partial work from the last crashed turn. It also invents events that never happened. A synthetic tool result is useful because it makes provider history valid, but it also means the resumed log contains model-visible text that no tool produced. The current design optimizes for maximum tail preservation before there is a released product or a real resume UX that proves partial-turn recovery matters. + +## Proposal + +On load, keep only the last completed turn. A backend still tolerates and truncates a torn final record, but if the parsed durable prefix ends after an open `turn/start`, the canonical repair is to drop every event after the previous `turn/end`. No synthetic `tool/result`, no synthetic `step/end`, no `turn/end { interrupted }`, and no `interrupted` turn-end reason. + +This makes the persisted turn boundary simple: a completed `turn/end` is the checkpoint. Anything after the last checkpoint is crash tail. The next prompt resumes from the last known-valid provider transcript, not from a partially reconstructed final turn. + +## Acceptance criteria + +- `TurnEndReasonMap` drops the `interrupted` variant. +- `interruptedTurnClosers()` and its tests disappear. +- The persistence coordinator's repair hook truncates backend-specific torn/open tail state without appending closers. +- Persistence docs say load returns the last completed turn, plus no partial final turn. +- Snapshot and contract tests update together with the behavior they pin. + +## What we give up + +A crash can lose real work from the final turn: assistant text, tool calls, and tool output appended after the previous `turn/end`. That is the deliberate simplification. The product is unreleased, the final-turn recovery semantics are not user-proven, and a clean completed-turn checkpoint is much easier to explain, test, and implement. A future "recover partial crashed work" feature should be designed as an explicit user-facing recovery view, not as synthetic events silently inserted into the canonical transcript. + +## 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. diff --git a/scripts/publint-all.ts b/scripts/publint-all.ts index 0b8f7870c7..376f02c9df 100644 --- a/scripts/publint-all.ts +++ b/scripts/publint-all.ts @@ -3,6 +3,7 @@ import { resolve } from 'node:path' // publint every publishable package (vendor/ is private upstream code and // examples/ are not packages; both are out of scope). +// TODO(package-inventory): derive this from package metadata/classification. const packages = [ 'packages/llm', 'packages/session', From ea3f138ae971ba2ca5d64127d4ab3176dbf8d6ea Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 17:26:23 +0800 Subject: [PATCH 2/8] docs: address simplification RFC review --- docs/rfc/proposed/2026-06-14-acp-multi-session.md | 2 ++ .../2026-06-20-assembled-assistant-messages-only.md | 11 ++++++----- .../proposed/2026-06-20-classify-support-packages.md | 2 +- .../2026-06-20-collapse-trace-only-session-events.md | 1 + .../proposed/2026-06-20-discover-package-inventory.md | 6 +++--- docs/rfc/proposed/2026-06-20-drop-acp-session-load.md | 2 +- .../rfc/proposed/2026-06-20-drop-acp-terminal-meta.md | 4 ++-- .../2026-06-20-drop-bash-output-spill-files.md | 2 +- .../2026-06-20-drop-durable-step-boundaries.md | 3 ++- .../2026-06-20-drop-unused-session-lineage.md | 1 + .../2026-06-20-fold-session-persistence-interface.md | 2 +- docs/rfc/proposed/2026-06-20-foreground-only-bash.md | 8 ++++---- .../proposed/2026-06-20-providerless-example-base.md | 10 +++++----- .../proposed/2026-06-20-public-agent-stop-surface.md | 6 +++++- .../2026-06-20-remove-agent-boundary-mirror-events.md | 5 +++-- ...026-06-20-remove-redundant-snapshot-log-goldens.md | 2 +- .../proposed/2026-06-20-retire-mid-turn-steering.md | 8 +++++--- .../proposed/2026-06-20-single-session-acp-bridge.md | 6 +++--- .../proposed/2026-06-20-truncate-interrupted-turns.md | 3 ++- scripts/publint-all.ts | 2 +- 20 files changed, 50 insertions(+), 36 deletions(-) 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 2eadf3eb63..33d4a7c177 100644 --- a/docs/rfc/proposed/2026-06-14-acp-multi-session.md +++ b/docs/rfc/proposed/2026-06-14-acp-multi-session.md @@ -5,6 +5,8 @@ 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. + ## Problem [ACP support](2026-06-14-acp-agent-client-protocol.md) ships with a single active session per connection: a second `session/new` is rejected. Editors expect to run several conversations over one agent subprocess — a user opens multiple threads, or a client pre-warms sessions. The single-session guard is a deliberate MVP scope cut, not an architectural limit; this RFC lifts it. diff --git a/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md b/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md index b7724c105c..ecbd06a734 100644 --- a/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md +++ b/docs/rfc/proposed/2026-06-20-assembled-assistant-messages-only.md @@ -4,27 +4,28 @@ Status: proposed ## Problem -The canonical session log currently persists every `assistant/chunk` exactly as streamed by the model. The persistence RFC chose this for token-level replay fidelity and contiguous `seq`, but the cost has grown: JSONL fixtures are dominated by tiny delta records, snapshot scenarios replay the model by grouping chunk events, ACP load reconstructs prior assistant output from chunks, and any future log reader must distinguish durable message history from token-level trace. +The canonical session log currently persists every `assistant/chunk` exactly as streamed by the model. The [session persistence RFC](../implemented/2026-06-14-session-persistence.md) chose this for token-level replay fidelity and contiguous `seq`, but the cost has grown: JSONL fixtures are dominated by tiny delta records, snapshot scenarios replay the model by grouping chunk events, ACP load reconstructs prior assistant output from chunks, and any future log reader must distinguish durable message history from token-level trace. -The loop already appends an assembled `assistant/message` for each step. That is the event `deriveMessages()` uses for the next model request. In other words, the resumable conversation state is already present without the chunks; chunks are a live rendering and deterministic-test artifact, not required conversation history. +For successful steps that assemble completed content, the loop already appends an `assistant/message`. That is the event `deriveMessages()` uses for the next model request. In other words, the normal resumable conversation state is already present without the chunks; chunks are a live rendering and deterministic-test artifact, not required conversation history. Failed or aborted streams are different: partial assistant output may exist only as chunks, and empty max-token steps may produce no `assistant/message` at all. ## 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. +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. 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. ## Acceptance criteria - `SessionEventMap` drops `assistant/chunk`, or marks it as non-persisted if a transitional live event is needed. -- Persistence docs no longer require every stream chunk to be stored verbatim. +- [Session persistence docs](../../../packages/session-persistence/README.md) no longer require every stream chunk to be stored verbatim. - `llm-replay` and ACP snapshots use an explicit replay fixture format or sidecar for model chunks. - `session/load` renders completed assistant messages from `assistant/message`. - Stored logs get much smaller and remain `seq`-contiguous without chunk holes. +- 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 -The canonical user session no longer reconstructs the exact token stream of an old turn. 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 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. ## Related diff --git a/docs/rfc/proposed/2026-06-20-classify-support-packages.md b/docs/rfc/proposed/2026-06-20-classify-support-packages.md index 13f5a2b04a..c3947c1c61 100644 --- a/docs/rfc/proposed/2026-06-20-classify-support-packages.md +++ b/docs/rfc/proposed/2026-06-20-classify-support-packages.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -`packages/` is flat. Core product packages, provider integrations, tool implementations, example UI support, and snapshot-only replay support all sit at the same level and look equally publishable. `packages/README.md` already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more foundational than they are and forces publish/lint/doc scripts to special-case intent in prose or static lists. +`packages/` is flat. Core product packages, provider integrations, tool implementations, example UI support, and snapshot-only replay support all sit at the same level and look equally publishable. The [package README](../../../packages/README.md) already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more foundational than they are and forces publish/lint/doc scripts to special-case intent in prose or static lists. This is not just cosmetic. A package's location currently says little about whether it is core API, an integration, an example harness helper, or test infrastructure. That makes future removal harder because every top-level package looks like part of the same public surface. 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 464eb9a154..143f3e822a 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 @@ -21,6 +21,7 @@ If analytics become real, add a projection helper or a dedicated telemetry store - The loop records durable failures only as `turn/end { kind: 'error' }` 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. +- 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 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 cafb70a948..f346b8bb91 100644 --- a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md @@ -4,19 +4,19 @@ Status: proposed ## Problem -Package and gate inventories are repeated by hand. `scripts/publint-all.ts` has a static list of publishable packages. The package cookbook tells authors to update several files. The package README carries a hand-written dependency graph. CI and development docs can drift from the actual `doc-sync` subcommands when new gates are added. These lists are small today, but every new package or gate creates another manual synchronization point. +Package and gate inventories are repeated by hand. [scripts/publint-all.ts](../../../scripts/publint-all.ts) has a static list of publishable packages. 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. These lists are small today, but every new package or gate creates another manual synchronization point. Static lists are appropriate when they encode policy; they are needless friction when they duplicate manifest data that already exists in `package.json`, workspace globs, or package metadata. ## Proposal -Make package/gate inventories discoverable. Publishability should come from package metadata or classification, not from a static array in a script. 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 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. 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. ## Acceptance criteria -- `publint-all` discovers publishable packages from manifests or a single classification source. +- `publint-all` discovers publishable packages from manifests plus a single classification source. - 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-acp-session-load.md b/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md index f8a0853524..fe58449645 100644 --- a/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md +++ b/docs/rfc/proposed/2026-06-20-drop-acp-session-load.md @@ -18,7 +18,7 @@ For now, ACP starts fresh sessions only. `initialize` advertises `loadSession: f - `initialize` does not advertise load support. - The `session/load` handler, loading-id tracking, cwd preflight for loaded sessions, and load replay tests are removed. - Snapshot fixtures no longer rely on load replay presentation. -- ACP docs describe fresh-session support only. +- [ACP docs](../../../packages/acp/README.md) describe fresh-session support only. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md b/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md index 897f23a2e8..05dacc100b 100644 --- a/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md +++ b/docs/rfc/proposed/2026-06-20-drop-acp-terminal-meta.md @@ -4,7 +4,7 @@ Status: proposed ## Problem -The ACP bridge implements a Zed-specific terminal-card convention through `_meta.terminal_info`, `_meta.terminal_output`, and `_meta.terminal_exit`. The implemented RFC 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 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. @@ -20,7 +20,7 @@ This proposal is narrower than [collapsing tool-owned UI presentation](2026-06-2 - `TerminalRendering`, terminal ids, terminal cwd resolution, and `_meta.terminal_*` update mapping disappear from `@deepseek-ai/dsh-acp`. - `ToolTerminal` disappears from `@deepseek-ai/dsh-tools`, or is unused and deleted with the presentation cleanup. - Bash result presentation no longer parses exit status for terminal pills. -- The implemented terminal-rendering RFC is superseded or moved to rejected with this proposal linked. +- The implemented [rich ACP bash rendering RFC](../implemented/2026-06-18-acp-terminal-and-tool-rendering.md) stays in `implemented/` as shipped history and is cross-linked from this proposal if superseded. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md b/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md index 2df48cd3cc..9f55e6e6f8 100644 --- a/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md +++ b/docs/rfc/proposed/2026-06-20-drop-bash-output-spill-files.md @@ -20,7 +20,7 @@ This proposal can land independently of [foreground-only bash](2026-06-20-foregr - `OutputCollector` keeps bounded buffers only and deletes the temp-file machinery. - `renderResult()` reports truncation without a filesystem path. - Tests cover tail truncation and no longer assert full-output file contents. -- Security docs stop treating private spill files as a model-visible interface. +- Security guidance in [root AGENTS.md](../../../AGENTS.md) stops treating private spill files as a model-visible interface. ## What we give up 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 356fb20c31..4bb5938b59 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 @@ -20,7 +20,8 @@ The invariants plugin should enforce that step-scoped events have valid positive - The loop has no `closeStep()` finalization path. - ACP snapshots and persistence contract fixtures stop expecting step-boundary lines. - `deriveMessages()` and replay derive the same message history from step-scoped events. -- The event taxonomy docs describe turns as the durable boundary and steps as a field on step-scoped records. +- The [event taxonomy docs](../../architecture.md) describe turns as the durable boundary and steps as a field on step-scoped records. +- 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 diff --git a/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md b/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md index 35fcb3fc63..0a5d1474f7 100644 --- a/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md +++ b/docs/rfc/proposed/2026-06-20-drop-unused-session-lineage.md @@ -20,6 +20,7 @@ If lineage returns, decide then whether it belongs in the immutable header, a se - JSONL and SQLite metadata schemas stop storing parent-session ids. - Resume and list APIs no longer round-trip `parentSession`. - Docs and tests remove fork-lineage claims that are not backed by a production consumer. +- The session format version, backend schema versions, and recorded fixtures are refreshed as needed; non-current stored data is rejected per the pre-release format policy, with no migration path. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md b/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md index d2e8466d07..bd760e7922 100644 --- a/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md +++ b/docs/rfc/proposed/2026-06-20-fold-session-persistence-interface.md @@ -20,7 +20,7 @@ The implementing PR should update the [capability seams](../implemented/2026-06- - `dsh-session` exports the persistence service type, coordinator, and contract helpers. - JSONL and SQLite backend packages depend on `dsh-session` directly. - `agent-loop` resume uses the session-owned service key. -- Persistence RFCs and package docs explain why backend implementations remain separate. +- [Session persistence](../implemented/2026-06-14-session-persistence.md), [shared persistence write coordinator](../implemented/2026-06-18-shared-persistence-write-coordinator.md), and [package docs](../../../packages/session-persistence/README.md) explain why backend implementations remain separate. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-foreground-only-bash.md b/docs/rfc/proposed/2026-06-20-foreground-only-bash.md index 42401151a1..3197e916ca 100644 --- a/docs/rfc/proposed/2026-06-20-foreground-only-bash.md +++ b/docs/rfc/proposed/2026-06-20-foreground-only-bash.md @@ -4,13 +4,13 @@ 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. Recent work added owner-token isolation because global predictable task ids become a cross-session read/kill hazard. +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 cookbook 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`. +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 `run_in_background`, `bash_output`, `bash_kill`, background task ownership, incremental task reads, completion injection, and task-listener APIs from the public bash executor seam. 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. +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. @@ -20,7 +20,7 @@ If long-running tasks return later, implement them once as a capability-agnostic - `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 cookbook either removes the background example or redirects long-running work to the future generic task RFC. +- 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 diff --git a/docs/rfc/proposed/2026-06-20-providerless-example-base.md b/docs/rfc/proposed/2026-06-20-providerless-example-base.md index cefcccea08..a824dd4fa1 100644 --- a/docs/rfc/proposed/2026-06-20-providerless-example-base.md +++ b/docs/rfc/proposed/2026-06-20-providerless-example-base.md @@ -4,23 +4,23 @@ Status: proposed ## Problem -The examples have two shared base files: `examples/base-core.yml` is providerless, while `examples/base.yml` includes that core plus the real `llm-deepseek` adapter. Snapshot replay needs the providerless core with `llm-replay`, because loading the real adapter without a key throws. The normal demos need the real adapter. The result is a naming inversion: the file named `base.yml` is not the reusable base for all examples, while the true base is `base-core.yml`. +The examples have two shared base files: [examples/base-core.yml](../../../examples/base-core.yml) is providerless, while [examples/base.yml](../../../examples/base.yml) includes that core plus the real `llm-deepseek` adapter. Snapshot replay needs the providerless core with `llm-replay`, because loading the real adapter without a key throws. The normal demos need the real adapter. The result is a naming inversion: the file named `base.yml` is not the reusable base for all examples, while the true base is `base-core.yml`. The split is understandable, but it makes every config explanation longer. It also leads to awkward test setup like a keyless smoke test carrying a dummy API key so an adapter can boot even though the model is not called. ## Proposal -Rename the providerless core to `examples/base.yml` and make adapter selection explicit in each concrete example. The coding and ACP real configs add a tiny `llm-deepseek` include or local block; snapshot config adds `llm-replay`. Delete `base-core.yml`. +Rename the providerless core to [examples/base.yml](../../../examples/base.yml) and make adapter selection explicit in each concrete example. The coding and ACP real configs add a tiny `llm-deepseek` include or local block; snapshot config adds `llm-replay`. Delete [examples/base-core.yml](../../../examples/base-core.yml). The shared base should contain only provider-neutral services and tools: `llm`, sessions, system prompt, tools, agents, invariants, bash executor, and bash tool schemas. Anything that chooses a model provider belongs at the leaf config. ## Acceptance criteria -- `examples/base.yml` is providerless. -- `examples/base-core.yml` is deleted. +- [examples/base.yml](../../../examples/base.yml) is providerless. +- [examples/base-core.yml](../../../examples/base-core.yml) is deleted. - Real demo configs explicitly add the DeepSeek adapter. - Snapshot replay config includes the same providerless base and its replay adapter. -- README and RFC references stop explaining "base = base-core plus adapter". +- The [examples README](../../../examples/README.md), example-specific READMEs, and RFC references stop explaining "base = base-core plus adapter". ## What we give up 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 dfc2da2677..e793b7bde6 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 @@ -16,7 +16,7 @@ Delete public `abort()` and `whenIdle()`, the tests that exercise them as standa ## Acceptance criteria -- `Agent` exposes `send()`, `inject()`, `cancel()`, status, options, session, and identity, with no public `abort()` or `whenIdle()`. +- `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. - 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. @@ -24,3 +24,7 @@ Delete public `abort()` and `whenIdle()`, the tests that exercise them as standa ## What we give up A future plugin cannot abort only the current model/tool step while preserving queued prompts through the public interface. If that use case becomes real, it should return with a named consumer and a narrower contract. Today it is latent generality that keeps private loop mechanics public. + +## 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. 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 7b2aa2d748..cbb299dfca 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 @@ -4,13 +4,13 @@ Status: proposed ## Problem -The loop records the canonical transcript in `SessionEvent` and also emits a parallel set of live `agent/*` mirror events: `agent/turn-start`, `agent/turn-end`, `agent/step-start`, `agent/step-end`, `agent/queued`, and `agent/steering`. The mirrors make consumers choose between two sources of truth. ACP already chose the session log for the editor-facing transcript because a throwing peer listener can prevent later `agent/*` listeners from observing a boundary, while the session event was already appended. The stdio UI is the only production consumer that still renders primarily from the mirror stream. +The loop records the canonical transcript in `SessionEvent` and also emits a parallel set of live `agent/*` mirror events: `agent/turn-start`, `agent/turn-end`, `agent/step-start`, `agent/step-end`, `agent/stream-chunk`, and `agent/steering`. The mirrors make consumers choose between two sources of truth. ACP already chose the session log for the editor-facing transcript because a throwing peer listener can prevent later `agent/*` listeners from observing a boundary, while the session event was already appended. The stdio UI is the only production consumer that still renders turn boundaries and the token stream from the mirror events; it already renders tool calls and results from `session/event`. This duplication is not free. Every lifecycle change has to update the session event, the mirror event, docs, invariants, tests, and snapshot expectations. The duplicate boundary events also make failure ordering subtle: a turn can be durably closed before a live `agent/turn-end` listener runs, so a post-boundary listener failure has no valid in-log position left and must be reported out of band. ## Proposal -Make `session/event` the live transcript stream. Consumers that render turns, tool calls, tool results, assistant messages, and durable boundaries subscribe to `session/event` and derive their UI from the same event vocabulary persistence uses. Keep agent lifecycle/control events that are not transcript data: `agent/created`, `agent/disposed`, `agent/status`, and `agent/error`. Keep any live-only token stream only if the canonical log separately stops storing chunks; otherwise `assistant/chunk` session events cover that too. +Make `session/event` the live transcript stream. Consumers that render turns, tool calls, tool results, assistant messages, and durable boundaries subscribe to `session/event` and derive their UI from the same event vocabulary persistence uses. Keep agent lifecycle/control events that are not transcript data: `agent/created`, `agent/disposed`, `agent/status`, `agent/error`, and `agent/queued`. `agent/queued` is an inbox acknowledgement rather than a transcript mirror: it fires before any durable event exists, and cancelled queued work may never enter the log. Remove the duplicate durable-boundary mirrors from the agent event taxonomy. If a UI wants an agent handle from a session event, it can keep a small map from session id to agent built from `agent/created`/`agent/disposed`, or the registry can offer an explicit lookup. The canonical record remains the event-sourced session log. @@ -18,6 +18,7 @@ Remove the duplicate durable-boundary mirrors from the agent event taxonomy. If - ACP and stdio render transcript content from `session/event`. - `agent/turn-start`, `agent/turn-end`, `agent/step-start`, `agent/step-end`, and `agent/steering` are removed or reduced to private implementation details. +- `agent/queued` is either retained and documented as live-only inbox/control state, or deleted in a separate proposal that names the queue-acknowledgement capability loss. - Tests assert the persisted event stream, not a second mirror stream, for turn and step ordering. - Documentation presents `SessionEvent` as both the durable source and the live transcript feed. 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 e4dc94e2be..aedb1b9a40 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 @@ -20,7 +20,7 @@ Stdout goldens remain unchanged; they are the editor-facing projection and are n - 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. -- The snapshot-test RFC is updated to describe the reduced fixture set. +- The [ACP snapshot tests RFC](../implemented/2026-06-19-acp-snapshot-tests.md) is updated to describe the reduced fixture set. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md b/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md index 6b699eb771..45092fc1e6 100644 --- a/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md +++ b/docs/rfc/proposed/2026-06-20-retire-mid-turn-steering.md @@ -6,13 +6,13 @@ Status: proposed The agent exposes two user-message paths that look close but have different lifecycle semantics: `send()` queues a normal user turn, while `steer()` injects a message between steps of the currently running turn and falls back to `send()` when idle. That distinction leaks through the whole stack: `Agent.steer()` is public API, the session log has a durable `steering/message` event, the agent event taxonomy has `agent/steering`, the loop maintains a steering FIFO beside the queued-message FIFO, cancellation clears both queues, and `deriveMessages()` has to render steering as a tagged synthetic user message rather than a normal prompt. -The continuation seam amplifies the cost. `agent/turn-continuation` defaults to `hadToolCalls || steeringInjected`, so a same-turn steering message can force the loop to call the model again even if the model did not ask for tools. The comments name future `/goal`, `/loop`, and budget-guard uses, but the current repo has no production listener. The only production UI that mentions steering is the stdio demo; ACP already sends prompts through the ordinary queue while a turn is running. +The continuation seam amplifies the cost. `agent/turn-continuation` defaults to `hadToolCalls || steeringInjected`, so a same-turn steering message can force the loop to call the model again even if the model did not ask for tools. The comments name future `/goal`, `/loop`, and budget-guard uses, but the current repo has no production listener; only tests register the waterfall. Separately, the only production UI that calls `steer()` is the stdio demo. ACP already sends prompts through the ordinary queue while a turn is running. ## Proposal Delete mid-turn user steering for now. `Agent.send()` becomes the single public way to submit user content; when the agent is running, the content waits for the next turn. The loop continues within a turn only for tool calls, not because a user typed while a step was running. A caller that wants to interrupt the current turn uses `cancel()` and then `send()`. -Remove `Agent.steer()`, the steering FIFO, `steering/message`, `agent/steering`, steering-derived continuation, and the cancellation logic that distinguishes queued messages from steering messages. Revisit `agent/turn-continuation` at the same time: if there is still no production listener, remove the waterfall too and let the loop continue only on the closed set of reasons it owns. If a real budget or goal plugin later needs forced continuation, it should reintroduce a narrower seam with that plugin as the concrete consumer. +Remove `Agent.steer()`, the steering FIFO, `steering/message`, `agent/steering`, steering-derived continuation, and the cancellation logic that distinguishes queued messages from steering messages. Remove `agent/turn-continuation` in the same change unless the implementing PR discovers a production listener; without steering, the current repo has no concrete continuation consumer left. If a real budget or goal plugin later needs forced continuation, it should reintroduce a narrower seam with that plugin as the concrete consumer. ## Acceptance criteria @@ -20,7 +20,9 @@ Remove `Agent.steer()`, the steering FIFO, `steering/message`, `agent/steering`, - The durable session event vocabulary no longer contains `steering/message`. - `deriveMessages()` renders normal user messages and context injections, with no steering tag path. - The loop has one queued-message FIFO and no same-turn user-message continuation path. +- `agent/turn-continuation` is removed or narrowed to a named production consumer. - The stdio UI and docs describe input while running as queued next-turn input. +- 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 @@ -28,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 leaves tool calls as the only reason a turn contains multiple model steps. +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. diff --git a/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md b/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md index 6c3c88a2cc..0558975949 100644 --- a/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md +++ b/docs/rfc/proposed/2026-06-20-single-session-acp-bridge.md @@ -4,7 +4,7 @@ Status: proposed ## 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. A separate proposed RFC still tracks the unfinished permission-ownership piece. +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 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. @@ -19,8 +19,8 @@ Remove the multi-session maps and demux where a single `SessionRecord | undefine - ACP has one active session record per connection. - `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 to a rejected/superseded proposal. -- The existing [multi-session ACP proposal](2026-06-14-acp-multi-session.md) is updated to link this RFC if rejected. +- 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. ## What we give up diff --git a/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md b/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md index a0a16031c5..388af09716 100644 --- a/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md +++ b/docs/rfc/proposed/2026-06-20-truncate-interrupted-turns.md @@ -19,8 +19,9 @@ This makes the persisted turn boundary simple: a completed `turn/end` is the che - `TurnEndReasonMap` drops the `interrupted` variant. - `interruptedTurnClosers()` and its tests disappear. - The persistence coordinator's repair hook truncates backend-specific torn/open tail state without appending closers. -- Persistence docs say load returns the last completed turn, plus no partial final turn. +- [Session persistence docs](../../../packages/session-persistence/README.md) say load returns the last completed turn, plus no partial final turn. - Snapshot and contract tests update together with the behavior they pin. +- The session format version and recorded fixtures are refreshed; non-current stored logs are rejected per the pre-release format policy, with no migration path. ## What we give up diff --git a/scripts/publint-all.ts b/scripts/publint-all.ts index 376f02c9df..9d439fba98 100644 --- a/scripts/publint-all.ts +++ b/scripts/publint-all.ts @@ -3,7 +3,7 @@ import { resolve } from 'node:path' // publint every publishable package (vendor/ is private upstream code and // examples/ are not packages; both are out of scope). -// TODO(package-inventory): derive this from package metadata/classification. +// TODO(package-inventory): derive this from explicit package classification metadata. const packages = [ 'packages/llm', 'packages/session', From 552612622caf8cf715aced0612d58be9ea55bc17 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 19:23:05 +0800 Subject: [PATCH 3/8] docs: fold useful simplification RFCs from pr 74 --- docs/rfc/README.md | 3 ++ ...6-06-20-drop-unconsumed-llm-block-views.md | 44 ++++++++++++++++ ...-drop-unconsumed-registry-change-events.md | 50 +++++++++++++++++++ .../2026-06-20-prune-dead-seam-methods.md | 49 ++++++++++++++++++ .../2026-06-20-public-agent-stop-surface.md | 4 +- packages/acp/src/index.ts | 4 ++ packages/bash-local/src/run.ts | 6 ++- packages/tools/src/schema.ts | 9 +++- 8 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-block-views.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-unconsumed-registry-change-events.md create mode 100644 docs/rfc/proposed/2026-06-20-prune-dead-seam-methods.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 5f2e25ae31..b75f95d0bf 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -35,6 +35,9 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [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 | +| [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 | 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 new file mode 100644 index 0000000000..a0fb71d40b --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-block-views.md @@ -0,0 +1,44 @@ +# 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 new file mode 100644 index 0000000000..16c1b8faf2 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-drop-unconsumed-registry-change-events.md @@ -0,0 +1,50 @@ +# 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-prune-dead-seam-methods.md b/docs/rfc/proposed/2026-06-20-prune-dead-seam-methods.md new file mode 100644 index 0000000000..d38583c60b --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-prune-dead-seam-methods.md @@ -0,0 +1,49 @@ +# RFC: Prune dead methods from the persistence and bash capability seams + +Status: proposed + +## Problem + +Two capability seams ([interface / implementation / consumer](../implemented/2026-06-13-capability-seams.md)) carry abstract methods that no consumer calls. The seam exists to let implementations and consumers evolve independently — but a method no consumer programs against is not a seam, it is speculative surface every implementation must still implement and test. + +### `SessionPersistence.has()` and `.delete()` + +The abstract service declares four operations beyond create/append: `load`, `list`, `has`, `delete` ([packages/session-persistence/src/index.ts:142-151](../../../packages/session-persistence/src/index.ts)). Production consumers of `ctx.sessionPersistence` use only two of them: the agent-loop resume path calls `load()` ([packages/agent-loop/src/index.ts:176-194](../../../packages/agent-loop/src/index.ts)), and the ACP bridge calls `list()` for `session/list` ([packages/acp/src/index.ts](../../../packages/acp/src/index.ts)). Grepping every `sessionPersistence.*` / `persistence.*` use across `packages/*/src` and `examples/` finds no `has(` and no `delete(` on the service. The `.has(`/`.delete(` calls in `packages/acp/src/index.ts` are on the in-memory `SessionStore` and a local `Set` of loading ids, not persistence. The only callers of `has`/`delete` are the contract suites and per-backend specs. + +`has()` is not just unused — it is the most intricate branch in the shared coordinator: a tracked-vs-untracked dual-probe (`loadLive(id, cwd)` for a live-tracked session vs `loadStored(id)` for an untracked one) with a multi-line rationale ([packages/session-persistence/src/coordinator.ts:298-310](../../../packages/session-persistence/src/coordinator.ts)). `delete()` drags the `deleteStored` backend hook ([coordinator.ts:99](../../../packages/session-persistence/src/coordinator.ts), [coordinator.ts:313-319](../../../packages/session-persistence/src/coordinator.ts)) that every backend must implement. This is the [drop-mutable-session-summary](../implemented/2026-06-19-drop-mutable-session-summary.md) pattern: a contract test exercises both, but no shipping code asks "is this session persisted?" or removes one. + +### `BashExecutor.get()` and `.list()` + +The bash seam declares `get(id)` ("look up a background task by id") and `list()` ("all tracked background tasks") ([packages/bash/src/index.ts:88-107](../../../packages/bash/src/index.ts)), both implemented by `LocalBashExecutor` ([packages/bash-local/src/index.ts:179-191](../../../packages/bash-local/src/index.ts)). The sole production consumer — `dsh-tool-bash` — drives tasks via `ownerOf`, `onTaskDone`, `start`, `readOutput`, `kill`, `resolve`, `run`; it never calls `get`/`list` in shipping code, and there is no `bash_list` tool exposing a task roster to the model. So both are dead production seam surface. They are used by tests, more broadly than a single idiom: the bash seam/executor specs assert them directly ([packages/bash/tests/service.spec.ts](../../../packages/bash/tests/service.spec.ts), [packages/bash-local/tests/executor.spec.ts](../../../packages/bash-local/tests/executor.spec.ts) both call `get()`/`list()`), and several `dsh-tool-bash` tests reach through `ctx.bash.get(id)` to await a task's `done`, read its `status`, or inspect task fields ([packages/tool-bash/tests/tools.spec.ts](../../../packages/tool-bash/tests/tools.spec.ts), [packages/tool-bash/tests/integration.spec.ts](../../../packages/tool-bash/tests/integration.spec.ts)). These are test-harness conveniences, not shipping consumers — but they are real test code an implementing PR must migrate or delete. + +## Proposal + +Remove the methods nothing consumes, from the abstract seam, the implementation, and the contract/spec suites that exist only to exercise them: + +- `SessionPersistence.has()` / `.delete()`: delete the abstract declarations, the coordinator's `has`/`delete`/`deleteCore`, and the `PersistenceBackend.deleteStored` hook. Remove the `has`/`delete` rows from the contract suite and the per-backend specs (jsonl + sqlite each implement `deleteStored` only to satisfy the hook — that implementation goes too). The backends are the [dual-backend](../implemented/2026-06-14-session-persistence.md) design and otherwise out of scope, but removing a hook they implement for no consumer is part of removing the hook, not a backend redesign. +- `BashExecutor.get()` / `.list()`: delete the abstract declarations and the `LocalBashExecutor` impls. The seam/executor specs that assert `get()`/`list()` directly (`bash/tests/service.spec.ts`, `bash-local/tests/executor.spec.ts`) lose those assertions (the behavior is being removed). The `dsh-tool-bash` tests that reach through `ctx.bash.get(id)` to await `done`, read `status`, or inspect task fields switch to the public completion/status seam they should use — `onTaskDone` (or the `done` promise and status the `start()` return already exposes) — keeping their coverage without the removed lookup method. +- Update every doc and source-comment reference to the removed methods — not only literal `has(`/`delete(`/`get(`/`list(`/`deleteStored` call spellings, but also `{@link has}`/`{@link delete}` JSDoc links and prose that counts the methods (removing 2 of the persistence service's 6 public methods makes any "six public methods" phrasing wrong). The implementing PR greps `has`/`delete`/`get`/`list`/`deleteStored`/`{@link `/`six ` across `docs/`, `packages/*/README.md`, and source comments, and fixes each. The known doc sites: the seam READMEs ([packages/session-persistence/README.md](../../../packages/session-persistence/README.md)'s `has(id)`/`delete(id)` API row and its "delegates its six public service methods" prose → four, [packages/bash/README.md](../../../packages/bash/README.md)'s `get(id)`/`list()` row), the backend READMEs that describe `has`/`list` semantics ([packages/session-persistence-sqlite/README.md](../../../packages/session-persistence-sqlite/README.md), [packages/session-persistence-jsonl/README.md](../../../packages/session-persistence-jsonl/README.md) — reword "absent from `has()`/`list()`" to just `list()`), the service-map / seam docs in [docs/architecture.md](../../../docs/architecture.md), and the persistence prose in the [session-persistence RFC](../implemented/2026-06-14-session-persistence.md) and [shared write-coordinator RFC](../implemented/2026-06-18-shared-persistence-write-coordinator.md). The known source-comment sites: the abstract `create()` JSDoc's `{@link has}/{@link list}` link ([packages/session-persistence/src/index.ts](../../../packages/session-persistence/src/index.ts) — drop the `has` link), the coordinator's "six public methods"/"six public service methods" module + class JSDoc and its lazy-materialization JSDoc justifying the `materialized` flag by "the signal `has`/`list` rely on" ([packages/session-persistence/src/coordinator.ts](../../../packages/session-persistence/src/coordinator.ts)), the JSONL backend's `loadStored`/`deleteStored` comment, and the SQLite backend's `schema.ts` and `index.ts` comments that mention "absent from `has`/`list`" — all reworded to the surviving four-method, `list()`-only contract. + +## Why not keep them as "the seam should be complete"? + +The instinct that a persistence seam "should" offer delete, or a task executor "should" offer enumeration, is real — and it is exactly the speculative-completeness the pre-release stance warns against ([AGENTS.md](../../../AGENTS.md): optimize for the correct foundation, not for hypothetical callers you do not have). Each of these is one method to re-add the day a consumer needs it: + +- A session-management UI that deletes old sessions will want `delete()` — add it then, designed against that UI's real needs (soft-delete? cascade? confirmation?), not guessed now. +- A `bash_list` tool that shows the model its running tasks will want `list()` — add it with the tool. + +Re-adding a seam method with a live consumer is cheap and better-designed than the speculative version, because the consumer pins the contract. Carrying it unused means every implementation (and every future backend) must implement and test a method that does nothing. + +## Acceptance criteria + +- `has`/`delete`/`deleteStored` and `get`/`list` are gone from their seams, impls, and contract suites; `pnpm run knip` reports no new dead exports. +- The remaining seam operations (`create`/`append`/`load`/`list` for persistence; `run`/`start`/`ownerOf`/`onTaskDone`/`readOutput`/`kill`/`resolve` for bash) are untouched; ACP `session/list`, bash tool flows, and crash-recovery behave identically. +- `pnpm run test:coverage` stays 100% per-file (the contract/spec rows for the removed methods are deleted with them). +- Seam READMEs and `docs/architecture.md` no longer list the removed methods. + +## Risks + +- **`delete()` is the kind of operation a product eventually wants.** True — but "eventually" is the point. Deleting it now and re-adding it against a real consumer is strictly better than shipping a guessed contract. The dual backends each shed a `deleteStored` impl, which is a bounded edit in otherwise-out-of-scope packages. +- **`list()` on the bash seam is the natural seed for a future `bash_list`.** Acknowledged in the [pre-release foundation stance](../../../AGENTS.md): add the seed when the tool lands. The executor still tracks tasks internally (the `tasks` map backs `ownerOf`/`readOutput`/`kill`); exposing an enumeration is a one-line re-add. +- **Low coupling.** Both removals are confined to their seam + impl + tests; no cross-package consumer references the removed methods, so there is no ripple beyond the docs. + +Modest size, but it converts two seams from "what an implementation must provide for nobody" back to "exactly what a consumer uses." 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 e793b7bde6..8ff4ebb46b 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 @@ -6,13 +6,15 @@ Status: proposed The public `Agent` handle exposes three ways to reason about stopping work: `abort(reason?)`, `cancel(reason?)`, and `whenIdle()`. `abort()` kills only the in-flight step and leaves queued work alone; `cancel()` clears queued and steering work, aborts the running step, and handles the pre-step race; `whenIdle()` exposes the loop's private quiescence waiter to any consumer. In production, ACP uses `cancel()` for `session/cancel`, while lifecycle owners tear down agents through `AgentHandle.dispose()`. No production caller needs bare `abort()` or `whenIdle()`. +The `abort()`/`cancel()` distinction is real — `abort()` preserves queued prompts and steering while `cancel()` drops them — but no shipping code calls the public `abort()` verb. The loop's own stop paths (`cancel()` and disposal) abort the current `AbortController` directly rather than routing through `Agent.abort()`. Most tests that call `abort()` interrupt an empty queue and can switch to `cancel(reason)`; the one steering re-delivery test that deliberately depends on queue preservation should drive the in-flight `AbortController` directly, because `cancel()` would drop the queued steering it is trying to prove survives a step abort. The no-argument `abort()` default reason (`'aborted'`) is also deleted with the verb rather than preserved by accident; `cancel()` keeps its own `'cancelled'` default. + The extra surface area makes the loop carry public semantics that are mostly teardown internals. `whenIdle()` needs waiter state, special disposed-agent behavior, and a loop-exit promise so it resolves after quiescence rather than merely after a status flip. `abort()` has to be documented as distinct from queue-aware cancellation even though a UI cancellation almost always wants the broader operation. ## Proposal Keep `cancel()` as the only public stop primitive on `Agent`. Lifecycle owners use `AgentHandle.dispose()` to stop and unregister an agent; non-owners use `cancel()` to abandon current and queued work. The implementation can keep private abort controllers and quiescence promises, but they are not part of the plugin-facing `Agent` contract. -Delete public `abort()` and `whenIdle()`, the tests that exercise them as standalone API, and the docs that describe step-only abort as an embedding feature. The disposer remains async and still waits for the loop to stop; that guarantee moves entirely onto `AgentHandle.dispose()`. +Delete public `abort()` and `whenIdle()`, the tests that exercise them as standalone API, and the docs that describe step-only abort as an embedding feature. Empty-queue abort tests migrate to `cancel(reason)` where they still prove cancellation behavior; tests whose subject is the loop's internal `AbortController` behavior drive that controller directly; tests that only pin the removed no-arg `abort()` default go away with the method. The disposer remains async and still waits for the loop to stop; that guarantee moves entirely onto `AgentHandle.dispose()`. ## Acceptance criteria diff --git a/packages/acp/src/index.ts b/packages/acp/src/index.ts index d107c2c0cb..089379e83a 100644 --- a/packages/acp/src/index.ts +++ b/packages/acp/src/index.ts @@ -203,6 +203,10 @@ interface SessionRecord { * others are no-ops (settle-exactly-once). */ export function apply(ctx: Context, config: AcpConfig): void { + // TODO(double-default): these literals duplicate the Config schema defaults + // (`agentName`/`agentVersion` `.default(...)` above). The Loader applies the + // schema before apply() runs, so the `??` only fires for direct-apply unit + // tests. Pick one home for the default to avoid drift. const agentName = config.agentName ?? 'deepseek-harness-acp' const agentVersion = config.agentVersion ?? '0.0.1' diff --git a/packages/bash-local/src/run.ts b/packages/bash-local/src/run.ts index 46d48cf5fd..8a8d2065d2 100644 --- a/packages/bash-local/src/run.ts +++ b/packages/bash-local/src/run.ts @@ -159,7 +159,11 @@ export class OutputCollector { writeSync(this.spillFd, chunk) } - /** Read the collected tail without finalizing (used by background polling). */ + // TODO(snapshot-scope): `snapshot()` has one internal caller (`finalize()` at + // the bottom of this file) and `totalBytes` is read only by a test. The live + // background-poll path goes through `readFrom()`, so inline snapshot() into + // finalize() and drop or privatize the totalBytes getter. + /** Read the collected tail without finalizing (the final-result snapshot). */ snapshot(): CollectedOutput { return { text: Buffer.concat(this.chunks).toString('utf8'), diff --git a/packages/tools/src/schema.ts b/packages/tools/src/schema.ts index 5e8887f11b..b717eabf9a 100644 --- a/packages/tools/src/schema.ts +++ b/packages/tools/src/schema.ts @@ -39,7 +39,14 @@ export interface SchemaProp { description?: string /** Enum of allowed values (strings only). */ enum?: string[] - /** Default value. */ + /** + * Default value, emitted into the JSON Schema only (validation never applies + * it — see the validator note below). + * + * XXX(unused-default): no tool definition in the repo sets `default`; it rides + * into the wire schema for a model that no tool surfaces it to. Drop the field + * and its converter line unless a real tool needs a model-visible default. + */ default?: unknown /** Nested properties for type: 'object'. */ properties?: SchemaSpec From 99a203f445f4db36f97e69d1a7b00f8bfc497415 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 20:09:26 +0800 Subject: [PATCH 4/8] docs: add simplification discovery skill --- .../skills/dsh-find-simplifications/SKILL.md | 106 ++++++++++++++++++ .../agents/openai.yaml | 4 + 2 files changed, 110 insertions(+) create mode 100644 .agents/skills/dsh-find-simplifications/SKILL.md create mode 100644 .agents/skills/dsh-find-simplifications/agents/openai.yaml diff --git a/.agents/skills/dsh-find-simplifications/SKILL.md b/.agents/skills/dsh-find-simplifications/SKILL.md new file mode 100644 index 0000000000..ac8565e210 --- /dev/null +++ b/.agents/skills/dsh-find-simplifications/SKILL.md @@ -0,0 +1,106 @@ +--- +name: dsh-find-simplifications +description: 'Use when working in the deepseek-harness repo to find non-obvious simplification candidates and write proposed RFCs or inline TODO/FIXME/XXX notes for dead, duplicated, speculative, or over-built code surfaces; especially for requests like "find simplification RFCs", "look for unnecessary complexity", "audit for removal-style cleanups", or "fold worthwhile simplification ideas from another PR".' +--- + +# Finding DeepSeek Harness Simplifications + +This skill helps turn a broad "find things to simplify" request into evidence-backed RFCs that remove or collapse existing harness surface area. It is guidance, not a checklist: follow the code, keep judgment active, and prefer a few well-proven candidates over a pile of thin guesses. + +## Start With Repo Context + +- Read `AGENTS.md`, especially the pre-release stance, tests-document-behavior section, conventions, defensive patterns, and Type Safety and Documentation section. +- Skim [docs/architecture.md](../../../docs/architecture.md) before judging anything under `packages/`; simplifications that fight the service map or event taxonomy need extra evidence. +- Use the RFC index ([docs/rfc/README.md](../../../docs/rfc/README.md)) to understand intentional architecture. The most relevant implemented examples are [drop mutable session summary](../../../docs/rfc/implemented/2026-06-19-drop-mutable-session-summary.md), [shared persistence write coordinator](../../../docs/rfc/implemented/2026-06-18-shared-persistence-write-coordinator.md), [capability seams](../../../docs/rfc/implemented/2026-06-13-capability-seams.md), and the twin adapter / dual persistence backend RFCs. +- Treat dual LLM adapters and dual persistence backends as intentional by default. Do not propose deleting either twin/backend as "low effort" unless the user explicitly overrides that constraint. Removing an unused method or hook inside a protected seam can still be valid if it does not collapse the protected design. + +## What Counts As A Strong Candidate + +A strong simplification removes, folds, or demotes something real and has clear evidence that the current shape costs more than it buys: + +- A public method, event, config knob, registry notification, helper, package, durable event, or test artifact has no production consumer. +- Tests or docs are the only consumers, and the behavior they pin is not load-bearing. +- Two representations mirror the same fact, especially across durable session events and transient `agent/*` events. +- A seam has methods every implementation must support but no consumer uses. +- A package boundary exists only for test/demo/support code and adds publish or dependency overhead. +- A feature implements speculative product generality: multi-session/session-load, background task rosters, live registry invalidation, mid-turn steering, tool-owned UI rendering, and similar shapes with no product owner. +- An invariant, rollback path, goldens set, or special-case test exists only to protect an unused surface. +- The simplified behavior may differ slightly, but the new behavior is still reasonable and easier to explain. + +Thin candidates are usually not enough for an RFC: deleting one typo, running `knip` once, removing an intentionally documented backend/adapter, or flagging "this looks complex" without call-site proof. + +## Survey Broadly + +Use parallel subagents when the user asks for breadth or many candidates. Give each agent a domain and require evidence, not guesses. Useful domains: + +- Agent loop and session log: turn/step boundaries, steering, abort/cancel, durable events, replay, load/resume. +- ACP and UI surfaces: `session/*` methods, terminal `_meta`, transcript rendering, single vs multi-session state. +- LLM/tools/system prompt: stream/generate surfaces, assemblers, registries, tool schema defaults, presentation hooks. +- Bash and tool execution: foreground/background split, task ownership, output spill files, executor methods. +- Packages/examples/scripts/tests: package boundaries, static inventories, redundant snapshot goldens, support packages. + +If subagents are unavailable, simulate the same breadth yourself. Do not let the first good candidate stop the survey. + +## Prove Or Reject Each Candidate + +For every symbol or behavior, classify consumers before writing: + +- Production corpus: `packages/*/src`, `examples/*/src`, `examples/**/*.yml`, runtime scripts, and loader/config paths. +- Non-production corpus: tests, README/docs, RFCs, snapshots, generated goldens, and comments. +- Ambiguous corpus: examples and scripts that may be product smoke paths. Inspect usage before classifying. + +Use `rg` first. Good searches include the exact symbol, event name, package name, config key, method name with both `.name(` and `name(`, and any wire strings. Then read the call sites. `knip` can help, but it is not a substitute for understanding public interfaces, dynamic event names, tests, docs, and Cordis loader paths. + +Reject or downgrade a candidate when: + +- A production caller exists and the simplification would be a feature decision rather than a cleanup. +- The surface is explicitly justified by an implemented RFC or a hard-won defensive pattern, and the new evidence does not beat that reason. +- The removal would force unrelated churn without actually making the contract smaller. +- The idea is correct but tiny. Add a targeted TODO/FIXME/XXX instead, using the urgency semantics in [docs/development.md](../../../docs/development.md). + +## Write The RFC + +Create one file per durable proposal under `docs/rfc/proposed/yyyy-mm-dd-topic.md` and add it to the Proposed table in `docs/rfc/README.md`. Keep prose paragraphs on one physical line and use relative Markdown links. + +Prefer this shape, adjusting when the idea needs it: + +- `# RFC: ` +- `Status: proposed` +- `## Problem`: name the current surface, cite the relevant files, and state the consumer evidence. Separate production callers from tests/docs. +- `## Proposal`: say exactly what to remove, fold, demote, or rehome. Include tests, docs, READMEs, JSDoc, event-taxonomy, snapshot, and generated-file cleanup when relevant. +- `## Why not keep it?` or `## What we give up`: make the strongest counterargument legible. +- `## Acceptance criteria`: observable end state and gates. +- `## Risks`: public API changes, behavior changes, future product wants, and why the tradeoff is still reasonable. + +Be concrete enough that an implementing PR can follow the trail. Avoid vague "simplify this package" RFCs. When a proposal overlaps an existing RFC, consolidate the useful details into the existing one rather than creating a duplicate. + +## Inline TODO Notes + +Use inline TODO/FIXME/XXX only for small, local cleanups that are clearly useful but not durable design decisions. Keep them short and actionable: + +- Name the smell with a stable tag, e.g. `TODO(double-default)` or `XXX(unused-default)`. +- Explain why it is safe to revisit and what action would simplify it. +- Do not add TODOs for speculative complaints or for behavior that needs an RFC-level decision. + +## When Folding Another PR Or Branch + +Diff the sibling branch against `origin/master`, not against the current PR branch, so you see its independent contribution. For each item: + +- Port non-overlapping RFCs or TODOs that meet the quality bar. +- Consolidate overlapping material into the existing RFC that owns the topic. +- Do not port duplicate or lower-confidence proposals just to preserve the count. +- Update the PR body so reviewers see the true candidate count and scope. +- Close the duplicate PR only when the user asked you to, or when you clearly own that housekeeping. + +## Validation And PR Hygiene + +For docs-only RFC work, run at least `pnpm run doc-sync`, `pnpm run lint`, and `git diff --check`. For code comments or skill changes, also run the relevant validator when one exists. Before pushing, expect the pre-push hook to run module graph freshness, unit tests, snapshots, doc-sync, and hygiene. + +When opening or updating a PR, summarize: + +- How many RFCs and inline notes were added. +- The main areas surveyed. +- What was intentionally excluded. +- Which checks passed. + +Use a draft PR while the survey is still expanding; mark ready only when the candidate set, review responses, and validation are settled. diff --git a/.agents/skills/dsh-find-simplifications/agents/openai.yaml b/.agents/skills/dsh-find-simplifications/agents/openai.yaml new file mode 100644 index 0000000000..4016526f85 --- /dev/null +++ b/.agents/skills/dsh-find-simplifications/agents/openai.yaml @@ -0,0 +1,4 @@ +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." From 80179a5ed0c106af81a6a0230b7ae55bb655e78a Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 20:34:56 +0800 Subject: [PATCH 5/8] docs: address simplification review feedback --- .../agents/openai.yaml | 4 -- docs/rfc/README.md | 32 ++++++------ .../proposed/2026-06-14-acp-multi-session.md | 2 +- ...6-20-collapse-trace-only-session-events.md | 21 +++++--- .../2026-06-20-discover-package-inventory.md | 6 +-- ...2026-06-20-drop-durable-step-boundaries.md | 2 +- ...rop-unconsumed-llm-adapter-change-event.md | 43 ++++++++++++++++ ...-drop-unconsumed-llm-assembled-surfaces.md | 45 +++++++++++++++++ ...6-06-20-drop-unconsumed-llm-block-views.md | 44 ---------------- ...-drop-unconsumed-registry-change-events.md | 50 ------------------- .../2026-06-20-foreground-only-bash.md | 27 ---------- ...06-20-generic-long-running-tool-runtime.md | 35 +++++++++++++ .../2026-06-20-public-agent-stop-surface.md | 4 +- ...-20-remove-agent-boundary-mirror-events.md | 2 +- ...0-remove-redundant-snapshot-log-goldens.md | 8 +-- ...06-20-assembled-assistant-messages-only.md | 6 +-- .../2026-06-20-classify-support-packages.md | 4 +- .../2026-06-20-drop-acp-session-load.md | 4 +- .../2026-06-20-drop-acp-terminal-meta.md | 4 +- ...2026-06-20-drop-bash-output-spill-files.md | 4 +- .../2026-06-20-drop-unused-session-lineage.md | 4 +- ...6-20-fold-session-persistence-interface.md | 2 +- .../2026-06-20-generic-tool-rendering.md | 2 +- .../2026-06-20-retire-mid-turn-steering.md | 4 +- .../2026-06-20-single-session-acp-bridge.md | 8 +-- .../2026-06-20-truncate-interrupted-turns.md | 4 +- packages/acp/README.md | 2 +- 27 files changed, 189 insertions(+), 184 deletions(-) delete mode 100644 .agents/skills/dsh-find-simplifications/agents/openai.yaml create mode 100644 docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-adapter-change-event.md create mode 100644 docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-assembled-surfaces.md delete mode 100644 docs/rfc/proposed/2026-06-20-drop-unconsumed-llm-block-views.md delete mode 100644 docs/rfc/proposed/2026-06-20-drop-unconsumed-registry-change-events.md delete mode 100644 docs/rfc/proposed/2026-06-20-foreground-only-bash.md create mode 100644 docs/rfc/proposed/2026-06-20-generic-long-running-tool-runtime.md rename docs/rfc/{proposed => rejected}/2026-06-20-assembled-assistant-messages-only.md (85%) rename docs/rfc/{proposed => rejected}/2026-06-20-classify-support-packages.md (82%) rename docs/rfc/{proposed => rejected}/2026-06-20-drop-acp-session-load.md (85%) rename docs/rfc/{proposed => rejected}/2026-06-20-drop-acp-terminal-meta.md (89%) rename docs/rfc/{proposed => rejected}/2026-06-20-drop-bash-output-spill-files.md (79%) rename docs/rfc/{proposed => rejected}/2026-06-20-drop-unused-session-lineage.md (79%) rename docs/rfc/{proposed => rejected}/2026-06-20-fold-session-persistence-interface.md (90%) rename docs/rfc/{proposed => rejected}/2026-06-20-generic-tool-rendering.md (93%) rename docs/rfc/{proposed => rejected}/2026-06-20-retire-mid-turn-steering.md (87%) rename docs/rfc/{proposed => rejected}/2026-06-20-single-session-acp-bridge.md (64%) rename docs/rfc/{proposed => rejected}/2026-06-20-truncate-interrupted-turns.md (90%) 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`. From 7f2769c529b10a6d174447555e02aac29666e0b7 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:07:25 +0800 Subject: [PATCH 6/8] docs: address latest simplification review --- docs/rfc/README.md | 5 +- .../2026-06-20-classify-packages-by-aspect.md | 46 +++++++++++++++++++ .../2026-06-20-discover-package-inventory.md | 2 +- ...0-remove-redundant-snapshot-log-goldens.md | 20 ++++---- .../2026-06-20-classify-support-packages.md | 2 +- ...2026-06-20-drop-durable-step-boundaries.md | 6 +-- .../2026-06-20-retire-mid-turn-steering.md | 2 +- .../2026-06-20-truncate-interrupted-turns.md | 2 +- 8 files changed, 68 insertions(+), 17 deletions(-) create mode 100644 docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md rename docs/rfc/{proposed => rejected}/2026-06-20-drop-durable-step-boundaries.md (67%) diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 84677bda8a..6e2fbb9a7e 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -37,11 +37,11 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [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 | | [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 | -| [Use the recorded session fixture as the snapshot log golden](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | +| [Use `session.jsonl` as the only snapshot session-log artifact](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | +| [Classify packages by aspect metadata](proposed/2026-06-20-classify-packages-by-aspect.md) | 2026-06-20 | | [Discover package inventories instead of maintaining static lists](proposed/2026-06-20-discover-package-inventory.md) | 2026-06-20 | ## Implemented @@ -84,6 +84,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [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 durable step boundary events](rejected/2026-06-20-drop-durable-step-boundaries.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 | diff --git a/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md b/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md new file mode 100644 index 0000000000..e9723a5660 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md @@ -0,0 +1,46 @@ +# RFC: Classify packages by aspect metadata + +Status: proposed + +## Problem + +The harness package tree is flat, and every package manifest is currently `private: true`. That is fine as a pre-release safety default, but it means neither paths nor npm publish flags tell scripts what role a package plays. [publint-all](../../../scripts/publint-all.ts) needs to know which packages are release-shaped, docs need to describe which packages are core product surface, and future cleanup work needs a way to distinguish support utilities from load-bearing product modules. + +A single hierarchy such as product, integration, support, or testing is too coarse. Packages naturally carry overlapping facts: an LLM adapter is provider-facing and publish-shaped; `tool-bash` is a tool consumer and bash-related; `llm-replay` is an LLM adapter shape and test/snapshot support; ACP is an editor bridge and current product surface. Forcing each package into one bucket would either hide useful facts or recreate static exception lists under different names. + +## Proposal + +Add explicit, repo-owned package aspect metadata to each `packages/*/package.json`, using a manifest-local key such as `dsh.aspects` unless the implementing change finds an already-established repo metadata key. The metadata is a controlled vocabulary, not free-form prose. + +For example: + +```json +{ + "dsh": { + "aspects": ["core", "llm", "publishable"] + } +} +``` + +The initial vocabulary should stay small and useful to scripts. Expected facets include `core`, `implementation`, `consumer`, `llm`, `bash`, `fs`, `persistence`, `agent`, `acp`, `ui`, `example-support`, `test-support`, `replay`, and `publishable`. A package may declare multiple facets; no script should assume exactly one role. + +`publishable` is a repo policy facet, not a mirror of npm's `private` flag. While the harness is unreleased, packages can remain `private: true` and still declare `publishable` so publish-shape gates know which manifests to check. When release policy changes, the aspect continues to describe intent while the npm flag controls whether publication is allowed. + +Scripts should consume the metadata directly. `publint-all` filters on `publishable`, module graph or package inventory docs can group by domain facets, and the adding-a-package cookbook asks authors to choose aspects when creating a new package. Unknown facets should fail loudly so typoed metadata does not silently fork the taxonomy. + +## Acceptance criteria + +- Every `packages/*` manifest declares package aspects from a documented controlled vocabulary. +- The vocabulary explains each facet's meaning and when a new facet is appropriate. +- `publint-all` derives its package list from `publishable` metadata instead of a hard-coded array. +- Package inventory docs and module-graph grouping can read aspects without inferring intent from package names or folder paths. +- Adding a package requires choosing aspects, and CI fails if a package is missing aspect metadata or uses an unknown facet. +- No package path moves are required just to express classification. + +## What we give up + +Aspect metadata is less visually obvious than folders, and a package can be over-tagged if reviewers are careless. The counterweight is that metadata preserves the current package import shape while making policy facts explicit and machine-checkable. If a future package truly needs a new physical boundary, that move can still happen for architectural reasons rather than as a classification workaround. + +## Related + +This supersedes the rejected [product/integration/support package taxonomy](../rejected/2026-06-20-classify-support-packages.md) and supplies the package source of truth expected by [discover package inventories](2026-06-20-discover-package-inventory.md). 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 c75f93fd99..c99412c14b 100644 --- a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md @@ -12,7 +12,7 @@ Static lists are appropriate when they encode policy; they are needless friction 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. -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. +The metadata should come from [classifying packages by aspect](2026-06-20-classify-packages-by-aspect.md) 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 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 cf853ce341..cb40dbbf9f 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,27 +1,31 @@ -# RFC: Use the recorded session fixture as the snapshot log golden +# RFC: Use `session.jsonl` as the only snapshot session-log artifact Status: proposed ## Problem -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. +Model-driving 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, when one committed session log can serve as both replay input and expected persisted output. +Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario. ## Proposal -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. +Remove the `session.golden.jsonl` concept entirely. Every scenario has at most one committed session-log artifact, `session.jsonl`: + +- For recorded scenarios, `session.jsonl` remains the raw harvested log. Replay still derives model chunks from it, and the snapshot test compares the replay run's normalized persisted log against normalized `session.jsonl`. +- For authored override scenarios, `replay.override.json` drives model behavior and `session.jsonl` holds the expected produced session log. The replay adapter ignores the fixture for model chunks when the override exists, so the same file can be the expected log without affecting replay behavior. +- For no-model scenarios, `session.jsonl` can stay as the minimal fixture needed to boot `llm-replay`; no session-log comparison is needed unless the scenario creates a persisted session. Stdout goldens remain unchanged; they are the editor-facing projection and are not redundant with the session fixture. ## Acceptance criteria -- 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. +- `session.golden.jsonl` disappears from the snapshot harness, fixtures, orphan guards, and docs. +- The snapshot test derives the expected session log from `session.jsonl` for every model scenario. +- Authored sidecar scenarios commit their expected produced log in `session.jsonl`; `replay.override.json` remains the model-behavior override. - Orphan-fixture guards understand which files are required by scenario kind. - The [ACP snapshot tests RFC](../implemented/2026-06-19-acp-snapshot-tests.md) is updated to describe the reduced fixture set. ## What we give up -Reviewers lose one redundant artifact that made the expected persisted log visually separate from the replay fixture. The stdout golden still protects the editor transcript, and comparing replay output to the recorded fixture preserves the loop/persistence regression check without duplicating files. +Reviewers lose one artifact name that made the expected persisted log visually separate from the replay fixture. The stdout golden still protects the editor transcript, and comparing replay output to `session.jsonl` preserves the loop/persistence regression check without duplicating files. diff --git a/docs/rfc/rejected/2026-06-20-classify-support-packages.md b/docs/rfc/rejected/2026-06-20-classify-support-packages.md index 3634974a5b..254ddbdc7c 100644 --- a/docs/rfc/rejected/2026-06-20-classify-support-packages.md +++ b/docs/rfc/rejected/2026-06-20-classify-support-packages.md @@ -12,7 +12,7 @@ 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. +The rejected part is the one-dimensional taxonomy. The useful follow-up is [explicit package aspect metadata](../proposed/2026-06-20-classify-packages-by-aspect.md) 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. diff --git a/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md b/docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md similarity index 67% rename from docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md rename to docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md index ad424cb06e..60b2af391a 100644 --- a/docs/rfc/proposed/2026-06-20-drop-durable-step-boundaries.md +++ b/docs/rfc/rejected/2026-06-20-drop-durable-step-boundaries.md @@ -1,12 +1,12 @@ # RFC: Drop durable step boundary events -Status: proposed +Status: rejected — `step/end` is the durable indication that a model step finished, and keeping the symmetric `step/start` / `step/end` pair makes crash repair, invariants, and transcript inspection clearer than inferring completion from adjacent step-scoped events. ## Problem The session log stores `step/start` and `step/end` events even though every step-scoped event already carries `{ turn, step }`: assistant chunks, assistant messages, tool calls, tool results, usage, and errors. `deriveMessages()` ignores step boundaries, ACP ignores them for UI, and the main consumers are invariants, tests, snapshot goldens, and crash repair. -The boundary events make the log more ceremonial than informative. The loop tracks open steps solely to close them, repair synthesizes `step/end` when a crash leaves a step open, invariants track a second nesting stack inside the turn, and snapshots carry lines that do not affect replayed message history. A model request that crashes before producing any step-scoped event is the only information represented by a bare `step/start`, and that case has no useful resumable content. +The rejected argument was that boundary events make the log more ceremonial than informative. In practice, `step/end` is concrete information: a reader can tell whether a model request finished, crashed, or is being repaired without deriving that state from the next event. A bare `step/start` is likewise useful for a model request that began but produced no chunks before failing. ## Proposal @@ -25,4 +25,4 @@ The invariants plugin should enforce that step-scoped events have valid positive ## What we give up -The log no longer records "a model request started but produced no event before the process died" as a durable fact. That is acceptable: there is no assistant content, tool call, usage, or error to replay from that empty request. A live UI can still show an in-progress step from a transient event if it needs one; the durable log should not store an empty bracket. +The log no longer records "a model request started but produced no event before the process died" as a durable fact, and no longer has an explicit "this step completed" marker. That loss is not acceptable while the session log is the durable replay and audit surface. diff --git a/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md b/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md index cbfa238370..97c7d5dc61 100644 --- a/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md +++ b/docs/rfc/rejected/2026-06-20-retire-mid-turn-steering.md @@ -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](../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. +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. diff --git a/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md b/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md index 410237daa4..771388ede9 100644 --- a/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md +++ b/docs/rfc/rejected/2026-06-20-truncate-interrupted-turns.md @@ -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](../proposed/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](2026-06-20-drop-durable-step-boundaries.md) smaller. From 800e08e9308ea558e96bf58069e7e2133198627e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:28:02 +0800 Subject: [PATCH 7/8] docs: refine package hierarchy RFC --- docs/rfc/README.md | 3 +- .../2026-06-20-classify-packages-by-aspect.md | 46 --------------- .../2026-06-20-discover-package-inventory.md | 8 +-- .../proposed/2026-06-20-package-hierarchy.md | 57 +++++++++++++++++++ ...0-remove-redundant-snapshot-log-goldens.md | 2 +- .../2026-06-20-classify-support-packages.md | 28 --------- scripts/publint-all.ts | 2 +- 7 files changed, 64 insertions(+), 82 deletions(-) delete mode 100644 docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md create mode 100644 docs/rfc/proposed/2026-06-20-package-hierarchy.md delete mode 100644 docs/rfc/rejected/2026-06-20-classify-support-packages.md diff --git a/docs/rfc/README.md b/docs/rfc/README.md index 6e2fbb9a7e..7098ea216e 100644 --- a/docs/rfc/README.md +++ b/docs/rfc/README.md @@ -41,7 +41,7 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r | [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 | | [Use `session.jsonl` as the only snapshot session-log artifact](proposed/2026-06-20-remove-redundant-snapshot-log-goldens.md) | 2026-06-20 | -| [Classify packages by aspect metadata](proposed/2026-06-20-classify-packages-by-aspect.md) | 2026-06-20 | +| [Reorganize packages into a modular hierarchy](proposed/2026-06-20-package-hierarchy.md) | 2026-06-20 | | [Discover package inventories instead of maintaining static lists](proposed/2026-06-20-discover-package-inventory.md) | 2026-06-20 | ## Implemented @@ -80,7 +80,6 @@ Do NOT write one for a mechanical or local choice (a variable name, a one-file r |---|---| | [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 | diff --git a/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md b/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md deleted file mode 100644 index e9723a5660..0000000000 --- a/docs/rfc/proposed/2026-06-20-classify-packages-by-aspect.md +++ /dev/null @@ -1,46 +0,0 @@ -# RFC: Classify packages by aspect metadata - -Status: proposed - -## Problem - -The harness package tree is flat, and every package manifest is currently `private: true`. That is fine as a pre-release safety default, but it means neither paths nor npm publish flags tell scripts what role a package plays. [publint-all](../../../scripts/publint-all.ts) needs to know which packages are release-shaped, docs need to describe which packages are core product surface, and future cleanup work needs a way to distinguish support utilities from load-bearing product modules. - -A single hierarchy such as product, integration, support, or testing is too coarse. Packages naturally carry overlapping facts: an LLM adapter is provider-facing and publish-shaped; `tool-bash` is a tool consumer and bash-related; `llm-replay` is an LLM adapter shape and test/snapshot support; ACP is an editor bridge and current product surface. Forcing each package into one bucket would either hide useful facts or recreate static exception lists under different names. - -## Proposal - -Add explicit, repo-owned package aspect metadata to each `packages/*/package.json`, using a manifest-local key such as `dsh.aspects` unless the implementing change finds an already-established repo metadata key. The metadata is a controlled vocabulary, not free-form prose. - -For example: - -```json -{ - "dsh": { - "aspects": ["core", "llm", "publishable"] - } -} -``` - -The initial vocabulary should stay small and useful to scripts. Expected facets include `core`, `implementation`, `consumer`, `llm`, `bash`, `fs`, `persistence`, `agent`, `acp`, `ui`, `example-support`, `test-support`, `replay`, and `publishable`. A package may declare multiple facets; no script should assume exactly one role. - -`publishable` is a repo policy facet, not a mirror of npm's `private` flag. While the harness is unreleased, packages can remain `private: true` and still declare `publishable` so publish-shape gates know which manifests to check. When release policy changes, the aspect continues to describe intent while the npm flag controls whether publication is allowed. - -Scripts should consume the metadata directly. `publint-all` filters on `publishable`, module graph or package inventory docs can group by domain facets, and the adding-a-package cookbook asks authors to choose aspects when creating a new package. Unknown facets should fail loudly so typoed metadata does not silently fork the taxonomy. - -## Acceptance criteria - -- Every `packages/*` manifest declares package aspects from a documented controlled vocabulary. -- The vocabulary explains each facet's meaning and when a new facet is appropriate. -- `publint-all` derives its package list from `publishable` metadata instead of a hard-coded array. -- Package inventory docs and module-graph grouping can read aspects without inferring intent from package names or folder paths. -- Adding a package requires choosing aspects, and CI fails if a package is missing aspect metadata or uses an unknown facet. -- No package path moves are required just to express classification. - -## What we give up - -Aspect metadata is less visually obvious than folders, and a package can be over-tagged if reviewers are careless. The counterweight is that metadata preserves the current package import shape while making policy facts explicit and machine-checkable. If a future package truly needs a new physical boundary, that move can still happen for architectural reasons rather than as a classification workaround. - -## Related - -This supersedes the rejected [product/integration/support package taxonomy](../rejected/2026-06-20-classify-support-packages.md) and supplies the package source of truth expected by [discover package inventories](2026-06-20-discover-package-inventory.md). 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 c99412c14b..bb2f3e95ee 100644 --- a/docs/rfc/proposed/2026-06-20-discover-package-inventory.md +++ b/docs/rfc/proposed/2026-06-20-discover-package-inventory.md @@ -6,17 +6,17 @@ Status: proposed Package and gate inventories are repeated by hand. [scripts/publint-all.ts](../../../scripts/publint-all.ts) has a static list of publishable packages. 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. These lists are small today, but every new package or gate creates another manual synchronization point. -Static lists are appropriate when they encode policy; they are needless friction when they duplicate manifest data that already exists in `package.json`, workspace globs, or package metadata. +Static lists are appropriate when they encode policy; they are needless friction when they duplicate manifest data or layout facts that already exist in `package.json`, workspace globs, or the package hierarchy. ## Proposal -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. +Make package/gate inventories discoverable. Publishability should come from the deliberate [package hierarchy](2026-06-20-package-hierarchy.md) plus package manifests, 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. -The metadata should come from [classifying packages by aspect](2026-06-20-classify-packages-by-aspect.md) 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. +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. ## Acceptance criteria -- `publint-all` discovers publishable packages from manifests plus explicit aspect metadata. +- `publint-all` discovers publishable packages from the hierarchy plus manifests instead of a hard-coded array. - 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-package-hierarchy.md b/docs/rfc/proposed/2026-06-20-package-hierarchy.md new file mode 100644 index 0000000000..bc0c401ca2 --- /dev/null +++ b/docs/rfc/proposed/2026-06-20-package-hierarchy.md @@ -0,0 +1,57 @@ +# RFC: Reorganize packages into a modular hierarchy + +Status: proposed + +## Problem + +`packages/` is flat. Core product packages, provider integrations, capability seams, example UI support, and snapshot-only replay support all sit at the same level and look equally foundational. The [package README](../../../packages/README.md) already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more product-shaped than they are and forces publish/lint/doc scripts to encode intent through comments or static lists. + +This is not just cosmetic. A package's location currently says little about whether it is core API, a swappable capability, an adapter integration, an example harness helper, or test infrastructure. That makes future removal harder because every top-level package looks like part of the same public surface. + +## Proposal + +Move packages into a deliberate hierarchy under `packages/`. The exact layout is deferred to the implementing PR, but it should group packages by modular role rather than keep every package at one flat level. + +One plausible shape: + +```text +packages/ + core/ + llm/ + session/ + system-prompt/ + tools/ + agent/ + agent-loop/ + invariants/ + capabilities/ + bash/ + bash-local/ + tool-bash/ + session-persistence/ + session-persistence-jsonl/ + session-persistence-sqlite/ + integrations/ + llm-deepseek/ + llm-pi-ai/ + acp/ + support/ + ui-stdio/ + llm-replay/ +``` + +The final implementation may choose different names or groupings, but it should keep the same intent: core APIs, capability seams, concrete integrations, and support/test/example packages are distinguishable from the filesystem alone. Npm package names can stay `@deepseek-ai/dsh-*`; the hierarchy is about repo structure and maintenance policy, not public package renaming. + +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 + +- Packages move from the flat `packages//` layout into a documented modular hierarchy. +- The implementing PR chooses the exact hierarchy and updates workspace globs, TypeScript paths, package docs, generated module graphs, `cordis.yml` package paths, build scripts, and publish/lint scripts in one coordinated move. +- Scripts that publish, lint publishability, or generate package inventories use the hierarchy instead of an ad hoc static list where the hierarchy is enough to express the policy. +- Docs explain which package groups are part of the product API and which groups are support/test/example infrastructure. +- New package guidance tells authors where to place a package and discourages new one-off top-level groups. + +## What we give up + +The restructure churns imports, workspace globs, docs links, and package paths. That churn is acceptable pre-release if it prevents the flat layout from fossilizing support packages as product contracts. 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 cb40dbbf9f..15b996cf61 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 @@ -6,7 +6,7 @@ Status: proposed Model-driving 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. -Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario. +Authored override scenarios (`error-finish`, `cancel`) currently use `replay.override.json` to drive model behavior and keep `session.jsonl` as a minimal dummy fixture, while `session.golden.jsonl` holds the expected persisted log. The override file is a JSON array of `ReplayEntry` objects: `{ "kind": "chunks", "chunks": StreamChunk[] }`, `{ "kind": "throw", "chunks": StreamChunk[], "message": string, "code": string, "status"?: number }`, or `{ "kind": "hang" }`. That split is also unnecessary: when an override sidecar exists, `llm-replay` replaces the derived script and does not need `session.jsonl` for model chunks, so `session.jsonl` can still be the expected session-log artifact for the scenario. ## Proposal diff --git a/docs/rfc/rejected/2026-06-20-classify-support-packages.md b/docs/rfc/rejected/2026-06-20-classify-support-packages.md deleted file mode 100644 index 254ddbdc7c..0000000000 --- a/docs/rfc/rejected/2026-06-20-classify-support-packages.md +++ /dev/null @@ -1,28 +0,0 @@ -# RFC: Classify product, integration, and support packages - -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 - -`packages/` is flat. Core product packages, provider integrations, tool implementations, example UI support, and snapshot-only replay support all sit at the same level and look equally publishable. The [package README](../../../packages/README.md) already has a `FIXME(package-hierarchy)` noting that `ui-stdio` and `llm-replay` were extracted from examples mostly for reuse and coverage. The flat layout makes support packages appear more foundational than they are and forces publish/lint/doc scripts to special-case intent in prose or static lists. - -This is not just cosmetic. A package's location currently says little about whether it is core API, an integration, an example harness helper, or test infrastructure. That makes future removal harder because every top-level package looks like part of the same public surface. - -## Proposal - -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](../proposed/2026-06-20-classify-packages-by-aspect.md) 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 - -- Each package has an explicit classification visible from path or package metadata. -- Scripts that publish, lint publishability, or generate module graphs use the classification instead of an ad hoc static list. -- Docs explain which package classes are part of the product API. -- YAML loader paths and TypeScript path aliases are updated in one coordinated move. - -## What we give up - -The restructure churns imports, workspace globs, docs links, and package paths. That churn is acceptable pre-release if it prevents the flat layout from fossilizing a support package as a product contract. diff --git a/scripts/publint-all.ts b/scripts/publint-all.ts index 9d439fba98..e0bee4393e 100644 --- a/scripts/publint-all.ts +++ b/scripts/publint-all.ts @@ -3,7 +3,7 @@ import { resolve } from 'node:path' // publint every publishable package (vendor/ is private upstream code and // examples/ are not packages; both are out of scope). -// TODO(package-inventory): derive this from explicit package classification metadata. +// TODO(package-inventory): derive this from the deliberate package hierarchy. const packages = [ 'packages/llm', 'packages/session', From c079b74c789c59e4dee4a74b2f4e9ad3c5e7a75c Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Sat, 20 Jun 2026 21:36:49 +0800 Subject: [PATCH 8/8] docs: clarify package hierarchy example --- .../rfc/proposed/2026-06-20-package-hierarchy.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/rfc/proposed/2026-06-20-package-hierarchy.md b/docs/rfc/proposed/2026-06-20-package-hierarchy.md index bc0c401ca2..7351cefc93 100644 --- a/docs/rfc/proposed/2026-06-20-package-hierarchy.md +++ b/docs/rfc/proposed/2026-06-20-package-hierarchy.md @@ -17,30 +17,32 @@ One plausible shape: ```text packages/ core/ - llm/ session/ system-prompt/ tools/ agent/ agent-loop/ invariants/ - capabilities/ + llm/ + llm/ + adapters/ + llm-deepseek/ + llm-pi-ai/ + bash/ bash/ bash-local/ tool-bash/ + session-persistence/ session-persistence/ session-persistence-jsonl/ session-persistence-sqlite/ - integrations/ - llm-deepseek/ - llm-pi-ai/ - acp/ + acp/ support/ ui-stdio/ llm-replay/ ``` -The final implementation may choose different names or groupings, but it should keep the same intent: core APIs, capability seams, concrete integrations, and support/test/example packages are distinguishable from the filesystem alone. Npm package names can stay `@deepseek-ai/dsh-*`; the hierarchy is about repo structure and maintenance policy, not public package renaming. +The final implementation may choose different names or groupings, but it should keep the same intent: core APIs, package families such as LLM/bash/session persistence, standalone integrations such as ACP, and support/test/example packages are distinguishable from the filesystem alone. Npm package names can stay `@deepseek-ai/dsh-*`; the hierarchy is about repo structure and maintenance policy, not public package renaming. 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.