From 4dafad4db6f6f2e362cfe26be5e3e8063e74b7e0 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 11 Jun 2026 15:27:07 +0800 Subject: [PATCH] Add RFCs for the remaining quality-proposal ideas Eight proposals grouped by category, each with problem statement, concrete plan, and risks: property-based testing over the protocol-shaped core (chunk streams, event logs, schema DSL); mutation testing as the counterweight to the 100%-coverage gate; deterministic tests + a universal replay-invariant fixture + nightly race stress; architectural conformance (dependency-cruiser rules and the LlmAdapter conformance kit); runtime arg validation at the model boundary with a structured error taxonomy and dev-mode invariants; doc-sync enforcement (typechecked doc snippets, API reports); supply-chain checks and nightly vendor-drift verification against the manifest; and deep-readonly public surfaces (logged-vs-in-flight mutability boundary). AGENTS.md points at docs/adr and docs/rfc. --- AGENTS.md | 4 +- docs/rfc/001-property-based-testing.md | 47 ++++++++++++++++++ docs/rfc/002-mutation-testing.md | 38 ++++++++++++++ .../003-deterministic-and-stress-testing.md | 40 +++++++++++++++ docs/rfc/004-architectural-conformance.md | 43 ++++++++++++++++ ...5-runtime-validation-and-error-taxonomy.md | 49 +++++++++++++++++++ docs/rfc/006-doc-sync-and-api-reports.md | 39 +++++++++++++++ docs/rfc/007-supply-chain-and-vendor-drift.md | 41 ++++++++++++++++ docs/rfc/008-immutable-public-surfaces.md | 44 +++++++++++++++++ docs/rfc/README.md | 17 +++++++ 10 files changed, 361 insertions(+), 1 deletion(-) create mode 100644 docs/rfc/001-property-based-testing.md create mode 100644 docs/rfc/002-mutation-testing.md create mode 100644 docs/rfc/003-deterministic-and-stress-testing.md create mode 100644 docs/rfc/004-architectural-conformance.md create mode 100644 docs/rfc/005-runtime-validation-and-error-taxonomy.md create mode 100644 docs/rfc/006-doc-sync-and-api-reports.md create mode 100644 docs/rfc/007-supply-chain-and-vendor-drift.md create mode 100644 docs/rfc/008-immutable-public-surfaces.md create mode 100644 docs/rfc/README.md diff --git a/AGENTS.md b/AGENTS.md index d3403b0768..23c35ed3d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,9 @@ packages/ Harness packages, all named @deepseek-ai/dsh-: agent-loop/ THE concrete plugin: LoopAgent + the loop driver examples/ Runnable demos (not workspaces). echo-agent = mock model + echo tool + stdio UI + JSONL persistence, wired via cordis.yml. -docs/ architecture.md — the design doc. +docs/ architecture.md — the design doc. adr/ — decision records (the + why behind vendoring, event-sourcing, the schema DSL, …). + rfc/ — proposals for substantial future work. scripts/ build.ts — dumble JS bundling for all packages. ``` diff --git a/docs/rfc/001-property-based-testing.md b/docs/rfc/001-property-based-testing.md new file mode 100644 index 0000000000..78193496de --- /dev/null +++ b/docs/rfc/001-property-based-testing.md @@ -0,0 +1,47 @@ +# RFC 001: Property-based testing for protocol-shaped code + +Status: proposed + +## Problem + +Example-based tests pin the cases we thought of. The harness's core is +protocol-shaped — chunk streams, event logs, schema conversion — where the +input space is combinatorial and the interesting bugs live in interleavings +nobody wrote an example for (the `streamBlocks` ordering bug survived 100% +line coverage of the happy paths). + +## Proposal + +Adopt fast-check (vitest integration) with generators for our vocabulary: + +- **BlockAssembler**: arbitrary chunk sequences (valid and malformed — + duplicate indices, stragglers after block-end, missing block-start). + Invariants: `flushReady() + flushRemaining() ≡ blocks()` in order; + `streamBlocks ≡ generate().message.content`; memory bounded (partials map + size ≤ distinct indices); idempotent re-assembly. +- **Session**: arbitrary event logs (seeded generators over SessionEventMap). + Invariants: `deriveMessages` deterministic; replay-from-seed produces + identical derivation; seq strictly monotonic; derived history unaffected by + non-message events. +- **Schema DSL**: arbitrary SchemaSpecs. Invariants: generated JSON Schema's + `required` array equals the `required: true` keys at every nesting level; + conversion is total (never throws); generated args satisfying `InferArgs` + validate against the generated schema (once RFC 005's validator exists — + the two RFCs compose). +- **Inbox/loop**: arbitrary send/steer/abort schedules against a scripted + adapter. Invariants: no message lost (every send/steer appears in the log + exactly once), turn numbers strictly increase, status transitions follow + idle→running→idle/disposed. + +## Plan + +One `tests/properties.spec.ts` per package; fast-check as devDependency; +numRuns tuned so the suite stays under ~10s locally, with a nightly CI job +running 100× the iterations. Failures persist their seed in the report so +agents can reproduce deterministically. + +## Risks + +Generator quality determines value — invest in generators that produce +*realistic-but-adversarial* streams, not uniform noise. Property flake from +timeouts must be treated as a finding, not retried away. diff --git a/docs/rfc/002-mutation-testing.md b/docs/rfc/002-mutation-testing.md new file mode 100644 index 0000000000..0ac4f95e08 --- /dev/null +++ b/docs/rfc/002-mutation-testing.md @@ -0,0 +1,38 @@ +# RFC 002: Mutation testing as the coverage counterweight + +Status: proposed + +## Problem + +The per-file 100% coverage gate (ADR 0007) proves every line *executes* under +test — not that any assertion would notice if the line were wrong. Under +agent-written tests, coverage pressure can produce execution-without-assertion. +Mutation testing measures what coverage cannot: whether the suite *kills* +deliberately injected bugs. + +## Proposal + +Stryker (`@stryker-mutator/vitest-runner`) over `packages/*/src`: + +- **PR-scoped incremental runs** (changed files only) as a CI job — fast + enough to gate merges once tuned. +- **Nightly full runs** with a tracked mutation score; start by recording, + then set the threshold at the observed baseline and ratchet upward (same + policy as coverage: thresholds only ever tighten). +- Surviving mutants are work items: an agent picks a survivor, writes the + killing test, repeats — a well-shaped autonomous loop. +- Equivalent mutants (provably behavior-preserving) get annotated exclusions + with reasons, mirroring the `/* v8 ignore */` policy. + +## Plan + +1. Add Stryker config scoped to one package (llm — smallest, most algorithmic) + and measure runtime. +2. Expand to all packages; record baseline scores in the config. +3. Wire the nightly job; add the incremental PR job once runtime is acceptable. + +## Risks + +Runtime: mutation testing is expensive; per-file 100% coverage helps (every +mutant is at least reached). If PR-scoped runs stay too slow, keep them +nightly-only and rely on the score ratchet. diff --git a/docs/rfc/003-deterministic-and-stress-testing.md b/docs/rfc/003-deterministic-and-stress-testing.md new file mode 100644 index 0000000000..2b1bcb5e2e --- /dev/null +++ b/docs/rfc/003-deterministic-and-stress-testing.md @@ -0,0 +1,40 @@ +# RFC 003: Deterministic tests, the replay invariant fixture, and race stress + +Status: proposed + +## Problem + +Several loop tests synchronize with `setTimeout(30)` sleeps — flakiness debt +that wastes agent cycles on retries and can mask ordering bugs. Separately, +our core architectural promise (any session log replays to identical derived +history) is asserted in two tests but is cheap to assert *everywhere*. And +the inbox wakeup race was verified by hand exactly once; nothing re-verifies +it continuously. + +## Proposal + +Three measures: + +1. **No wall-clock sleeps in tests.** Replace `setTimeout(N)` waits with + event-driven waits (the existing `waitForIdle` pattern, extended to + `waitForStatus`, `waitForEvent(n)`) or vitest fake timers where time + itself is under test. Enforce with a lint rule banning `setTimeout` in + `packages/*/tests` outside an allowlisted helper module. +2. **Universal replay fixture.** A shared test helper wraps the loop harness + so that after every test, the agent's session log is replayed into a fresh + Session and `deriveMessages()` equality is asserted automatically. The + invariant then gets checked hundreds of times per CI run across every + scenario the suite produces, not twice. +3. **Nightly race stress.** A CI job running the agent-loop and inbox suites + with `vitest --repeat=200` (and `--shuffle`) to flush scheduling-dependent + failures; any flake found is a bug to fix, never a retry. + +## Plan + +Land 1 and 2 together (they touch the same helpers); add the nightly job +after the suite is sleep-free so repeats are fast. + +## Risks + +Fake timers interact subtly with Promise scheduling in the loop — prefer +event-driven waits; reserve fake timers for timer-service behavior itself. diff --git a/docs/rfc/004-architectural-conformance.md b/docs/rfc/004-architectural-conformance.md new file mode 100644 index 0000000000..338659557a --- /dev/null +++ b/docs/rfc/004-architectural-conformance.md @@ -0,0 +1,43 @@ +# RFC 004: Architectural conformance — dependency rules and the adapter kit + +Status: proposed + +## Problem + +Two architectural guarantees currently live only in prose: (1) nothing +depends on the concrete loop package (the microkernel promise, ADR 0002), and +(2) every LlmAdapter speaks the chunk protocol correctly. Both should be +mechanical (ADR 0007). + +## Proposal + +**dependency-cruiser** with rules: + +- `packages/*` (except agent-loop's own tests and examples/) must not import + `@deepseek-ai/dsh-agent-loop`. +- No cross-package deep imports (`@deepseek-ai/dsh-*/src/...` paths) — public + entry points only. +- No import cycles anywhere in packages/. +- `vendor/*` must not import from `packages/*`. +- Layering: dsh-llm imports nothing from other dsh packages; dsh-session only + dsh-llm; etc. (the dependency table in packages/README.md, enforced). + +**Adapter conformance kit** in dsh-llm (`@deepseek-ai/dsh-llm/conformance`): +a reusable vitest suite parameterized by an adapter factory, asserting the +chunk-protocol contract — index monotonicity per block, no deltas after +`block-end` for an index, exactly one `finish`, usage at most once, every +`tool-call-delta` carries the call id, abort honored promptly. Run it against +the mocks now; the DeepSeek V4 adapter inherits it on day one. Optionally a +dev-mode `strictAdapter()` wrapper enforcing the same at runtime behind a +debug flag (pairs with RFC 005's invariants). + +## Plan + +dependency-cruiser config + CI step first (an hour of work, permanent +guarantee); the conformance kit lands with its first consumer test against +MockAdapter, and is a prerequisite for the V4 adapter phase. + +## Risks + +Dep-cruiser rule maintenance as packages are added — keep rules pattern-based +(`dsh-*`) rather than enumerated. diff --git a/docs/rfc/005-runtime-validation-and-error-taxonomy.md b/docs/rfc/005-runtime-validation-and-error-taxonomy.md new file mode 100644 index 0000000000..742152e0cd --- /dev/null +++ b/docs/rfc/005-runtime-validation-and-error-taxonomy.md @@ -0,0 +1,49 @@ +# RFC 005: Runtime validation at the model boundary, error taxonomy, dev-mode invariants + +Status: proposed + +## Problem + +Three gaps where compile-time guarantees stop: + +1. Tool args are model-generated JSON — `defineTool`'s `InferArgs` claim + is only as true as the model's output. Today a malformed call reaches + `execute` untyped-in-practice. +2. Tool errors flatten to a text block; name/code/stack are lost, so future + sandbox/retry plugins can't distinguish ENOENT from EACCES, and the model + gets less actionable feedback than it could. +3. Loop ordering invariants (seq monotonicity, step/turn event nesting, + turn-number continuity) are asserted only where tests look. + +## Proposal + +1. **Schema validation in defineTool**: before `execute`, validate parsed + args against the SchemaSpec (the converter already encodes the structure — + a small interpreter walks it: presence of required keys, primitive type + checks, enum membership, recursion into objects/arrays). On mismatch, + return an `isError` ToolExecutionResult describing the violation — the + model can self-correct. Raw-registered tools (MCP) keep validating their + own input. +2. **Structured error taxonomy**: per-package error classes extending a + common `HarnessError` (name, `code`, `cause` chaining). + `ToolExecutionResult` gains optional `error: { name, code }` alongside the + model-facing text. The loop's `errorData` consumes it; session `error` + events carry the code. This also properly fixes the non-Error-throw + message degradation found in review. +3. **Dev-mode invariants**: a `dsh-invariants` debug plugin (everything is a + plugin — it's just listeners) asserting, when enabled: session seq strictly + increases; `step/start` precedes its chunks; `turn/start`/`turn/end` pair + and nest; tool/call has a matching tool/result; status transitions are + legal. Enabled in tests and the demo; off in production. Doubles as + executable documentation of the event contract. + +## Plan + +2 first (taxonomy is a dependency of 1's error shape), then 1, then 3. +Property tests (RFC 001) then close the loop: generated args ↔ validator ↔ +InferArgs agreement. + +## Risks + +Validator/InferArgs drift — covered by the RFC 001 composition property. +Validation cost per call is negligible next to a model call. diff --git a/docs/rfc/006-doc-sync-and-api-reports.md b/docs/rfc/006-doc-sync-and-api-reports.md new file mode 100644 index 0000000000..cc3a546173 --- /dev/null +++ b/docs/rfc/006-doc-sync-and-api-reports.md @@ -0,0 +1,39 @@ +# RFC 006: Doc-sync enforcement and API reports + +Status: proposed + +## Problem + +AGENTS.md policy says docs and code must stay strictly in sync, but sync is +verified by eyeball. Review has already caught drift twice (a cookbook +example contradicting the type policy; a README citing the wrong +registerAdapter call). Public API changes are similarly invisible — nothing +makes "this commit changed the public surface" an explicit, reviewable fact. + +## Proposal + +1. **Typecheck documentation code blocks.** A script extracts fenced ```ts + blocks from README.md / docs/architecture.md / packages/*/README.md into a + temp project resolving workspace packages, and runs tsc. Blocks that are + intentionally elided get an explicit `ts ignore-check` info string — + opt-out is visible in the source. (twoslash is the fancier alternative; + start with plain extraction.) +2. **Generate or verify the event-taxonomy table.** The table in + docs/architecture.md duplicates the `Events` declarations. Either generate + it from source (ts-morph walk over the `declare module 'cordis'` blocks) + or CI-assert that every declared event name appears in the table and vice + versa. +3. **API reports.** api-extractor (or `tsc --emitDeclarationOnly` + a + normalized public-surface dump) producing a checked-in `etc/.api.md` + per package; CI fails if regeneration differs. Every public-API change + becomes a diff line a reviewer (or review agent) must see. + +## Plan + +1 is a standalone script + CI step. 3 next (it also documents the surface for +plugin authors). 2 last — verify-don't-generate is likely sufficient. + +## Risks + +Doc blocks often show fragments; the ignore-check escape hatch must stay rare +or the gate is theater — lint the ratio if needed. diff --git a/docs/rfc/007-supply-chain-and-vendor-drift.md b/docs/rfc/007-supply-chain-and-vendor-drift.md new file mode 100644 index 0000000000..8f639b84fc --- /dev/null +++ b/docs/rfc/007-supply-chain-and-vendor-drift.md @@ -0,0 +1,41 @@ +# RFC 007: Supply chain checks and vendor drift verification + +Status: proposed + +## Problem + +The vendor manifest (ADR 0001) is enforced at commit time in the *forward* +direction (vendored change ⇒ manifest update) but nothing verifies the +manifest's *claims*: that vendor/ actually equals upstream-at-SHA plus +exactly the logged modifications. And the handful of true npm dependencies +have no advisory monitoring or update cadence. + +## Proposal + +1. **Vendor drift check** (nightly CI): clone the upstream repos at the + manifest SHAs (shallow), copy the corresponding package sources, and diff + against `vendor/*/src`. The job fails unless the diff matches the logged + local modifications (kept as a checked-in patch file per modification — + the log entries become verifiable artifacts rather than prose). +2. **Dependency advisories**: osv-scanner (or `yarn npm audit`) job on the + lockfile, scheduled + on lockfile-touching PRs. +3. **License inventory**: a script asserting every vendored package carries + its LICENSE and that package.json `license` fields match the inventory in + vendor/README.md (we mix vendored MIT with our BSD-3) — CI step. +4. **Renovate** (or a scheduled agent task) proposing npm dependency updates + in small PRs that ride the full gate suite; vendored packages are excluded + (their updates follow the manifest sync procedure, ideally as a + semi-automated agent workflow: fetch upstream, re-apply patches, run + gates, open PR with the manifest table updated). + +## Plan + +3 is trivial — do first. 1 requires network access from CI to the upstream +repos (private — needs a token) and converting the two existing logged +modifications into patch files. 2 and 4 are config. + +## Risks + +Upstream repos are private mirrors; CI credentials and availability are the +main friction for the drift check. If blocked, run it as a local scheduled +agent task instead of CI. diff --git a/docs/rfc/008-immutable-public-surfaces.md b/docs/rfc/008-immutable-public-surfaces.md new file mode 100644 index 0000000000..ca0b6b6eff --- /dev/null +++ b/docs/rfc/008-immutable-public-surfaces.md @@ -0,0 +1,44 @@ +# RFC 008: Deep-readonly public surfaces + +Status: proposed + +## Problem + +The session log is append-only by contract, but `session.events` returns +`readonly SessionEvent[]` whose *elements* are mutable: a plugin can reach in +and rewrite history (`events[0].data.content.push(...)`), silently breaking +replay equivalence and the derived-history guarantee. The same applies to +derived messages and prompt assemblies passed through waterfalls — mutation +is sometimes the intended idiom (waterfall middleware mutates the request) +and sometimes corruption (mutating a *logged* event), and the types don't +distinguish. + +## Proposal + +Make immutability part of the type where mutation is corruption: + +- `SessionEvent` data becomes `DeepReadonly` on the way OUT of a session + (`events`, `session/event` listeners); `append()` keeps taking plain + mutable input. A `DeepReadonly` utility type lands in dsh-llm next to + the brand/never helpers. +- `deriveMessages()` returns deep-readonly messages; the loop clones before + handing a mutable request to the `agent/request` waterfall (mutation there + is sanctioned — the clone makes the boundary explicit and cheap, once per + step). +- `PromptAssembly` stays mutable through its waterfall (sanctioned) but the + registry's internal section list is cloned per assembly (already true). +- Optionally, dev-mode `Object.freeze` of event data behind the RFC 005 + invariants flag, so sanctioned-mutation violations throw in tests rather + than corrupting silently. + +## Plan + +Introduce `DeepReadonly`, flip the session read paths, fix resulting +compile errors in consumers (expected: a handful in tests), add the +freeze-in-dev option alongside RFC 005's invariants plugin. + +## Risks + +`DeepReadonly` types can produce noisy errors at waterfall boundaries where +mutation IS the API — keep the mutable/readonly boundary exactly at "logged +vs in-flight" and document it in the session README. diff --git a/docs/rfc/README.md b/docs/rfc/README.md new file mode 100644 index 0000000000..b7e50632bf --- /dev/null +++ b/docs/rfc/README.md @@ -0,0 +1,17 @@ +# RFCs + +Proposals for substantial future work — reviewed before implementation, +unlike ADRs (which record decisions already made). Each RFC groups a related +set of ideas from the quality/robustness proposal (2026-06-11); statuses +move proposed → accepted → implemented (then usually graduate to an ADR). + +| # | Title | Status | +|---|---|---| +| [001](001-property-based-testing.md) | Property-based testing for protocol-shaped code | proposed | +| [002](002-mutation-testing.md) | Mutation testing as the coverage counterweight | proposed | +| [003](003-deterministic-and-stress-testing.md) | Deterministic tests + replay invariant fixture + race stress | proposed | +| [004](004-architectural-conformance.md) | Architectural rules: dependency-cruiser, adapter conformance kit | proposed | +| [005](005-runtime-validation-and-error-taxonomy.md) | Runtime arg validation, structured error taxonomy, dev-mode invariants | proposed | +| [006](006-doc-sync-and-api-reports.md) | Doc-sync enforcement and API extractor reports | proposed | +| [007](007-supply-chain-and-vendor-drift.md) | Supply chain checks and vendor drift verification | proposed | +| [008](008-immutable-public-surfaces.md) | Deep-readonly public surfaces | proposed |