Browse Source

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.
Tianyi Cui 4 tháng trước cách đây
mục cha
commit
4dafad4db6

+ 3 - 1
AGENTS.md

@@ -29,7 +29,9 @@ packages/    Harness packages, all named @deepseek-ai/dsh-<name>:
   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.
 ```
 

+ 47 - 0
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.

+ 38 - 0
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.

+ 40 - 0
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.

+ 43 - 0
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.

+ 49 - 0
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<S>` 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.

+ 39 - 0
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/<pkg>.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.

+ 41 - 0
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.

+ 44 - 0
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<T>` 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.

+ 17 - 0
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 |