Merge branch 'split/agent-factory' into split/session-persistence-sqlite
# Conflicts: # yarn.lock
This commit is contained in:
Tianyi Cui
@@ -13,7 +13,7 @@ Every AGENTS.md promise gets a command that exits non-zero, wired into git hooks
|
||||
- Max-strict TypeScript (`noUncheckedIndexedAccess`, `exactOptionalPropertyTypes`, …); tests and examples typecheck in CI via `tsconfig.typecheck.json` (vendored packages resolve as built declarations).
|
||||
- ESLint strict-type-checked + @stylistic (the house style, enforced); vendored code excluded.
|
||||
- Per-file 100% coverage on `packages/*/src` (v8); unreachable defensive guards carry `/* v8 ignore */ ` with stated reasons instead of deletion.
|
||||
- knip (dead code/deps), publint (package correctness), yarn constraints (workspace rules: private, cordis peer+dev, uniform version, ESM).
|
||||
- knip (dead code/deps), publint (package correctness), workspace constraints (workspace rules: private, cordis peer+dev, uniform version, ESM).
|
||||
- lefthook pre-commit (lint staged, typecheck, vendor-manifest guard) and pre-push (tests, hygiene); CI runs the full matrix on node 24/26 plus a demo smoke test driving the echo-agent end to end.
|
||||
|
||||
## Consequences
|
||||
|
||||
@@ -6,16 +6,16 @@ Status: accepted (2026-06-11)
|
||||
|
||||
The initial build used **dumble**, the cordiverse zero-config esbuild wrapper that upstream Cordis itself builds with — maximum alignment with the vendored packages' conventions (it reads each package.json and infers entries/formats from the `exports` field). But dumble is a liability as a load-bearing tool in this repo: v0.2.x, ~530 npm downloads/week, effectively one maintainer, and we were invoking it through a custom orchestration script (`scripts/build.ts`) because it has no workspace mode.
|
||||
|
||||
Build output currently matters only for `yarn build` + publint (nothing publishes yet; dev/test/demo run unbuilt via tsx), so the switching cost is at its lowest now and only grows once packages publish.
|
||||
Build output currently matters only for `pnpm run build` + publint (nothing publishes yet; dev/test/demo run unbuilt via tsx), so the switching cost is at its lowest now and only grows once packages publish.
|
||||
|
||||
## Decision
|
||||
|
||||
Replace dumble with **tsdown** (rolldown-based, ~2.5M downloads/week, VoidZero-backed, actively released):
|
||||
|
||||
- Root `tsdown.config.ts` with `workspace: ['vendor/*', 'packages/*']` (explicit globs, not `workspace: true`, which would also pick up `examples/*` — they have package.json files but are not yarn workspaces).
|
||||
- Root `tsdown.config.ts` with `workspace: ['vendor/*', 'packages/*']` (explicit globs, not `workspace: true`, which would also pick up `examples/*` — they have package.json files but are not pnpm workspaces).
|
||||
- Shared shape: entry `src/index.ts`, `outDir: 'lib'`, ESM, `platform: node`, `target: es2024`, `fixedExtension: false` (keeps `.js` for `"type": "module"` packages), `dts: false` (tsc -b owns declarations), `clean: false` (lib/ holds tsc's .d.ts output).
|
||||
- Two per-package overrides in vendor/ (ours, like the regenerated tsconfigs; logged in vendor/README.md): schemastery (dual `.mjs`/`.cjs` via `outExtensions`), logger-console (two single-entry passes so the shared base class is inlined into each entry instead of a hash-named chunk, matching upstream's published shape).
|
||||
- `scripts/build.ts` deleted; `yarn build` = `tsc -b && tsdown`.
|
||||
- `scripts/build.ts` deleted; `pnpm run build` = `tsc -b && tsdown`.
|
||||
|
||||
Alternatives considered: **direct esbuild script** (most established engine, zero wrapper risk, but hand-maintains the per-package spec table tsdown's workspace mode gives us); **pkgroll** (closest drop-in philosophically, but 78k dl/wk and Rollup-based — strictly weaker maintenance story than tsdown); **keep dumble** (perfect upstream alignment, unacceptable bus factor).
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ Two gates, mirroring the existing `scripts/` style (tsx ESM, one job each):
|
||||
1. **`doc-typecheck`** extracts every fenced ` ```ts ` block from `README.md`, `docs/**`, and `packages/*/README.md`, writes them to a temp project, and compiles with `tsc --noEmit`. The temp tsconfig copies only resolution-relevant options and the workspace `paths` map from `tsconfig.typecheck.json` (vendor → built `lib`, harness → `src`) — resolving vendor to `lib` is essential, or tsc type-checks raw vendor source and floods the run. A block that is a deliberate sketch opts out with an explicit ` ```ts ignore-check ` info string; the script reports the opt-out ratio and fails if it exceeds half, so the escape hatch can't quietly become the norm.
|
||||
2. **`verify-event-taxonomy`** extracts the event names from the `interface Events` blocks across `packages/*/src` and from the taxonomy table in `docs/architecture.md`, and asserts the two sets match exactly. Verify, don't generate: the table keeps its hand-written Mode/Purpose columns; only the set of names is checked. (Landing this surfaced three events the table had been missing — `tools/change`, `llm/adapter-change`, `system-prompt/change`.)
|
||||
|
||||
Both run via a shared `doc-sync` package.json script that the lefthook pre-push hook and CI both invoke (ADR 0007: hooks and CI call the same scripts, so the gate fires locally before a push — not only after it). They run after `yarn typecheck` (which emits the vendor `lib/` that doc-typecheck resolves against). API-extractor golden reports (RFC 006 part 3) were deliberately **deferred** — low value for an internal monorepo where reviewers already see the source diff, and a heavy, finicky dependency.
|
||||
Both run via a shared `doc-sync` package.json script that the lefthook pre-push hook and CI both invoke (ADR 0007: hooks and CI call the same scripts, so the gate fires locally before a push — not only after it). They run after `pnpm run typecheck` (which emits the vendor `lib/` that doc-typecheck resolves against). API-extractor golden reports (RFC 006 part 3) were deliberately **deferred** — low value for an internal monorepo where reviewers already see the source diff, and a heavy, finicky dependency.
|
||||
|
||||
## Consequences
|
||||
|
||||
|
||||
37
docs/adr/0016-pnpm-over-yarn.md
Normal file
37
docs/adr/0016-pnpm-over-yarn.md
Normal file
@@ -0,0 +1,37 @@
|
||||
# ADR 0016: pnpm as the package manager instead of Yarn 4
|
||||
|
||||
Status: accepted (2026-06-16)
|
||||
|
||||
## Context
|
||||
|
||||
The repo shipped on **Yarn 4** with the `node-modules` linker — a deliberately conservative choice that behaves like npm's flat layout while giving us Yarn's workspaces and `yarn constraints`. It worked. But Yarn 4's Plug'n'Play heritage makes the `node-modules` linker the off-the-beaten-path mode, and the broader JS ecosystem — tooling defaults, CI actions, Corepack examples, contributor familiarity — increasingly centers on pnpm. For a repo that is built primarily by agents and read by occasional human contributors, "the package manager most tools and people expect" has real value: fewer surprises, better-trodden failure paths, more copy-pasteable answers.
|
||||
|
||||
The switching cost is at its lowest right now. Nothing publishes from this repo yet (every package is `private: true`); dev/test/demo all run **unbuilt** via tsx, so the package manager only has to (a) resolve and link `node_modules`, (b) run the workspace scripts, and (c) enforce the workspace constraints. The one Yarn-specific asset is `yarn.config.cjs` (the `@yarnpkg/types` constraints engine), which is small and mechanical to re-express. This mirrors the reasoning in [ADR 0008](0008-tsdown-over-dumble.md): swap a load-bearing tool for the healthier-ecosystem option while the blast radius is still small.
|
||||
|
||||
## Decision
|
||||
|
||||
Adopt **pnpm 11.7.0**, pinned via the `packageManager` field and installed through Corepack (same mechanism Yarn used):
|
||||
|
||||
- **Workspaces** move from the `package.json` `workspaces` array + `.yarnrc.yml` to `pnpm-workspace.yaml` (`vendor/*`, `packages/*` — the same globs; `examples/*` stay non-workspace, matching the prior setup and tsdown's explicit globs).
|
||||
- **Strict symlinked linker** (pnpm's default) replaces Yarn's hoisted `node-modules` linker. We deliberately add **no** `node-linker=hoisted` / `shamefully-hoist` escape hatch: pnpm's non-flat `node_modules` makes phantom dependencies (importing an undeclared transitive dep) fail loudly, which is a *feature* for a repo whose whole quality story is mechanical gates ([ADR 0007](0007-quality-gates.md)). The gate suite — typecheck, lint, test, build, knip — is the safety net that proves no such phantom imports exist.
|
||||
- **Build-script allowlist.** pnpm 10+ does not run dependency lifecycle scripts unless allowlisted. `pnpm-workspace.yaml` carries an explicit `allowBuilds` map (`esbuild`, `lefthook`, `@google/genai`, `protobufjs`) — the same supply-chain-hardening posture the repo already takes toward model/tool output, now applied to install-time code execution. `peerDependencyRules.allowedVersions.typescript: '>=5 <7'` silences benign peer-range warnings for the in-repo TypeScript.
|
||||
- **Constraints become package-manager-independent.** `yarn.config.cjs` (which imported `@yarnpkg/types` and used `Yarn.workspaces()` / `workspace.set()`) is replaced by `scripts/check-workspace-constraints.ts`, a plain tsx script run as `pnpm run constraints`. It enforces the identical invariants — every package `private: true`; `@deepseek-ai/dsh-*` packages declare `cordis` as both a peer- and dev-dependency with matching ranges, `version: 0.0.1`, `type: module`; vendored packages checked for privacy only — over the same `vendor` + `packages` scope.
|
||||
- All `yarn …` verbs across CI, lefthook hooks, `package.json` scripts, and docs become `pnpm …` / `pnpm run …`. `yarn.lock` → `pnpm-lock.yaml` (lockfile v9). `.gitignore` swaps `.yarn/` for `.pnpm-store/`. Vendored READMEs (e.g. `vendor/cordis/README.md`) keep their upstream `yarn` examples untouched per the Vendoring Policy.
|
||||
|
||||
Alternatives considered: **keep Yarn 4** (zero churn, but bets on the less-traveled linker mode and a constraints engine tied to one package manager); **npm workspaces** (ubiquitous, but no constraints story and weaker monorepo ergonomics); **pnpm with hoisted linker** (smoother migration, but throws away the phantom-dependency safety that is the main correctness reason to move).
|
||||
|
||||
## Consequences
|
||||
|
||||
The constraints check loses Yarn's auto-**fix** (`workspace.set()` could rewrite a manifest in place); the tsx script is check-only and exits non-zero with a message instead. This is acceptable — CI never ran `--fix`, and a one-line manual edit is rare. Contributors now `corepack enable` for pnpm rather than Yarn; `pnpm exec lefthook install` replaces `yarn lefthook install` (the `postinstall` hook still runs `lefthook install`).
|
||||
|
||||
Performance (measured at migration time on the dev NFS filesystem; single-digit-run samples, high variance — directional, not a benchmark suite):
|
||||
|
||||
| Scenario | Yarn 4 | pnpm 11 |
|
||||
|---|---|---|
|
||||
| Cold (empty cache/store, no `node_modules`) | ~14 s | ~16 s |
|
||||
| Warm relink (cache/store warm, `node_modules` removed) | ~12–14 s | ~15–22 s |
|
||||
| Frozen, `node_modules` present (no-op revalidate) | ~2–8 s | ~0.5–7 s |
|
||||
|
||||
On a fast local disk pnpm's content-addressed store typically wins on cold/warm installs and, especially, on **disk footprint** across multiple checkouts (one global store hardlinked into every `node_modules` vs Yarn copying ~279 MB per worktree — some devs regularly keep ~10 or more worktrees for this repo). That dedup advantage did **not** show in the migration-time numbers above because the test store and `node_modules` sat on different filesystems, defeating hardlinks; on a single-filesystem dev box or CI cache it applies. The honest summary: install speed on our NFS dev filesystem is a wash within noise; the move is justified by ecosystem alignment, phantom-dependency safety, and cross-checkout disk dedup — not by a raw install-time win.
|
||||
|
||||
All quality gates (constraints, typecheck, lint, doc-sync, test:coverage at 100%, build, knip, publint, echo-agent demo smoke) pass unchanged on pnpm, which is the correctness proof that the linker swap introduced no phantom-dependency breakage.
|
||||
@@ -4,7 +4,7 @@ Status: accepted (2026-06-15)
|
||||
|
||||
## Context
|
||||
|
||||
The durable JSONL backend ([ADR 0016](0016-session-persistence.md)) uses the **turn** as its crash-recovery boundary: `load` returns events only up to the last complete `turn/end`, and the first post-load `append` truncates whatever follows as a never-committed crash tail. This is safe only if nothing *legitimately* durable can sit after the last `turn/end`.
|
||||
The durable JSONL backend ([ADR 0018](0018-session-persistence.md)) uses the **turn** as its crash-recovery boundary: `load` returns events only up to the last complete `turn/end`, and the first post-load `append` truncates whatever follows as a never-committed crash tail. This is safe only if nothing *legitimately* durable can sit after the last `turn/end`.
|
||||
|
||||
That assumption did not hold. Two paths recorded events outside any turn:
|
||||
|
||||
@@ -29,7 +29,7 @@ The serializability invariant is enforced at the same source boundary (`Session.
|
||||
|
||||
## Consequences
|
||||
|
||||
The turn is now the *single* durability/replay boundary, so [ADR 0016](0016-session-persistence.md)'s "last `turn/end` = commit point" rule is complete, not merely sufficient: a backend can discard everything after the last `turn/end` with zero risk of losing between-turn context, because there is no between-turn context. `scanLog` stays simple (no partial-turn boundary walk), and an idle background-task notice survives persist + resume.
|
||||
The turn is now the *single* durability/replay boundary, so [ADR 0018](0018-session-persistence.md)'s "last `turn/end` = commit point" rule is complete, not merely sufficient: a backend can discard everything after the last `turn/end` with zero risk of losing between-turn context, because there is no between-turn context. `scanLog` stays simple (no partial-turn boundary walk), and an idle background-task notice survives persist + resume.
|
||||
|
||||
Costs: `agent.inject()` while idle now writes three log lines instead of one, and the derived history gains a turn that carries only injected context (no assistant output) — `deriveMessages()` already derives purely by event type, so this renders identically. The `injection` trigger is a new on-disk vocabulary value; like every `SessionEventMap`/`TurnTriggerMap` addition it is part of the frozen format. Event ordering within a turn changed (`turn/start` now precedes `user/message`), which is observable to anything that asserted the old order — the loop's own tests were the only such consumers.
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
# ADR 0016: Session persistence as an abstract service over the existing `SessionEvent`
|
||||
# ADR 0018: Session persistence as an abstract service over the existing `SessionEvent`
|
||||
|
||||
Status: accepted (2026-06-15)
|
||||
|
||||
@@ -27,5 +27,6 @@ Do NOT write an ADR for: a mechanical or local choice (a variable name, a one-fi
|
||||
| [0013](0013-property-based-testing.md) | Property-based testing for protocol-shaped code | accepted |
|
||||
| [0014](0014-doc-sync-enforcement.md) | Doc-sync enforcement (doc code blocks + event taxonomy) | accepted |
|
||||
| [0015](0015-structured-error-taxonomy.md) | Structured error taxonomy (HarnessError base) | accepted |
|
||||
| [0016](0016-session-persistence.md) | Session persistence as an abstract service over the existing `SessionEvent` | accepted |
|
||||
| [0016](0016-pnpm-over-yarn.md) | pnpm as the package manager instead of Yarn 4 | accepted |
|
||||
| [0017](0017-turn-enclosure-invariant.md) | Every session event is enclosed in a turn | accepted |
|
||||
| [0018](0018-session-persistence.md) | Session persistence as an abstract service over `SessionEvent` | accepted |
|
||||
|
||||
@@ -15,7 +15,7 @@ packages/<name>/
|
||||
README.md # service API, events, extension points, design notes
|
||||
```
|
||||
|
||||
package.json invariants (enforced by `yarn constraints` / yarn.config.cjs): `private: true`, `version: 0.0.1`, `type: module`, `cordis` in BOTH peerDependencies and devDependencies (same range). Mirror every dsh peer dependency in devDependencies. `schemastery` goes in `dependencies` (it is a runtime validator), matching agent-loop.
|
||||
package.json invariants (enforced by `pnpm run constraints` / `scripts/check-workspace-constraints.ts`): `private: true`, `version: 0.0.1`, `type: module`, `cordis` in BOTH peerDependencies and devDependencies (same range). Mirror every dsh peer dependency in devDependencies. `schemastery` goes in `dependencies` (it is a runtime validator), matching agent-loop.
|
||||
|
||||
## 2. Register it in the root configs
|
||||
|
||||
@@ -36,10 +36,10 @@ For a swappable capability, split interface / implementation / consumer into sep
|
||||
## 4. Verify
|
||||
|
||||
```sh
|
||||
yarn install # registers the workspace
|
||||
yarn constraints && yarn typecheck && yarn lint
|
||||
yarn test:coverage # 100% per-file over src (types.ts exempt)
|
||||
yarn build && yarn knip && yarn publint
|
||||
pnpm install # registers the workspace
|
||||
pnpm run constraints && pnpm run typecheck && pnpm run lint
|
||||
pnpm run test:coverage # 100% per-file over src (types.ts exempt)
|
||||
pnpm run build && pnpm run knip && pnpm run publint
|
||||
```
|
||||
|
||||
Test expectations: every registry/registration needs an HMR-safety test (register from a child fiber, dispose it, assert cleanup). Excessive tests are welcome — see AGENTS.md.
|
||||
|
||||
@@ -48,9 +48,9 @@ Covered automatically by globs — no edits needed: root `package.json` workspac
|
||||
## 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
|
||||
pnpm install # registers the workspace
|
||||
pnpm run typecheck # the base→lib path split means: run once after a fresh add
|
||||
pnpm run build && pnpm run test && pnpm run 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.
|
||||
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 `pnpm run typecheck` (which builds them) once after adding the package or lint reports unresolved-type errors.
|
||||
|
||||
@@ -39,5 +39,5 @@ Split the adapter into testable stages (llm-deepseek's layout): wire types (`typ
|
||||
|
||||
- **Unit: mock the provider, not the harness.** A scripted `node:http` server speaking the provider's wire format covers happy paths, every error status, malformed payloads, premature closes, and aborts — no network, and it drives the 100% per-file coverage gate. Works for SDK-backed adapters too (point the SDK's baseURL at the mock).
|
||||
- **Hostile framing tests.** Split stream payloads at arbitrary byte positions (including mid-UTF-8) — real networks do.
|
||||
- **E2E: `tests/*.e2e.ts`** under `yarn test:e2e`, gated with `describe.skipIf(!process.env.MY_KEY)` so CI (no secrets) stays green. Cover each model × each provider mode you map (thinking on/off, effort levels), a tool-call round trip INCLUDING the follow-up turn with results in history, and loose assertions only (substring/structure, bounded maxTokens — real models are nondeterministic).
|
||||
- **E2E: `tests/*.e2e.ts`** under `pnpm run test:e2e`, gated with `describe.skipIf(!process.env.MY_KEY)` so CI (no secrets) stays green. Cover each model × each provider mode you map (thinking on/off, effort levels), a tool-call round trip INCLUDING the follow-up turn with results in history, and loose assertions only (substring/structure, bounded maxTokens — real models are nondeterministic).
|
||||
- Register the e2e file pattern in `knip.json` (per-workspace `entry` override) or knip flags it unused.
|
||||
|
||||
@@ -55,4 +55,4 @@ export function apply(ctx: Context) {
|
||||
|
||||
## 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`).
|
||||
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, `pnpm run demo:echo`) and [`examples/coding-agent`](../../examples/coding-agent) (DeepSeek V4 + the bash tool suite — the real thing, `pnpm run demo:coding`).
|
||||
|
||||
@@ -5,7 +5,7 @@ This guide covers the local setup needed to work on DeepSeek Harness and underst
|
||||
## Prerequisites
|
||||
|
||||
- Node.js 24 or newer. The repo declares `node >=24`; CI runs the matrix on Node 24 and 26.
|
||||
- Corepack-enabled Yarn. The repo pins `yarn@4.14.1` in `package.json`; run `corepack enable` if `yarn --version` does not resolve through Corepack.
|
||||
- Corepack-enabled pnpm. The repo pins `pnpm@11.7.0` in `package.json`; run `corepack enable` if `pnpm --version` does not resolve through Corepack.
|
||||
- Git.
|
||||
- Optional: a DeepSeek API key for the coding-agent demo and real-API e2e tests.
|
||||
|
||||
@@ -14,32 +14,32 @@ This guide covers the local setup needed to work on DeepSeek Harness and underst
|
||||
Install dependencies from the repo root:
|
||||
|
||||
```sh
|
||||
yarn install
|
||||
pnpm install
|
||||
```
|
||||
|
||||
Yarn uses the `node-modules` linker in this repo. The install also runs the root `postinstall` script, which installs lefthook from the repo dev dependency.
|
||||
The install also runs the root `postinstall` script, which installs lefthook from the repo dev dependency.
|
||||
|
||||
If hooks are missing because dependencies were restored from cache or `postinstall` was skipped, install them manually:
|
||||
|
||||
```sh
|
||||
yarn lefthook install
|
||||
pnpm exec lefthook install
|
||||
```
|
||||
|
||||
Run typecheck once after a fresh clone:
|
||||
|
||||
```sh
|
||||
yarn typecheck
|
||||
pnpm run typecheck
|
||||
```
|
||||
|
||||
That first typecheck builds declaration output used by type-aware linting for vendored packages. Without it, `yarn lint` can report unresolved-type `no-unsafe-*` errors even when source code is fine.
|
||||
That first typecheck builds declaration output used by type-aware linting for vendored packages. Without it, `pnpm run lint` can report unresolved-type `no-unsafe-*` errors even when source code is fine.
|
||||
|
||||
If you are preparing to push from a fresh clone or worktree, also build once:
|
||||
|
||||
```sh
|
||||
yarn build
|
||||
pnpm run build
|
||||
```
|
||||
|
||||
`yarn hygiene` includes `publint`, which validates package entrypoints against the built `lib/*.js` files. A fresh worktree has no bundled JS until `yarn build` runs.
|
||||
`pnpm run hygiene` includes `publint`, which validates package entrypoints against the built `lib/*.js` files. A fresh worktree has no bundled JS until `pnpm run build` runs.
|
||||
|
||||
## Environment variables
|
||||
|
||||
@@ -56,61 +56,61 @@ DEEPSEEK_BASE_URL=https://... # optional
|
||||
|
||||
lefthook is configured in `lefthook.yml` as an early local checkpoint before review:
|
||||
|
||||
- `pre-commit` runs staged-file ESLint fixes, `yarn typecheck`, and the vendor manifest guard.
|
||||
- `pre-push` runs `yarn test`, `yarn hygiene`, and `yarn doc-sync`.
|
||||
- `pre-commit` runs staged-file ESLint fixes, `pnpm run typecheck`, and the vendor manifest guard.
|
||||
- `pre-push` runs `pnpm run test`, `pnpm run hygiene`, and `pnpm run doc-sync`.
|
||||
|
||||
The vendor manifest guard checks that changes under `vendor/*/src` are staged with the matching `vendor/README.md` manifest update. See `vendor/README.md` before editing vendored code.
|
||||
|
||||
These hooks do not exactly mirror CI. Notably, `pre-push` runs unit tests without coverage, while CI runs `yarn test:coverage`; CI also runs an echo-agent smoke test and exercises the matrix on Node 24 and 26.
|
||||
These hooks do not exactly mirror CI. Notably, `pre-push` runs unit tests without coverage, while CI runs `pnpm run test:coverage`; CI also runs an echo-agent smoke test and exercises the matrix on Node 24 and 26.
|
||||
|
||||
## CI gates
|
||||
|
||||
The GitHub workflow runs these gates on each pull request:
|
||||
|
||||
- `yarn install --immutable`
|
||||
- `yarn constraints`
|
||||
- `yarn typecheck`
|
||||
- `yarn lint`
|
||||
- `yarn doc-sync`
|
||||
- `yarn test:coverage`
|
||||
- `yarn build`
|
||||
- `yarn knip && yarn publint`
|
||||
- `pnpm install --frozen-lockfile`
|
||||
- `pnpm run constraints`
|
||||
- `pnpm run typecheck`
|
||||
- `pnpm run lint`
|
||||
- `pnpm run doc-sync`
|
||||
- `pnpm run test:coverage`
|
||||
- `pnpm run build`
|
||||
- `pnpm run knip && pnpm run publint`
|
||||
- an echo-agent smoke test that checks the demo's tool call, tool result, and JSONL output
|
||||
|
||||
`yarn hygiene` is the local shorthand for `yarn knip && yarn publint && yarn constraints`; CI splits `yarn constraints` into its own earlier step, then runs `yarn knip && yarn publint` after `yarn build`.
|
||||
`pnpm run hygiene` is the local shorthand for `pnpm run knip && pnpm run publint && pnpm run constraints`; CI splits `pnpm run constraints` into its own earlier step, then runs `pnpm run knip && pnpm run publint` after `pnpm run build`.
|
||||
|
||||
## Daily commands
|
||||
|
||||
Use these from the repo root:
|
||||
|
||||
```sh
|
||||
yarn test # unit tests
|
||||
yarn test:coverage # unit tests with per-file coverage gates
|
||||
yarn test:e2e # real-API tests; self-skips without DEEPSEEK_API_KEY
|
||||
yarn typecheck # build declarations, then typecheck source, tests, and examples
|
||||
yarn lint # eslint .
|
||||
yarn lint:fix # eslint . --fix
|
||||
yarn doc-typecheck # compile checked TypeScript snippets in Markdown docs
|
||||
yarn verify-event-taxonomy # compare docs/architecture.md event names with source
|
||||
yarn doc-sync # doc-typecheck plus event taxonomy verification
|
||||
yarn build # build declarations and JS bundles
|
||||
yarn hygiene # knip, publint, and yarn constraints
|
||||
pnpm run test # unit tests
|
||||
pnpm run test:coverage # unit tests with per-file coverage gates
|
||||
pnpm run test:e2e # real-API tests; self-skips without DEEPSEEK_API_KEY
|
||||
pnpm run typecheck # build declarations, then typecheck source, tests, and examples
|
||||
pnpm run lint # eslint .
|
||||
pnpm run lint:fix # eslint . --fix
|
||||
pnpm run doc-typecheck # compile checked TypeScript snippets in Markdown docs
|
||||
pnpm run verify-event-taxonomy # compare docs/architecture.md event names with source
|
||||
pnpm run doc-sync # doc-typecheck plus event taxonomy verification
|
||||
pnpm run build # build declarations and JS bundles
|
||||
pnpm run hygiene # knip, publint, and workspace constraints
|
||||
```
|
||||
|
||||
When changing package public behavior, update the relevant README or JSDoc in the same change. `yarn doc-sync` catches checked TypeScript snippets and event-taxonomy drift, but broader prose/API sync still needs review.
|
||||
When changing package public behavior, update the relevant README or JSDoc in the same change. `pnpm run doc-sync` catches checked TypeScript snippets and event-taxonomy drift, but broader prose/API sync still needs review.
|
||||
|
||||
## Demos
|
||||
|
||||
The echo demo does not need API credentials:
|
||||
|
||||
```sh
|
||||
yarn demo:echo
|
||||
pnpm run demo:echo
|
||||
```
|
||||
|
||||
The coding-agent demo uses the real DeepSeek adapter and needs `DEEPSEEK_API_KEY` in the environment or repo-root `.env`:
|
||||
|
||||
```sh
|
||||
yarn demo:coding
|
||||
pnpm run demo:coding
|
||||
```
|
||||
|
||||
## Architecture context
|
||||
|
||||
@@ -9,7 +9,7 @@ The vendor manifest (ADR 0001) is enforced at commit time in the *forward* direc
|
||||
## 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.
|
||||
2. **Dependency advisories**: osv-scanner (or `pnpm 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).
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# RFC 009: Durable session persistence — an abstract, append-only, event-based store
|
||||
|
||||
Status: implemented (see [ADR 0016](../adr/0016-session-persistence.md))
|
||||
Status: implemented (see [ADR 0018](../adr/0018-session-persistence.md))
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
@@ -45,7 +45,7 @@ Lifecycle and disposal: the connection, listeners, and in-flight permission prom
|
||||
3. Internal edit — turn-end reason fidelity (sanctioned: edit internals to fit ACP). Extend `TurnEndReasonMap` in the proper places: (a) declaration-merge a `max-tokens` variant in the owning package (`packages/session/src/types.ts`, alongside `completed|aborted|error|disposed`) — add `max-tokens` because `FinishReasonMap` produces it (DeepSeek maps `length` → `max-tokens`); do not add `refusal`, since no current adapter produces it (unknown DeepSeek finish reasons collapse to `error`), but leave a comment in `TurnEndReasonMap` noting `refusal` should be added when an adapter first emits it (`FinishReasonMap` is merge-extensible); (b) make `agent-loop`'s `loop.ts` populate the reason from the model `finish` chunk — `assembler.finish` lives inside `runStep`, so `runStep` must return it up to `runTurn`, and the rule is "the last step's finish reason wins, but any `max-tokens` in the turn surfaces as `max-tokens`"; (c) no consumer exhaustively switches over `TurnEndReason` today (the invariants plugin switches on `SessionEventType`, and `deriveMessages` ignores `turn/end`), so adding `max-tokens` is a non-breaking extension — but recheck before landing; (d) update [docs/architecture.md](../architecture.md) (the CI-verified loop-lifecycle/event-taxonomy doc) and the affected package READMEs/JSDoc (`dsh-session`, `dsh-agent`, `dsh-agent-loop`) per the repo doc-sync policy. This replaces a fragile "observe the finish chunk in the bridge" hack with a real, documented contract.
|
||||
4. Prompt-turn streaming plus load: translate `agent/stream-chunk` and `session/event` into `session/update`; resolve `session/prompt` on settle, mapping the harness `TurnEndReason` to the ACP `StopReason` wire enum (`completed`→`end_turn`, `max-tokens`→`max_tokens`, `aborted`→`cancelled`) — a small total function with a test asserting the exact wire strings, since the SDK rejects an unknown `stopReason`. Concrete correlation, since the loop batches queued messages into one turn and `send()` does not synchronously flip to running: install listeners before `send()`; gate on an observed `agent/turn-start` (confirms work was accepted) then resolve on the next `agent/turn-end`; reject an empty/whitespace prompt up front rather than calling `send()` (no turn would ever start, so the RPC would hang). Implement `session/load` on RFC 009's resume seam.
|
||||
5. Permission gate: a single `tools/execute` listener registered with `prepend: true`, owning a `WeakMap<Agent, sessionId>` of bridge-created agents; no-op (`next()`) for unowned/no-agent calls; for owned calls → `session/request_permission` → allow (`next()`) / veto; settle the stored resolver exactly once on outcome, cancel, or connection close.
|
||||
6. Example wiring (extract a shared base). `@cordisjs/plugin-include` is itself a plugin entry that resets `ctx.baseUrl` and loads a path, so a child `cordis.yml` can nest-include a shared base; the extraction is safe because every dependent plugin declares `inject` (loader groups initialize via `Promise.all`, so YAML order is NOT the dependency mechanism — never rely on it). Extract the provider/tool core (`llm, sessions, system-prompt, tools, agents, invariants, llm-deepseek, bash-local, tool-bash`) into `examples/base.yml`; have both `coding-agent` and a new `examples/acp-agent/` include it and add their own UI plugin plus logger. Keep `agent-loop` per-example (NOT in the base): `AgentLoop` creates its configured agents in its constructor, and the two examples disagree — `coding-agent` needs a pre-created `main` (its `stdio-chat` calls `ctx.agents.get('main')`), while `acp-agent` must pre-create none (ACP `session/new` creates agents). So `coding-agent` declares `agent-loop` with `agents: [{ id: main, … }]` and `acp-agent` with `agents: []`. `acp-agent` loads `dsh-session-persistence-jsonl` (from RFC 009 — required for `session/load`), omits the stdout logger (see Risks), and adds `yarn demo:acp` plus the Zed `agent_servers` snippet.
|
||||
6. Example wiring (extract a shared base). `@cordisjs/plugin-include` is itself a plugin entry that resets `ctx.baseUrl` and loads a path, so a child `cordis.yml` can nest-include a shared base; the extraction is safe because every dependent plugin declares `inject` (loader groups initialize via `Promise.all`, so YAML order is NOT the dependency mechanism — never rely on it). Extract the provider/tool core (`llm, sessions, system-prompt, tools, agents, invariants, llm-deepseek, bash-local, tool-bash`) into `examples/base.yml`; have both `coding-agent` and a new `examples/acp-agent/` include it and add their own UI plugin plus logger. Keep `agent-loop` per-example (NOT in the base): `AgentLoop` creates its configured agents in its constructor, and the two examples disagree — `coding-agent` needs a pre-created `main` (its `stdio-chat` calls `ctx.agents.get('main')`), while `acp-agent` must pre-create none (ACP `session/new` creates agents). So `coding-agent` declares `agent-loop` with `agents: [{ id: main, … }]` and `acp-agent` with `agents: []`. `acp-agent` loads `dsh-session-persistence-jsonl` (from RFC 009 — required for `session/load`), omits the stdout logger (see Risks), and adds `pnpm run demo:acp` plus the Zed `agent_servers` snippet.
|
||||
7. Tests (the repo cares a lot here): a property-based test for the protocol shape (precedent: RFC 001 / [ADR 0013](../adr/0013-property-based-testing.md)) — fuzz arbitrary harness event sequences and assert ACP-stream invariants (never a `tool_call_update` before its `tool_call`; exactly one `session/prompt` resolution per prompt; monotonic, well-formed ordering; `stopReason` in the legal set); codec unit tests over an in-memory `Duplex` pair (drive `AgentSideConnection` without a subprocess; assert exact frames for `initialize`, `session/new`, a full prompt turn); the mandatory HMR-safety test (dispose the fiber; assert the connection closed, all `ctx.on` listeners gone, any in-flight `request_permission` settled); failure-path tests (connection closes mid-stream; closes with a permission pending; a notification `send()` rejects but the turn survives; `finish{kind:'error'|'aborted'}`; a `tools/execute` throw with no `tool/result`; a second `session/new` rejected; a `session/prompt` while one is in flight; an empty prompt rejected without hanging; a `session/load` re-derives identical history and replays it); and an e2e (`*.e2e.ts`, self-skips without `DEEPSEEK_API_KEY`) that boots `examples/acp-agent`, connects a `ClientSideConnection`, sends a real prompt, owns and disposes the harness in `afterEach`, and verifies the world (files on disk), not the agent's self-report.
|
||||
8. Docs: module/JSDoc plus a package README; extend [the extension cookbook](../cookbook/extension-cookbook.md) with the client-driver pattern. Flip Status to `implemented` on landing; write an ADR only if a decision proves durable, contested, and surprising (candidates: the `tools/execute` permission-ownership rule, the npm-dependency choice) — not auto-required.
|
||||
|
||||
|
||||
@@ -85,7 +85,7 @@ It is insufficient for the **composition / round-trip** half, which is the decis
|
||||
2. Scaffold the implementation package `packages/code-runtime-vm/`: the node:vm stub — `safe = false`, a constructor that **throws unless given `{ unsafe: true }`**, transpile/type-erase, async-IIFE wrap, capturing `console`, SDK globals, return-value/logs/error capture, output cap, signal-tied timeout. Tests for output capture, return value, error-as-field, abort, the constructor refusal without `unsafe`, and a README documenting the "not a sandbox, trusted-only" caveat prominently.
|
||||
3. Scaffold the consumer plugin `packages/code-mode/`: `jsonSchemaToTs` codegen with namespace/quoted-access + alias handling (unit tests, including non-identifier MCP names and unsupported-shape → `unknown`); the registered lazy `ctx.systemPrompt.section()` carrying the SDK `.d.ts`; the `agent/request` listener (`prepend: true`) collapsing `request.tools` to `[run_code]` after `await next()`; the **unsafe-runtime gate** (refuse to register `run_code` when `ctx.codeRuntime.safe === false` unless `allowUnsafeRuntime` is set); the `run_code` tool with the dispatch bridge (per-run serialization queue, deterministic sub-call ids, before/after abort checks, `CodeRunError` on error results); and the `code/dispatch` event declared here via `SessionEventMap` merge. Declare `inject = ['tools', 'systemPrompt', 'codeRuntime']`.
|
||||
4. Tests: HMR-safety (dispose removes the tool, the section, and the listener); a waterfall test that the wire tool list is exactly `[run_code]` (spy adapter, asserting via `agent/request` and optionally `llm/stream`); an integration test that a program calling two tools returns only its printed/returned output (verify the world, not the self-report); a **serialization test** that `Promise.all([...])` over SDK calls does not overlap the underlying `ctx.tools.execute` invocations (a probe tool records enter/exit; assert no interleaving); `deriveMessages()` ignores `code/dispatch`; abort mid-program stops further dispatches; `CodeRunError` surfaces as `isError: true`; and the **unsafe-runtime refusal test** (§3, the VM-guard): with the unsafe flag unset, a non-mock agent's `run_code` is refused; with it set, the program runs.
|
||||
5. Wire an example: `examples/coding-agent-code-mode` (or a config flag on the existing example) loading the trio. Running it against the node:vm stub requires both opt-ins (`VmCodeRuntime({ unsafe: true })` and `code-mode`'s `allowUnsafeRuntime`); the example sets them explicitly and comments why, or uses a mock model — a real model never reaches the unsandboxed stub without those deliberate flags. Add a `yarn demo:*` entry.
|
||||
5. Wire an example: `examples/coding-agent-code-mode` (or a config flag on the existing example) loading the trio. Running it against the node:vm stub requires both opt-ins (`VmCodeRuntime({ unsafe: true })` and `code-mode`'s `allowUnsafeRuntime`); the example sets them explicitly and comments why, or uses a mock model — a real model never reaches the unsandboxed stub without those deliberate flags. Add a `pnpm run demo:*` entry.
|
||||
6. Docs: update [docs/architecture.md](../architecture.md) (a `ctx.codeRuntime` row in the service map, a Code Mode note under the tool pipeline / capability seams sections); add a [cookbook](../cookbook/) note on writing a `CodeRuntime` backend; and **file the follow-up RFC for the hardened execution substrate** (the isolate/sandboxed-process design, the additional-language backends sketched in §1 — AssemblyScript/WASM, Python — with their per-language SDK generators, plus the tool-visibility-tier design skipped here). Append the `| 012 | … | proposed |` row to [the RFC index](README.md).
|
||||
|
||||
## Risks
|
||||
|
||||
Reference in New Issue
Block a user