feat: enforce merge-commit policy and markdown wrap convention
- AGENTS.md: require merging PRs with a merge commit (gh pr merge --merge), never squash/rebase — the per-PR commit history (review-fix and regression-test commits) is intentional record. - Add scripts/verify-md-wrap.ts: a doc-sync gate that fails on hard-wrapped prose paragraphs (one physical line per paragraph), with smart exemptions for fenced code, tables, lists, blockquotes, headings, HTML comments, hrs, and reference defs. Scope covers README.md, docs/**/*.md, packages/*/README.md, plus AGENTS.md / packages/AGENTS.md (the files doc-sync did not previously cover). Folded into doc-sync so it rides the existing pre-push and CI gates. - Sync AGENTS.md and docs/development.md doc-sync descriptions and command lists to include verify-md-wrap.
This commit is contained in:
@@ -69,7 +69,9 @@ pnpm run doc-typecheck # typecheck every ```ts block in README.md, docs/**/*.md
|
|||||||
# packages/*/README.md (doc/code drift gate)
|
# packages/*/README.md (doc/code drift gate)
|
||||||
pnpm run verify-event-taxonomy # assert the event-taxonomy table in docs/architecture.md
|
pnpm run verify-event-taxonomy # assert the event-taxonomy table in docs/architecture.md
|
||||||
# matches the interface Events declarations in source
|
# matches the interface Events declarations in source
|
||||||
pnpm run doc-sync # doc-typecheck + verify-event-taxonomy (CI runs this)
|
pnpm run verify-md-wrap # assert no hard-wrapped prose paragraphs in README.md,
|
||||||
|
# docs/**/*.md, packages/*/README.md, AGENTS.md (one line per paragraph)
|
||||||
|
pnpm run doc-sync # doc-typecheck + verify-event-taxonomy + verify-md-wrap (CI runs this)
|
||||||
pnpm run demo:echo # run examples/echo-agent (no API key; type "echo hi" to
|
pnpm run demo:echo # run examples/echo-agent (no API key; type "echo hi" to
|
||||||
# see a tool call) — the mock skeleton
|
# see a tool call) — the mock skeleton
|
||||||
pnpm run demo:coding # run examples/coding-agent — the real agent (needs
|
pnpm run demo:coding # run examples/coding-agent — the real agent (needs
|
||||||
@@ -104,6 +106,7 @@ Dev/test/demo run **unbuilt** via tsx + the `paths` map in the root `tsconfig.js
|
|||||||
- **Explicit > implicit at package seams**: interface/vocabulary types spell out every field a consumer must supply — no optional field that the implementation silently fills with a hidden `?? default`. Put defaulting in the owning implementation as an explicit step (a `resolve(request): Spec` method that turns the optional-field request into the required-field spec), not smuggled inside `run()`/`start()`. Example: `dsh-bash` splits `BashExecRequest` (optional `workdir`/`timeoutMs`, model-facing) from `BashExecSpec` (required, what `run`/`start` act on); the tool layer calls `ctx.bash.resolve()` between them. The reader of a `BashExecSpec` never has to wonder where the working directory came from.
|
- **Explicit > implicit at package seams**: interface/vocabulary types spell out every field a consumer must supply — no optional field that the implementation silently fills with a hidden `?? default`. Put defaulting in the owning implementation as an explicit step (a `resolve(request): Spec` method that turns the optional-field request into the required-field spec), not smuggled inside `run()`/`start()`. Example: `dsh-bash` splits `BashExecRequest` (optional `workdir`/`timeoutMs`, model-facing) from `BashExecSpec` (required, what `run`/`start` act on); the tool layer calls `ctx.bash.resolve()` between them. The reader of a `BashExecSpec` never has to wonder where the working directory came from.
|
||||||
- **An empty `catch` must name what it swallows and why nothing else can hit it**: a bare `catch {}` hides bugs. When you deliberately ignore a throw, the comment must (a) name the single expected failure, (b) say why ignoring it is correct — usually because the useful state was already captured *before* the `try` — and (c) make clear nothing else of consequence can reach the catch (ideally the `try` wraps a single statement). Example: the error-body `response.json()` parse in `dsh-llm-deepseek`'s adapter sets `code` + HTTP `status` from the status line before the `try`, so a malformed provider body can only cost a richer message, never the real error.
|
- **An empty `catch` must name what it swallows and why nothing else can hit it**: a bare `catch {}` hides bugs. When you deliberately ignore a throw, the comment must (a) name the single expected failure, (b) say why ignoring it is correct — usually because the useful state was already captured *before* the `try` — and (c) make clear nothing else of consequence can reach the catch (ideally the `try` wraps a single statement). Example: the error-body `response.json()` parse in `dsh-llm-deepseek`'s adapter sets `code` + HTTP `status` from the status line before the `try`, so a malformed provider body can only cost a richer message, never the real error.
|
||||||
- **Symmetry is usually more correct**: when two related values play parallel roles (a test fixture and its expected output, a request shape and its response shape, a buggy input and the test that checks the fix), give them parallel form — both named consts, or both inline, not one each way. Asymmetry is a smell that usually points at a missed extraction.
|
- **Symmetry is usually more correct**: when two related values play parallel roles (a test fixture and its expected output, a request shape and its response shape, a buggy input and the test that checks the fix), give them parallel form — both named consts, or both inline, not one each way. Asymmetry is a smell that usually points at a missed extraction.
|
||||||
|
- **Merging PRs**: always merge with a **merge commit** (`gh pr merge --merge`), never squash or rebase. The per-PR commit history is intentional — review-fix commits, regression-test commits, and the reasoning in each message are part of the record — and squashing flattens it away.
|
||||||
- **Tests**: vitest, colocated under `packages/<name>/tests/*.spec.ts`. Every registry needs an HMR-safety test (dispose the contributing fiber, assert cleanup). **Excessive tests are welcome** — when in doubt, write the test; err on the side of covering edge cases, error paths, event ordering, and concurrency races even if they seem unlikely. Review findings get regression tests (see `packages/agent-loop/tests/review-fixes.spec.ts`).
|
- **Tests**: vitest, colocated under `packages/<name>/tests/*.spec.ts`. Every registry needs an HMR-safety test (dispose the contributing fiber, assert cleanup). **Excessive tests are welcome** — when in doubt, write the test; err on the side of covering edge cases, error paths, event ordering, and concurrency races even if they seem unlikely. Review findings get regression tests (see `packages/agent-loop/tests/review-fixes.spec.ts`).
|
||||||
|
|
||||||
## Defensive patterns (hard-won)
|
## Defensive patterns (hard-won)
|
||||||
@@ -125,9 +128,9 @@ This codebase aims to be **very type-safe and well documented** for maintainabil
|
|||||||
|
|
||||||
In the **core** packages (`packages/llm`, `packages/tools`, `packages/agent`, `packages/agent-loop`, `packages/session`, `packages/system-prompt`), **type gymnastics are acceptable when they improve the DX of plugin authors** for common plugin types. The `defineTool` typed schema DSL in `dsh-tools` is the canonical example: the `SchemaSpec` to `InferArgs<S>` type-level mapping gives tool authors zero-cast typed `execute` args, and the cost of the conditional types stays inside the core package.
|
In the **core** packages (`packages/llm`, `packages/tools`, `packages/agent`, `packages/agent-loop`, `packages/session`, `packages/system-prompt`), **type gymnastics are acceptable when they improve the DX of plugin authors** for common plugin types. The `defineTool` typed schema DSL in `dsh-tools` is the canonical example: the `SchemaSpec` to `InferArgs<S>` type-level mapping gives tool authors zero-cast typed `execute` args, and the cost of the conditional types stays inside the core package.
|
||||||
|
|
||||||
Verbose documentation is fine **as long as docs and code stay strictly in sync**. Out-of-sync docs are worse than no docs. **When you change code, update its docs in the SAME change** — grep the package README and the module/JSDoc comments for the old behavior (config keys, defaults, error codes, wire field names, event names) and fix every hit. CI runs `pnpm run doc-sync` (`doc-typecheck` + `verify-event-taxonomy`), which typechecks every fenced `ts` block in `README.md`, `docs/**/*.md`, and `packages/*/README.md` and verifies the event-taxonomy table against source — but that scope does NOT cover `AGENTS.md`, `packages/AGENTS.md`, or `packages/README.md`, nor does it catch prose drift (config keys, defaults, error codes), so keeping those in sync remains on the author. Every module has a module-level doc comment explaining its role. Every exported class, interface, type, function, and non-obvious method has a JSDoc that explains semantics (not just the name) — contracts (what events fire when), disposal behavior, error behavior, and extension intent. Internal helpers get docs only where non-obvious. Prefer one-liners when one line suffices.
|
Verbose documentation is fine **as long as docs and code stay strictly in sync**. Out-of-sync docs are worse than no docs. **When you change code, update its docs in the SAME change** — grep the package README and the module/JSDoc comments for the old behavior (config keys, defaults, error codes, wire field names, event names) and fix every hit. CI runs `pnpm run doc-sync` (`doc-typecheck` + `verify-event-taxonomy` + `verify-md-wrap`), which typechecks every fenced `ts` block in `README.md`, `docs/**/*.md`, and `packages/*/README.md`, verifies the event-taxonomy table against source, and asserts no hard-wrapped prose paragraphs across those files plus `AGENTS.md` / `packages/AGENTS.md` — but that scope does NOT catch prose drift in `AGENTS.md` / `packages/AGENTS.md` / `packages/README.md` (config keys, defaults, error codes), so keeping those in sync remains on the author. Every module has a module-level doc comment explaining its role. Every exported class, interface, type, function, and non-obvious method has a JSDoc that explains semantics (not just the name) — contracts (what events fire when), disposal behavior, error behavior, and extension intent. Internal helpers get docs only where non-obvious. Prefer one-liners when one line suffices.
|
||||||
|
|
||||||
**Markdown is not hard-wrapped**: write one line per paragraph and let the editor soft-wrap. Hard line breaks mid-paragraph make docs harder to edit and diff — a one-word change reflows and re-diffs the whole paragraph. This applies to prose only: leave fenced code blocks, tables, and list structure intact (a wrapped list item folds to one line per bullet). Code comments / JSDoc are exempt — they stay under the linter's column limit.
|
**Markdown is not hard-wrapped**: write one line per paragraph and let the editor soft-wrap. Hard line breaks mid-paragraph make docs harder to edit and diff — a one-word change reflows and re-diffs the whole paragraph. This applies to prose only: leave fenced code blocks, tables, and list structure intact (a wrapped list item folds to one line per bullet). Code comments / JSDoc are exempt — they stay under the linter's column limit. `pnpm run verify-md-wrap` (part of `doc-sync`) enforces this across `README.md`, `docs/**/*.md`, `packages/*/README.md`, and `AGENTS.md` / `packages/AGENTS.md`.
|
||||||
|
|
||||||
**Editing these instructions**: `AGENTS.md` is the real file; `CLAUDE.md` is a symlink to it (at the repo root and in `packages/`). Always edit `AGENTS.md` — never write through the `CLAUDE.md` symlink or replace it with a regular file.
|
**Editing these instructions**: `AGENTS.md` is the real file; `CLAUDE.md` is a symlink to it (at the repo root and in `packages/`). Always edit `AGENTS.md` — never write through the `CLAUDE.md` symlink or replace it with a regular file.
|
||||||
|
|
||||||
|
|||||||
@@ -93,14 +93,15 @@ pnpm run lint # eslint .
|
|||||||
pnpm run lint:fix # eslint . --fix
|
pnpm run lint:fix # eslint . --fix
|
||||||
pnpm run doc-typecheck # compile checked TypeScript snippets in Markdown docs
|
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 verify-event-taxonomy # compare docs/architecture.md event names with source
|
||||||
pnpm run doc-sync # doc-typecheck plus event taxonomy verification
|
pnpm run verify-md-wrap # fail on hard-wrapped prose paragraphs in docs/README markdown
|
||||||
|
pnpm run doc-sync # doc-typecheck, event taxonomy, and markdown wrap verification
|
||||||
pnpm run gen-module-graph # regenerate docs/module-graph.md from package peerDeps
|
pnpm run gen-module-graph # regenerate docs/module-graph.md from package peerDeps
|
||||||
pnpm run verify-module-graph # fail if docs/module-graph.md is stale
|
pnpm run verify-module-graph # fail if docs/module-graph.md is stale
|
||||||
pnpm run build # build declarations and JS bundles
|
pnpm run build # build declarations and JS bundles
|
||||||
pnpm run hygiene # knip, publint, and workspace constraints
|
pnpm run hygiene # knip, publint, and workspace constraints
|
||||||
```
|
```
|
||||||
|
|
||||||
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.
|
When changing package public behavior, update the relevant README or JSDoc in the same change. `pnpm run doc-sync` catches checked TypeScript snippets, event-taxonomy drift, and hard-wrapped markdown prose, but broader prose/API sync still needs review.
|
||||||
|
|
||||||
## Demos
|
## Demos
|
||||||
|
|
||||||
|
|||||||
@@ -23,10 +23,11 @@
|
|||||||
"publint": "tsx scripts/publint-all.ts",
|
"publint": "tsx scripts/publint-all.ts",
|
||||||
"doc-typecheck": "tsx scripts/doc-typecheck.ts",
|
"doc-typecheck": "tsx scripts/doc-typecheck.ts",
|
||||||
"verify-event-taxonomy": "tsx scripts/verify-event-taxonomy.ts",
|
"verify-event-taxonomy": "tsx scripts/verify-event-taxonomy.ts",
|
||||||
|
"verify-md-wrap": "tsx scripts/verify-md-wrap.ts",
|
||||||
"gen-module-graph": "tsx scripts/gen-module-graph.ts",
|
"gen-module-graph": "tsx scripts/gen-module-graph.ts",
|
||||||
"verify-module-graph": "tsx scripts/gen-module-graph.ts --check",
|
"verify-module-graph": "tsx scripts/gen-module-graph.ts --check",
|
||||||
"constraints": "tsx scripts/check-workspace-constraints.ts",
|
"constraints": "tsx scripts/check-workspace-constraints.ts",
|
||||||
"doc-sync": "pnpm run doc-typecheck && pnpm run verify-event-taxonomy",
|
"doc-sync": "pnpm run doc-typecheck && pnpm run verify-event-taxonomy && pnpm run verify-md-wrap",
|
||||||
"hygiene": "pnpm run knip && pnpm run publint && pnpm run constraints",
|
"hygiene": "pnpm run knip && pnpm run publint && pnpm run constraints",
|
||||||
"demo:echo": "node --expose-internals --import tsx examples/echo-agent/start.ts",
|
"demo:echo": "node --expose-internals --import tsx examples/echo-agent/start.ts",
|
||||||
"demo:coding": "node --expose-internals --import tsx examples/coding-agent/start.ts",
|
"demo:coding": "node --expose-internals --import tsx examples/coding-agent/start.ts",
|
||||||
|
|||||||
172
scripts/verify-md-wrap.ts
Normal file
172
scripts/verify-md-wrap.ts
Normal file
@@ -0,0 +1,172 @@
|
|||||||
|
/**
|
||||||
|
* Doc-sync gate: enforce the repo's "Markdown is not hard-wrapped" convention
|
||||||
|
* (AGENTS.md § Type Safety and Documentation) — prose paragraphs are written as
|
||||||
|
* one physical line per paragraph and the editor soft-wraps. A hard-wrapped
|
||||||
|
* paragraph (a one-word edit reflows and re-diffs the whole block) is a defect
|
||||||
|
* this script catches before review.
|
||||||
|
*
|
||||||
|
* Scope mirrors doc-typecheck plus the two AGENTS.md files that doc-sync does
|
||||||
|
* NOT otherwise cover (the convention itself lives there): README.md,
|
||||||
|
* docs/** /*.md, packages/* /README.md, AGENTS.md, packages/AGENTS.md. (The
|
||||||
|
* root and packages/ CLAUDE.md are symlinks to the AGENTS.md files, so they are
|
||||||
|
* skipped to avoid double-reporting.)
|
||||||
|
*
|
||||||
|
* A violation is two consecutive *prose* lines — a paragraph that spans
|
||||||
|
* physical lines instead of soft-wrapping. Structure that legitimately occupies
|
||||||
|
* multiple lines is exempt: fenced code blocks, tables, list items (and their
|
||||||
|
* indented continuations), headings, blockquotes, HTML blocks/comments,
|
||||||
|
* horizontal rules, and reference-link / footnote definitions.
|
||||||
|
*
|
||||||
|
* Run: `tsx scripts/verify-md-wrap.ts`.
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { readFileSync, realpathSync } from 'node:fs'
|
||||||
|
import { relative, resolve } from 'node:path'
|
||||||
|
import { glob } from 'node:fs/promises'
|
||||||
|
|
||||||
|
const root = resolve(import.meta.dirname, '..')
|
||||||
|
|
||||||
|
/** Files to check: doc-typecheck's scope plus the AGENTS.md pair. */
|
||||||
|
const PATTERNS = ['README.md', 'docs/**/*.md', 'packages/*/README.md', 'AGENTS.md', 'packages/AGENTS.md']
|
||||||
|
|
||||||
|
/** A located hard-wrap: the second line of a multi-line prose paragraph. */
|
||||||
|
interface Violation {
|
||||||
|
file: string
|
||||||
|
/** 1-based line number of the offending continuation line. */
|
||||||
|
line: number
|
||||||
|
text: string
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* True when a line is *prose* — ordinary paragraph text, not markdown
|
||||||
|
* structure. Structural lines (headings, lists, tables, blockquotes, HTML,
|
||||||
|
* fences, hrs, reference defs) legitimately stand alone or stack, so they never
|
||||||
|
* count toward a hard-wrap pair. Caller handles fenced-code and list-body state.
|
||||||
|
*/
|
||||||
|
function isProse(line: string): boolean {
|
||||||
|
if (line.trim() === '') return false
|
||||||
|
// Up to 3 leading spaces is still a "top-level" block in CommonMark; deeper
|
||||||
|
// indentation is handled as list continuation by the caller.
|
||||||
|
const s = line.replace(/^ {0,3}/, '')
|
||||||
|
if (/^#{1,6}\s/.test(s)) return false // ATX heading
|
||||||
|
if (/^([-*+])\s/.test(s)) return false // bullet list
|
||||||
|
if (/^\d{1,9}[.)]\s/.test(s)) return false // ordered list
|
||||||
|
if (/^>/.test(s)) return false // blockquote
|
||||||
|
if (/^\|/.test(s)) return false // table row
|
||||||
|
if (/^<!--/.test(s) || /-->\s*$/.test(s)) return false // HTML comment line
|
||||||
|
if (/^</.test(s)) return false // HTML block line
|
||||||
|
if (/^([-*_])( *\1){2,}\s*$/.test(s)) return false // thematic break (hr)
|
||||||
|
if (/^\[[^\]]+\]:\s/.test(s)) return false // reference-link / footnote definition
|
||||||
|
if (/^[=-]+\s*$/.test(s)) return false // setext heading underline
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Find every hard-wrapped prose paragraph in one Markdown file. */
|
||||||
|
function findViolations(absPath: string): Violation[] {
|
||||||
|
const file = relative(root, absPath)
|
||||||
|
const lines = readFileSync(absPath, 'utf8').split('\n')
|
||||||
|
const out: Violation[] = []
|
||||||
|
|
||||||
|
let inFence = false
|
||||||
|
let fenceMarker = '' // '```' or '~~~'
|
||||||
|
let inComment = false // inside a multi-line <!-- … --> HTML comment
|
||||||
|
let inListItem = false // inside a list item's body (its indented continuations)
|
||||||
|
let prevWasProse = false
|
||||||
|
|
||||||
|
lines.forEach((raw, i) => {
|
||||||
|
const trimmed = raw.trim()
|
||||||
|
|
||||||
|
// Fenced code blocks: everything between matching fences is exempt.
|
||||||
|
const fence = /^ {0,3}(```+|~~~+)/.exec(raw)
|
||||||
|
if (fence) {
|
||||||
|
const marker = (fence[1] ?? '').startsWith('`') ? '```' : '~~~'
|
||||||
|
if (!inFence) {
|
||||||
|
inFence = true
|
||||||
|
fenceMarker = marker
|
||||||
|
} else if (marker === fenceMarker) {
|
||||||
|
inFence = false
|
||||||
|
}
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (inFence) {
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Multi-line HTML comments are exempt (e.g. generated-file headers). Track
|
||||||
|
// open/close across lines so the body of a 3+ line comment isn't read as
|
||||||
|
// hard-wrapped prose.
|
||||||
|
if (inComment) {
|
||||||
|
if (/-->/.test(raw)) inComment = false
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (/^ {0,3}<!--/.test(raw) && !/-->/.test(raw)) {
|
||||||
|
inComment = true
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if (trimmed === '') {
|
||||||
|
inListItem = false
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Track list context so an item's wrapped continuation lines (indented or
|
||||||
|
// lazy) are treated as list structure, not a hard-wrapped prose paragraph.
|
||||||
|
const isListMarker = /^ {0,3}([-*+]|\d{1,9}[.)])\s/.test(raw)
|
||||||
|
if (isListMarker) {
|
||||||
|
inListItem = true
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if (inListItem) {
|
||||||
|
// Indented under the item, or lazy continuation — still the list item.
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!isProse(raw)) {
|
||||||
|
prevWasProse = false
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// A prose line. If the line before it was also prose, the paragraph spans
|
||||||
|
// physical lines — a hard wrap.
|
||||||
|
if (prevWasProse) {
|
||||||
|
out.push({ file, line: i + 1, text: trimmed })
|
||||||
|
}
|
||||||
|
prevWasProse = true
|
||||||
|
})
|
||||||
|
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
const seen = new Set<string>()
|
||||||
|
const all: Violation[] = []
|
||||||
|
let checked = 0
|
||||||
|
for (const pattern of PATTERNS) {
|
||||||
|
for await (const match of glob(pattern, { cwd: root })) {
|
||||||
|
const abs = resolve(root, match)
|
||||||
|
// CLAUDE.md symlinks resolve onto AGENTS.md; dedupe by real path so a file
|
||||||
|
// matched twice (or via symlink) is checked once.
|
||||||
|
const real = realpathSync(abs)
|
||||||
|
if (seen.has(real)) continue
|
||||||
|
seen.add(real)
|
||||||
|
checked++
|
||||||
|
all.push(...findViolations(abs))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (all.length === 0) {
|
||||||
|
console.log(`verify-md-wrap: ${checked} file(s) checked, no hard-wrapped prose paragraphs.`)
|
||||||
|
process.exit(0)
|
||||||
|
}
|
||||||
|
|
||||||
|
console.error('verify-md-wrap: hard-wrapped prose paragraphs found (write one physical line per paragraph):')
|
||||||
|
for (const v of all) {
|
||||||
|
console.error(` ${v.file}:${v.line} ${v.text.slice(0, 80)}${v.text.length > 80 ? '…' : ''}`)
|
||||||
|
}
|
||||||
|
process.exit(1)
|
||||||
Reference in New Issue
Block a user