docs: accuracy sweep, architecture restructure, two ADRs, review skill

- AGENTS.md Commands: fix typecheck/build descriptions; add lint, lint:fix,
  test:coverage, knip, publint, hygiene (were undocumented).
- Drop the bare `yarn demo` for explicit `demo:echo` + `demo:coding`; update
  README, examples READMEs (and document coding-agent in examples/README).
- New cookbook guide: adding-a-vendored-package.md (the missing "add" half of
  vendor/README's update-only procedure).
- architecture.md: add a table-of-contents and extract the Extension cookbook
  to docs/cookbook/extension-cookbook.md (link-preserving); drop the completed
  "restructure this document" TODO.
- ADR 0009 (capability seams) + 0010 (twin LLM adapters), and a "when to write
  an ADR" standard in adr/README.
- Add a committed dsh-code-review skill under .agents/skills, exposed to Claude
  Code via a tracked .claude/skills symlink (gitignore carve-out).
This commit is contained in:
Tianyi Cui
2026-06-13 18:50:13 +08:00
parent 066f94c7e0
commit 39b3db4b9c
18 changed files with 246 additions and 118 deletions

View File

@@ -0,0 +1,27 @@
# ADR 0009: Capability seams — interface / implementation / consumer split
Status: accepted (2026-06-13)
## Context
The harness has swappable capabilities — bash execution today, sandboxed/remote executors and alternative model providers tomorrow. A capability has three concerns that change at different rates and for different reasons: the *contract* (what the capability is), the *implementation* (how it runs), and the *consumer surface* (what the model and other plugins program against). Bundling them in one package couples those rates of change — swapping a local executor for a sandboxed one would churn the tool schemas the model sees, even though the model-facing contract never changed.
This is distinct from "who provides vs. needs a capability at runtime", which Cordis already answers with services + `inject` (a provider registers `ctx.bash`; a consumer declares `inject: ['bash']` and its fiber pends until the service exists). That mechanism is necessary but doesn't dictate package boundaries; this ADR does.
## Decision
A swappable capability is **three packages**:
1. **Interface** — an abstract service + the vocabulary types, owning the `ctx.<key>` and depending only on cordis (e.g. `dsh-bash`: `BashExecutor`, `BashRunResult`, `BashTask`).
2. **Implementation** — a concrete subclass loaded as a plugin (e.g. `dsh-bash-local`: subprocesses, process-group kills, spill-file truncation). Sandboxed/remote backends are sibling packages implementing the same interface.
3. **Consumer** — what the model and plugins see (e.g. `dsh-tool-bash`: the `bash`/`bash_output`/`bash_kill` tool schemas). Consumers `inject` the interface key and never import implementation types.
Implementation and consumer then evolve independently: a sandboxed executor replaces `dsh-bash-local` without touching a tool schema.
Alternatives considered: **one combined package** — rejected because it recouples the three rates of change the split exists to separate (the whole point). **`@cordisjs/plugin-capability`** — a different axis entirely: it is a permission/capability-*security* service (named permissions with inheritance, tested against a session via `ctx.capability.test`), a candidate for the deferred permissions/sandbox work on the `tools/execute` veto seam, NOT a mechanism for swapping implementations. Confusing the two ("capability") is the trap this ADR names.
The split is not mandatory when the parts are genuinely one concern: the LLM seam folds interface + consumer into `dsh-llm` (the consumer is the loop itself, not a swappable schema surface) with adapters as the implementation packages. Don't split preemptively — a capability with one conceivable implementation and one consumer stays one package until a second appears.
## Consequences
More packages and more boilerplate per capability (a `package.json`/`tsconfig`/README trio, the inject wiring). Bought: implementations and consumers ship and version independently, and a new backend never risks the model-facing contract. The rule is documented in [AGENTS.md](../../AGENTS.md) § Conventions ("Capability seams are three packages") and [architecture.md](../architecture.md) § "Capability seams"; the bash trio is the reference template. When to fold vs. split is a judgment call the architecture doc spells out — this ADR records *why* the default is to split.

View File

@@ -0,0 +1,22 @@
# ADR 0010: Two LLM adapters as a design-verification twin
Status: accepted (2026-06-13)
## Context
`dsh-llm` owns a provider-neutral streaming vocabulary — the `StreamChunk` protocol (`block-start`, `text-delta`, `reasoning-delta`, `tool-call-delta`, `block-end`, `usage`, `finish`) and the content-block types ([ADR 0004](0004-own-content-block-vocabulary.md)). A vocabulary defined against a single adapter risks baking that adapter's quirks into the "neutral" contract: anything the one implementation happens to do becomes the de-facto spec, and the abstraction is unverified until a second provider arrives — by which point the leak is expensive to fix.
## Decision
Ship **two** adapters against the one contract from the start, deliberately built on different internals:
- `dsh-llm-deepseek` — hand-rolled `fetch` + SSE parsing against the DeepSeek API.
- `dsh-llm-pi-ai` — the same endpoint through the `@earendil-works/pi-ai` library (its own event vocabulary).
The rule they enforce: **anything the StreamChunk vocabulary cannot express for BOTH implementations is a core-vocabulary bug**, caught immediately rather than at the next provider. The pair pinned down conventions now documented on `StreamChunk` in `dsh-llm/src/types.ts`: usage emitted before finish, nothing after finish, tool-call `arguments` as raw JSON strings end-to-end, and the two sanctioned error paths (throw from `stream()` *or* end with `finish {kind:'error'|'aborted'}`) that a consumer must handle on both sides — a divergence the library-backed adapter surfaced that a single hand-rolled adapter would have hidden.
Alternatives considered: **a single adapter** — less code and half the e2e cost, but leaves the "provider-neutral" claim unverified; the vocabulary would encode DeepSeek-via-fetch assumptions silently. **A mock second adapter** — cheaper but doesn't exercise a real provider's wire quirks, so it proves little. The twin is real-on-real.
## Consequences
Double the adapter maintenance and double the key-gated e2e surface (both adapters cover V4 Flash and Pro across representative thinking/effort modes). Bought: a continuously-verified neutrality guarantee for the most leak-prone abstraction in the codebase, and a worked second example for adapter authors. The two share the core Config shape (`apiKey`/`baseURL`/`models`) so a deployment swaps mostly one line, but the reasoning knob differs — `dsh-llm-deepseek` takes `thinking`/`reasoningEffort`, `dsh-llm-pi-ai` takes a single `reasoning` level — so a swap translates that field. If the maintenance cost ever outweighs the verification value (e.g. once conformance tests from [RFC 004](../rfc/004-architectural-conformance.md) cover the contract mechanically), retiring the twin to a single adapter + the conformance kit would be a new ADR superseding this one.

View File

@@ -4,6 +4,12 @@ Short, immutable records of the *why* behind decisions that shape this codebase.
Format: one file per decision, numbered, with Status / Context / Decision / Consequences. An ADR is never edited into a different decision — supersede it with a new one and cross-link.
## When to write an ADR
Write one when a decision is all three of: **durable** (it shapes the codebase beyond a single function or package), **contested** (there was a real alternative you rejected, and a reasonable engineer might have chosen it), and **surprising** (a future reader would otherwise ask "why on earth is it done this way?"). The ADR captures the *why* and *what we gave up* — the parts code and docs can't.
Do NOT write an ADR for: a mechanical or local choice (a variable name, a one-file refactor); anything already enforced and explained by a gate or a convention in AGENTS.md; or a still-provisional decision tagged `TODO(...)` in the code — record those as TODOs and promote to an ADR only once they settle. When in doubt, the test is the "why on earth" question: if the code alone would mislead a careful reader about intent, write the ADR.
| # | Title | Status |
|---|---|---|
| [0001](0001-vendor-cordis-as-source.md) | Vendor Cordis as source, not npm dependencies | accepted |
@@ -14,3 +20,5 @@ Format: one file per decision, numbered, with Status / Context / Decision / Cons
| [0006](0006-tool-schemas-in-prompt-assembly.md) | Tool schemas are part of the system-prompt assembly | accepted |
| [0007](0007-quality-gates.md) | Mechanical quality gates over prose guidelines | accepted |
| [0008](0008-tsdown-over-dumble.md) | tsdown for JS bundling instead of dumble | accepted |
| [0009](0009-capability-seams.md) | Capability seams — interface / implementation / consumer split | accepted |
| [0010](0010-twin-llm-adapters.md) | Two LLM adapters as a design-verification twin | accepted |

View File

@@ -8,6 +8,8 @@ The harness core is deliberately tiny: a handful of abstract services plus one c
Requirement context: [Coding Harness MVP 需求分析][mvp-doc].
**Contents:** [Layering](#layering) · [Service map](#service-map) · [Capability seams](#capability-seams-interface--implementation--consumer) · [The vocabulary (dsh-llm)](#the-vocabulary-dsh-llm) · [Event-sourced sessions](#event-sourced-sessions-dsh-session) · [Prompt assembly](#prompt-assembly-dsh-system-prompt) · [Tool pipeline](#tool-pipeline-dsh-tools) · [Agents and the loop](#agents-dsh-agent-and-the-loop-dsh-agent-loop) ([lifecycle](#loop-lifecycle-session--turn--step), [event taxonomy](#event-taxonomy), [waterfall semantics](#cordis-waterfall-semantics-important)) · [Plugin sanity checklist](#plugin-sanity-checklist) · [Extension cookbook](#extension-cookbook) · [Deferred work](#deferred-work-todo)
[microkernel-doc]: https://trtgsjkv6r.feishu.cn/wiki/VS9Lw1kQki6mDJk2UHocyuphnsc
[mvp-doc]: https://trtgsjkv6r.feishu.cn/wiki/ZwK6wfBE9i91V6kzMGYcgRGanxg
@@ -86,7 +88,7 @@ A `Session` is an append-only log of typed `SessionEvent`s — the single source
- `user/message` → user message
- `assistant/message` → assistant message (raw `assistant/chunk` events are replay/UI data and are skipped in derivation)
- `tool/result` → user message carrying a `tool-result` block
- `context/message`, `steering/message` → user-role messages wrapped in a tagged envelope (`<context source="…">…</context>`) at their chronological position — the "system-reminder" pattern; models distinguish them from real user prompts by the envelope. **TODO(review)**: revisit the envelope once a real adapter exists.
- `context/message`, `steering/message` → user-role messages wrapped in a tagged envelope (`<context source="…">…</context>`) at their chronological position — the "system-reminder" pattern; models distinguish them from real user prompts by the envelope. **TODO(review)**: the real adapters now exist (the original precondition); the envelope still wants a deliberate review against live model behavior (`TODO(review)` in dsh-session).
Replay/fork = `ctx.sessions.create(id, seedEvents)`. Trace/telemetry = listen to `session/event`.
@@ -104,7 +106,7 @@ Tool schemas are deliberately **part of the assembly**: "what the model is told
`execute()` runs through the **`tools/execute` waterfall** — the single seam where sandbox, permission, hooks, and plan-mode plugins wrap or veto a call. This collapses Claude Code's validate → PreToolUse → permission → execute → PostToolUse pipeline into ordered waterfall listeners.
**TODO**: tool shapes get revisited when real tools land (e.g. a concurrency-safety hint for parallel execution; phase 1 executes tool calls sequentially).
**TODO**: tool shapes get revisited now that real tools exist (the bash suite landed; the `TODO(review)` in dsh-tools is still open) — e.g. a concurrency-safety hint for parallel execution; phase 1 executes tool calls sequentially.
## Agents (dsh-agent) and the loop (dsh-agent-loop)
@@ -222,75 +224,12 @@ Every MVP feature (including the TODO-marked ones), with the mechanism that impl
## Extension cookbook
### A tool plugin
```ts
import type { Context } from 'cordis'
import { defineTool } from '@deepseek-ai/dsh-tools'
export const name = 'my-tool'
export const inject = ['tools']
export function apply(ctx: Context) {
ctx.tools.register(defineTool({
name: 'read_file',
description: 'Read a file from disk.',
parameters: {
path: { type: 'string', required: true, description: 'Absolute file path' },
},
async execute(args) {
// args is typed: { path: string }
const text = await readFile(args.path, 'utf8')
return [{ type: 'text', text }]
},
}))
}
```
(Raw JSON-Schema `ToolDefinition`s are still accepted by `ctx.tools.register()` directly — that's how MCP-sourced tools arrive. `defineTool` is the typed sugar for first-party tools.)
### A hook plugin (permission gate)
```ts
export const name = 'permission-gate'
export function apply(ctx: Context) {
ctx.on('tools/execute', async (exec, next) => {
if (!(await isAllowed(exec))) {
return {
callId: exec.callId,
content: [{ type: 'text', text: 'Denied by policy.' }],
isError: true,
}
}
return next()
})
}
```
### A UI plugin
```ts
export const name = 'my-ui'
export const inject = ['agents']
export function apply(ctx: Context) {
ctx.on('agent/stream-chunk', (agent, turn, step, chunk) => {
if (chunk.type === 'text-delta') render(chunk.text)
})
onUserInput(text => ctx.agents.get('main')?.send([{ type: 'text', text }]))
}
```
Two complete runnable wirings exist: [`examples/echo-agent`](../examples/echo-agent) (mock model + echo tool — the all-mock skeleton check) and [`examples/coding-agent`](../examples/coding-agent) (DeepSeek V4 + the bash tool suite — the real thing; `yarn demo:coding`). Both load from `cordis.yml` with HMR.
Step-by-step guides live in [`docs/cookbook`](./cookbook): adding a package, adding a tool, adding an LLM adapter.
Code skeletons for the three plugin shapes (tool, hook/permission-gate, UI) and the two runnable example wirings live in [docs/cookbook/extension-cookbook.md](./cookbook/extension-cookbook.md). Step-by-step guides: [adding a package](./cookbook/adding-a-package.md), [adding a tool](./cookbook/adding-a-tool.md), [adding an LLM adapter](./cookbook/adding-an-llm-adapter.md), [adding a vendored package](./cookbook/adding-a-vendored-package.md).
## Deferred work (TODO)
Tracked here deliberately — each is designed-for but not implemented:
- **Restructure this document** — it has grown long; split it into focused sections (or per-area files) so readers can navigate it without scrolling the whole thing.
- **Sub-agent spawn/fork semantics** (seam: `AgentLoop.create()`); inter-agent channels beyond `send`/`steer`/events.
- **Persistence backends** (JSONL session dirs, sqlite) on the `session/event` + `session/flush` seam.
- **Compaction implementation** (auto thresholds, summarization prompts) on the `agent/request` seam, with its session-event types added by declaration merging.

View File

@@ -47,7 +47,7 @@ Follow tool-bash's background pattern: a `run_in_background` flag returns a task
## Permissions / sandboxing
Prefer not to build policy into the tool. The seam is the `tools/execute` waterfall (veto or wrap — see the permission-gate example in docs/architecture.md), or a sandboxing implementation behind the tool's executor seam.
Prefer not to build policy into the tool. The seam is the `tools/execute` waterfall (veto or wrap — see the permission-gate example in [extension-cookbook.md](./extension-cookbook.md)), or a sandboxing implementation behind the tool's executor seam.
## Tests every tool needs

View File

@@ -0,0 +1,56 @@
# Cookbook: adding a vendored package
When the harness needs another upstream Cordis package (e.g. `@cordisjs/plugin-http`), it is **vendored** as pinned source under `vendor/`, not added as an npm dependency — see [ADR 0001](../adr/0001-vendor-cordis-as-source.md) for why. [vendor/README.md](../../vendor/README.md) covers *updating* an already-vendored package; this guide is the file-by-file checklist for adding a **new** one. (Verified against the existing vendored set; if it drifts, fix it here.)
## 1. Copy the source in
```
vendor/<dir>/
package.json # from upstream; set "private": true, keep name/exports/type
tsconfig.json # extends ../../tsconfig.base.json (see shape below)
src/ # the upstream src/ verbatim
README.md LICENSE # if upstream ships them
```
`tsconfig.json` mirrors the other vendored packages — `rootDir: src`, `outDir: lib`, the strictness relaxations upstream code needs, and a `references` entry for every other vendored package it imports:
```jsonc
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"rootDir": "src", "outDir": "lib",
"noUncheckedIndexedAccess": false, "exactOptionalPropertyTypes": false,
"noImplicitOverride": false, "noUnusedLocals": false, "noUnusedParameters": false
},
"include": ["src"],
"references": [{ "path": "../cordis" }, { "path": "../cosmokit" }]
}
```
`package.json` invariants: `"private": true` (vendored packages are never published), keep upstream's `name`/`version`/`exports`/`type`, and list its cordis deps in `peerDependencies` (matching the upstream manifest). Transitive upstream deps must themselves be vendored or already present — vendoring one package often means vendoring its dependency tree (e.g. `@cordisjs/plugin-http` pulls `@cordisjs/fetch-file`).
## 2. Register it in the root configs
| File | Change |
|---|---|
| `tsconfig.base.json` | add `"<npm-name>": ["./vendor/<dir>/src"]` to `paths` |
| `tsconfig.typecheck.json` | add `"<npm-name>": ["./vendor/<dir>/lib"]` — this file points at built declarations, not src. If the package's `types` entry isn't `lib/index.d.ts`, point at that built file instead (e.g. `logger-console` maps to `./vendor/logger-console/lib/shared`, matching its `"types": "lib/shared.d.ts"`). |
| `tsconfig.build.json` | add `{ "path": "./vendor/<dir>" }` to `references` (before the `packages/*` entries) |
| `vendor/README.md` | add a manifest table row (dir, npm name, version, upstream repo, commit SHA) and log any local modifications |
| `scripts/publint-all.ts` | only if the vendored package is itself published from here (vendored deps normally are not — skip) |
Covered automatically by globs — no edits needed: root `package.json` workspaces (`vendor/*`), `tsdown.config.ts`, `vitest.config.ts`, `eslint.config.mjs`. A per-package `vendor/<dir>/tsdown.config.ts` is needed ONLY if the build shape diverges from the root default (dual ESM/CJS or multiple entries — see `vendor/schemastery` and `vendor/logger-console`).
## 3. Mind the manifest guard
`scripts/check-vendor-manifest.sh` (a pre-commit hook) fails if anything under `vendor/*/src` is staged without `vendor/README.md` also staged. Stage the manifest update alongside the source so the commit passes.
## 4. Verify
```sh
yarn install # registers the workspace
yarn typecheck # the base→lib path split means: run once after a fresh add
yarn build && yarn test && yarn constraints
```
Note the `tsconfig` two-map split (called out in [AGENTS.md](../../AGENTS.md) § Secrets/.env): `lint`'s type-aware rules resolve vendored packages through their built `lib/` declarations, so run `yarn typecheck` (which builds them) once after adding the package or lint reports unresolved-type errors.

View File

@@ -0,0 +1,48 @@
# Cookbook: extension plugin shapes
The three plugin shapes you write against the harness extension surface, as illustrative snippets (elided imports and helper stubs — not copy-paste-complete). For the full step-by-step guides see [adding a package](./adding-a-package.md), [adding a tool](./adding-a-tool.md), and [adding an LLM adapter](./adding-an-llm-adapter.md); for the seams these hook into see [docs/architecture.md](../architecture.md).
## A tool plugin
A tool registers on `ctx.tools`. The annotated `defineTool` example (typed `execute` args, result shaping, the `run_in_background` pattern) lives in [adding-a-tool.md](./adding-a-tool.md) — that guide is the source of truth for the tool shape. Raw JSON-Schema `ToolDefinition`s are also accepted by `ctx.tools.register()` directly (that is how MCP-sourced tools arrive); `defineTool` is the typed sugar for first-party tools.
## A hook plugin (permission gate)
A hook wraps the `tools/execute` waterfall to veto or rewrite a call — the seam where sandbox, permission, and plan-mode plugins live.
```ts
export const name = 'permission-gate'
export function apply(ctx: Context) {
ctx.on('tools/execute', async (exec, next) => {
if (!(await isAllowed(exec))) {
return {
callId: exec.callId,
content: [{ type: 'text', text: 'Denied by policy.' }],
isError: true,
}
}
return next()
})
}
```
## A UI plugin
A UI plugin consumes `agent/stream-chunk` and session events for rendering, and drives input back in via `agent.send()` / `agent.steer()`.
```ts
export const name = 'my-ui'
export const inject = ['agents']
export function apply(ctx: Context) {
ctx.on('agent/stream-chunk', (agent, turn, step, chunk) => {
if (chunk.type === 'text-delta') render(chunk.text)
})
onUserInput(text => ctx.agents.get('main')?.send([{ type: 'text', text }]))
}
```
## Runnable wirings
Two complete examples load their plugin trees from `cordis.yml` with HMR: [`examples/echo-agent`](../../examples/echo-agent) (mock model + echo tool — the all-mock skeleton check, `yarn demo:echo`) and [`examples/coding-agent`](../../examples/coding-agent) (DeepSeek V4 + the bash tool suite — the real thing, `yarn demo:coding`).