Merge remote-tracking branch 'origin/master' into simpl-a2-vocab
# Conflicts: # docs/rfc/README.md
This commit is contained in:
Tianyi Cui
-32
@@ -1,32 +0,0 @@
|
||||
# RFC: Drop the unconsumed web observation surface — the `providers-change` event and the status methods
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`WebService` exposes an observation surface no production code observes:
|
||||
|
||||
- **`web/providers-change`** (`packages/web/web/src/index.ts`) is declared and emitted on every provider registration and disposal, and each registration effect's rollback yield is ordered BEFORE the emit solely so a throwing change listener unwinds the registration. No listener exists outside the package's own two unit tests (one of which exists to pin that rollback ordering).
|
||||
- **`searchStatus()` / `fetchStatus()` and the `WebCapabilityStatus` union** (same package) have zero production callers: `dsh-tool-web` executes directly through `ctx.web.search()`/`fetch()` and surfaces unavailability as the structured `WebError` codes the seam throws at execution time (`packages/web/tool-web/src/search.ts`, `packages/web/tool-web/src/fetch.ts`); the only status callers are the web packages' own tests. The prose in `packages/web/tool-web/README.md` and [architecture.md](../../../architecture.md) still claims the tool "reads only the aggregated `searchStatus()`/`fetchStatus()`" — drift that survives only because nothing checks prose against call sites.
|
||||
|
||||
The seam's own design starves both surfaces of consumers: tool registration follows product ENABLEMENT, not provider availability (`packages/web/tool-web/src/index.ts`), and provider selection resolves at execution time, never cached — so there is no cache to invalidate, no registration set to recompute, and no caller that needs an availability probe distinct from executing and routing the structured error. HMR cleanup is carried by the effect disposers themselves.
|
||||
|
||||
This mirrors [drop the unconsumed `llm/adapter-change` event](../../implemented/simplification/2026-06-20-drop-unconsumed-llm-adapter-change-event.md), which removed the same notification shape, the same rollback-before-emit machinery, and the same listener-throw test from `LlmService`. That RFC's keep/cut criterion — keep `tools/change` for its plausible user-facing tool-list consumer, cut the boot-time backend-registry signal — puts a web-provider registry squarely on the cut side; the status methods are the same judgment applied to a pull surface instead of a push one.
|
||||
|
||||
## Proposal
|
||||
|
||||
Delete the event declaration, both emits, and the rollback-before-emit ordering (the plain `ctx.effect` disposer keeps HMR cleanup). Delete `searchStatus()`/`fetchStatus()`/`WebCapabilityStatus` — the provider-private `status()` stays, since it feeds execution-time selection. Delete the two event tests and rewrite the status-based test assertions onto the behavior a real caller observes (a successful `search()`/`fetch()`, or the structured `WebError` codes for unavailable/ambiguous/misconfigured provider sets). Run `pnpm run gen-cordis-catalog`; update `packages/web/web/README.md`, `packages/web/tool-web/README.md` (the drifted reads-status sentence), [web.md](../../../core-data-structures/web.md), and the web paragraph in [architecture.md](../../../architecture.md). The implementing PR amends the [web capability seam RFC](../../implemented/architecture/2026-06-24-web-capability-seam.md)'s facts (it specifies the event and the status aggregation) per [implemented/AGENTS.md](../../implemented/AGENTS.md).
|
||||
|
||||
## Why not keep it?
|
||||
|
||||
The web seam RFC specified both deliberately — the event as a minimal HMR-visibility signal, the status methods as the tool's aggregated diagnostics — and a future provider-status panel is imaginable. But the same RFC's other choices starved them: derived-on-call selection and enablement-based registration leave no consumer that CAN need either, the shipped tool demonstrates the real pattern (execute and route the structured error), and the drifted README sentence shows the promised consumer never materialized. Per AGENTS.md "RFCs are proposals, not golden truth", these are the parts of that proposal the code has since shown to over-reach; a future observer reintroduces the smallest signal or query it actually consumes, shaped by that consumer.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- No `providers-change`, `searchStatus`, `fetchStatus`, or `WebCapabilityStatus` spelling outside RFC history; the catalog is regenerated and fresh (`verify-cordis-catalog` green).
|
||||
- Registration/disposal HMR-safety tests prove cleanup through execution behavior rather than the removed surfaces.
|
||||
- `packages/web/tool-web/README.md` and the architecture paragraph describe the execution-time error-routing contract the tool actually has.
|
||||
|
||||
## Risks
|
||||
|
||||
A future provider-picker UI or diagnostics panel wants change notifications or a status query — it re-adds the smallest surface it consumes; the identical judgment, and its reversal condition, is already recorded on the llm precedent.
|
||||
@@ -1,27 +0,0 @@
|
||||
# RFC: Fold the stdio UI helper into the stdio app
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`@deepseek-ai/dsh-ui-stdio` is a whole package whose only runtime importer is the app package `@deepseek-ai/dsh-stdio-agent` (`packages/ui/stdio-agent/src/index.ts`). The examples reach the readline UI by loading the app, never by composing the helper themselves; every other repo reference is mechanical or descriptive surface that exists BECAUSE the package boundary exists — manifest and tsconfig entries, generated module-graph rows, dependency-graph and README rows, and doc comments naming the package. [The ui group README](../../../../packages/ui/README.md) records the placement rationale — the helper "exists chiefly for the examples and the coverage gate — `ui/` is reserved for surfaces shipped as product" — which leaves a standing tension: a shipped product app depends on a support package documented as NOT product surface.
|
||||
|
||||
The boundary buys package metadata, workspace and tsconfig references, module-graph rows, README entries, and publint surface for a helper that is not independently swappable: the stdio app's front-door cluster always includes the readline UI, and nothing else can meaningfully consume it.
|
||||
|
||||
## Proposal
|
||||
|
||||
Fold the helper into `@deepseek-ai/dsh-stdio-agent`: move `createStdioChat`, its `StdioRuntime` test seam, and its unit tests into `packages/ui/stdio-agent`; delete the `packages/support/ui-stdio` package with its manifest, references, module-graph rows, and README rows; update every reference that names the package (the example e2e module docs, `packages/README.md`, the support and todo README rows, the stdio-agent README, the ui group README, tsconfig references, the generated module graph). Keep the runtime seam so EOF handling, rendering, disposal, and piped-vs-TTY behavior stay unit-covered without hijacking process globals; the keyless Loader-path smokes keep guarding the export shape end-to-end.
|
||||
|
||||
## Why not promote it to `ui/` instead?
|
||||
|
||||
Promotion would resolve the support-vs-product mismatch while keeping the boundary — the right call only if the readline UI were an independently swappable integration or had a second composer, and the consumer census says neither. The structured ACP bridge stays its own package because it is the product protocol surface with its own contract and snapshot tiers; the readline helper is scaffolding for one app's front door. Re-extraction stays cheap pre-release: if a second product app wants the readline UI, split it back out then, with that consumer shaping the package contract.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- `packages/support/ui-stdio` no longer exists; the helper and its tests live in `packages/ui/stdio-agent`; no reference to the deleted package remains outside RFC history.
|
||||
- The stdio app still renders transcript events, handles stdin lines and EOF, renders todo checklists, and disposes readline listeners under HMR; the echo/coding keyless smokes still boot through the real Loader path and guard the export shape.
|
||||
- Manifests, tsconfig references, the generated module graph, and docs are updated; `pnpm run test:coverage`, `pnpm run test:snapshot`, `pnpm run doc-sync`, `pnpm run build`, and `pnpm run hygiene` pass.
|
||||
|
||||
## Risks
|
||||
|
||||
A future standalone terminal UI may want the helper as a package again — reintroduce it with that second consumer rather than keeping the boundary for hypothetical reuse. Moving tests risks blurring app-composition tests with UI-rendering tests; keeping the runtime seam and the colocated unit tests avoids that.
|
||||
@@ -1,28 +0,0 @@
|
||||
# RFC: Remove the `agent/steering` mirror emit
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`agent/steering` is the last remaining transient mirror of a durable session event. The loop's steering drain appends the durable `steering/message { turn, content, source }` and, on the very next line, emits `agent/steering(agent, turn, content, source)` — the identical fact as a fire-and-forget event (`packages/core/agent-loop/src/loop.ts`, `drainSteering`). It has zero production listeners: the only subscriber anywhere is a loop regression test asserting the emit carries `source` — the same fact the durable event already records one line above.
|
||||
|
||||
Both mirror-removal RFCs retained it while explicitly deferring the decision this RFC now makes. The [boundary-mirror removal](../../implemented/simplification/2026-06-20-remove-agent-boundary-mirror-events.md) kept it as "a live control signal, not a boundary"; the [stream-chunk removal](../../implemented/simplification/2026-07-02-remove-stream-chunk-mirror.md) kept it as "a live control signal with no durable twin, retained (its fate is a separate future decision)". The second rationale does not survive the code: the durable twin is `steering/message`, appended immediately before the emit with the same payload. The mirrored-vs-live-only line the taxonomy actually draws puts it on the mirror side: `agent/queued` is genuinely live-only (it fires at enqueue time, before any durable event exists, and already carries a `steering: boolean` flag — cancelled queued work never enters the log), while `agent/steering` fires at the exact moment its durable twin lands, carrying nothing the log does not.
|
||||
|
||||
Steering carries real production traffic — the hook bridges' turn-continuation decisions inject their reasons through `inbox.steer()`, landing as durable `steering/message` events that the hook-matrix goldens pin — and every one of those consumers observes the durable event. Nothing observes the mirror.
|
||||
|
||||
## Proposal
|
||||
|
||||
Remove the `agent/steering` declaration from `packages/core/agent/src/types.ts` (and its mention in the live-events JSDoc list there), the emit in `drainSteering` (whose `ctx` parameter becomes unused and goes too), the row in `packages/core/agent/README.md`, and the emit line in the loop-pseudocode blocks (`packages/core/agent-loop/src/loop.ts` module doc and [architecture.md](../../../architecture.md)); run `pnpm run gen-cordis-catalog`. Retarget the one regression test at the durable `steering/message` event — the source-preservation fact it pins lives on the log. The implementing PR amends the two retaining RFCs' scope lines per [implemented/AGENTS.md](../../implemented/AGENTS.md): the boundary RFC's retained-list entry and the stream-chunk RFC's "no durable twin" clause.
|
||||
|
||||
## Why not keep it?
|
||||
|
||||
"It is a control signal, not a boundary" — but the taxonomy's operative distinction is mirrored-vs-live-only, not control-vs-boundary, and this event mirrors. A consumer that wants enqueue-time notification has `agent/queued` (with its steering flag); a consumer that wants drain-time notification is by definition asking for the moment `steering/message` is appended, which `session/event` delivers with the same payload plus durability. The rejected [retire-mid-turn-steering RFC](../../rejected/simplification/2026-06-20-retire-mid-turn-steering.md) defended the steering *capability* — `steer()`, the durable event, continuation forcing — all of which this removal keeps untouched.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- No `agent/steering` spelling outside this RFC and the two amended RFCs; the catalog is regenerated and fresh.
|
||||
- The retargeted test pins source preservation on `steering/message`; the suite is green.
|
||||
|
||||
## Risks
|
||||
|
||||
None known: zero production listeners exist to migrate, and both live-notification needs (enqueue, drain) have surviving homes (`agent/queued`, `session/event`).
|
||||
@@ -1,27 +0,0 @@
|
||||
# RFC: Share the app bins' boot glue instead of maintaining twin copies
|
||||
|
||||
Status: proposed
|
||||
|
||||
## Problem
|
||||
|
||||
`packages/ui/stdio-agent/src/bin.ts` and `packages/ui/acp-agent/src/bin.ts` carry four near-twin helpers — `loadEnv`, `installFailLoud`, `assertEntriesLoaded`, `boot` — whose bodies differ essentially in the diagnostic prefix, plus two copies of the hardest-won boot lore in the repo: the `Promise.allSettled` swallow inside `loader.await()`, the silent-exit-0 import-failure guard, and the `--expose-internals` resolution note (the failure classes behind AGENTS.md's "real entry path means the published artifact" pattern). Drift has already begun: `boot(configPath)` resolves the path internally in one bin but requires a pre-resolved absolute path in the other, and the twin JSDoc prose has forked.
|
||||
|
||||
The duplication is aggravated by a coverage hole: all of this logic sits OUTSIDE the per-file 100% gate — `vitest.config.ts` excludes `packages/*/*/src/bin.ts` because importing a self-executing bin (top-level `await main()`) runs it — which also makes the `export` keywords on these helpers decorative: no spec can import them, so the only exercisers are the subprocess smokes, and the two `built-bin.e2e.ts` suites duplicate their temp-node_modules scaffolding as well. The genuinely per-app pieces are small and real: the ACP bin owns snapshot-mode config selection (`resolveConfigPath`), replay-mode env skipping, the stdin-EOF dispose lifecycle, and stdout purity; the stdio bin owns nothing extra.
|
||||
|
||||
## Proposal
|
||||
|
||||
Extract the four helpers, parameterized by the bin's diagnostic prefix, into an importable non-bin module shared by both apps — a small published package in the `ui` group (the bins are published artifacts, so their runtime dependency must be published too, not `support/`). Each `bin.ts` becomes a thin self-executing `main()` plus its app-specific glue. The shared module gains unit tests and falls under the coverage gate; the loader-failure lore gets one home; the subprocess smokes remain the artifact-level guard — the published-bin smoke is NOT replaced by unit tests, per the "real entry path" defensive pattern. The implementing PR amends the [extract example app packages RFC](../../implemented/architecture/2026-06-20-extract-example-app-packages.md)'s facts ("boot glue moved into that bin, owned by the app" is the sentence that changes).
|
||||
|
||||
## Why not keep the duplication?
|
||||
|
||||
The bins were framed as independently-owned published artifacts, and a new package carries fixed overhead (manifest, README, tsconfig reference, publint surface) that rivals the deduplicated line count. But app-vs-app sharing was never weighed by that RFC — it consolidated three example `start.ts` copies INTO the bins and stopped there; the drift is now observed fact rather than speculation; and the coverage-gap argument is independent of the dedup argument: this is the only nontrivial runtime logic in the repo exempt from the per-file 100% gate. The alternative of a copy-by-convention shared source file is the current state with extra steps.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- The four helpers exist once, unit-tested, under the coverage gate; both bins are thin mains plus app-specific glue.
|
||||
- Both built-bin smokes still pass under plain node in the node_modules-shaped temp dir, including the missing-config non-zero exit.
|
||||
- The app-packages RFC's facts are amended in the same change.
|
||||
|
||||
## Risks
|
||||
|
||||
Churn in two published bins and one new package boundary; the shared module must stay dependency-light (cordis plus the loader). If the implementing PR finds the package overhead genuinely exceeds the dedup — the honest failure mode of this proposal — the fallback that still pays is extracting only the coverage-exempt pure logic (`assertEntriesLoaded`, `resolveConfigPath`) into an importable module within each app package, ending the coverage exemption without a new package.
|
||||
Reference in New Issue
Block a user