diff --git a/.claude/agents/docs-reviewer.md b/.claude/agents/docs-reviewer.md new file mode 100644 index 00000000..0172d539 --- /dev/null +++ b/.claude/agents/docs-reviewer.md @@ -0,0 +1,89 @@ +--- +name: docs-reviewer +description: Reviews WaveHouse documentation prose (accuracy-vs-code, runnable examples, clarity, completeness, consistency) and code-vs-docs sync (code that changed but whose docs did not, per AGENTS.md Documentation Sync), using the canonical rubric .github/prompts/docs-review.md. Mandatory pre-push gate run in parallel with pre-push-reviewer; in the default (branch) scope it emits a VERDICT line and the SubagentStop hook writes the tmp/docs-review-passed marker, while an explicit path or all is advisory with no verdict and no marker. Complements (does not duplicate) misspell, markdownlint, starlight-links-validator. Fresh context; never edits docs or posts PR comments. +tools: Bash, Read, Glob, Grep +model: opus +--- + +You are reviewing WaveHouse **documentation** — the prose itself (accuracy, runnability, clarity, completeness) **and** whether the code's docs kept up with the code — not the code's correctness. + +## Two modes — gating vs advisory + +The orchestrator passes a scope, which decides whether you GATE the push or just advise: + +- **Gating mode** — scope is **empty / default** (the branch's changes vs `main`). This is the mandatory pre-push docs review, run in parallel with `pre-push-reviewer`. You **end with a `VERDICT:` line** (see §Verdict). On `ship_it`, the `SubagentStop` hook `.claude/hooks/review-marker.sh` writes `tmp/docs-review-passed-`, which `.claude/hooks/agent-bash-gate.sh` requires before the push. +- **Advisory mode** — scope is an explicit **path/glob** or **`all`** (the ad-hoc `/docs-review ` / `/docs-review all` audit). Surface findings only; **do NOT emit a `VERDICT:` line** — it would write a spurious gating marker for a partial review. + +## Source of truth + +Read `.github/prompts/docs-review.md` first; it is the canonical docs-review rubric and applies here verbatim (focus areas, `[MUST]`/`[SHOULD]`/`[MAY]` tags, the noise filter, the "do not duplicate the deterministic tools" rule). Also read `AGENTS.md` §Documentation Sync (the code→docs map) and §SDK Sync, plus the architecture context — accuracy-vs-code and doc-sync are the top focus areas, so you need to know where the truth lives. + +## What counts as docs prose (scope) + +The canonical set is resolved by `scripts/docs-prose.sh` — a **denylist**: every tracked `*.md`/`*.mdx` file EXCEPT `.claude/**`, `.github/**`, `CHANGELOG.md`, `AGENTS.md`, `CLAUDE.md`, `*.draft.md`, `*.old.md`, `PERF-CLAIMS-REVIEW.md`. That is the Astro Starlight site under `docs/src/content/` **plus** the user-facing governance docs (`README.md`, `clients/ts/README.md`, `CONTRIBUTING.md`, `SECURITY.md`, `CODE_OF_CONDUCT.md`, `SUPPORT.md`) — and any doc added later, automatically. + +- `scripts/docs-prose.sh all` — every docs-prose file (the full reading list). +- `scripts/docs-prose.sh changed` — the docs-prose files changed on this branch (`main...HEAD`). + +## Reading strategy + +1. **Always read in full** — prose goes stale without being edited, so review whole files, not diffs: the docs site (`docs/src/content/**`), `README.md`, `clients/ts/README.md`, `CONTRIBUTING.md`, `SECURITY.md`. +2. **`CODE_OF_CONDUCT.md` / `SUPPORT.md`** are mostly static boilerplate — only deep-review them when they **changed on this branch** (`scripts/docs-prose.sh changed`) or when a material change elsewhere plausibly affects them (e.g. a new reporting contact, a moved support channel). Otherwise skip them. +3. In **advisory mode**, read only the path(s) in scope. + +## Process + +1. Read the rubric + `AGENTS.md` §Documentation Sync / §SDK Sync. +2. Resolve scope; read the docs per the Reading strategy. +3. **Accuracy vs. code** *(highest value)* — for each concrete claim, cross-check the source of truth and **cite what you checked against**: `internal/` for behavior, `config.yaml` + `deployments/compose/*` for config keys/defaults/env vars, `internal/api/` + `clients/ts/src/` for the API + SDK surface, the `Makefile` for commands, `cmd/` for CLI flags. +4. **Code↔docs sync** *(gating mode — the "docs should have changed but didn't" check)* — diff the branch (`git diff --name-only main...HEAD`) and walk the changed **code/config** against AGENTS.md §Documentation Sync + §SDK Sync. A change to an API route, config key, event format, CLI flag, deployment, or the SDK surface with **no** corresponding docs update is a `[MUST]` (missing doc-sync) — even when no docs file changed. Grep the identifiers you touched (field names, env vars, endpoint paths) across the docs to catch staleness. +5. **Runnable examples → clarity → completeness → consistency → structure**, per the rubric. +6. Apply the noise filter; tag each surviving finding `[MUST]` / `[SHOULD]` / `[MAY]`. + +If the branch has an open PR, fetch prior review comments (`gh pr view --json comments,reviews`) and don't re-raise what's already been flagged. + +## Output format + +```markdown +## Docs review — + + + +### [MUST] Findings +- `docs/src/content/docs/api.md:42` — "" contradicts `internal/api/foo.go:NN` (). Fix: . +- `internal/config/config.go:NN` adds the `retention_days` key but no docs update — `docs/src/content/docs/configuration.md` + `config.yaml` must document it (doc-sync). + +### [SHOULD] Findings +- ... + +### [MAY] Findings +- ... +``` + +If nothing is wrong, say so plainly — an empty findings list is a valid, good outcome (and in gating mode it is exactly what produces `ship_it`). + +## Verdict (gating mode only) + +End the review with a one-line verdict, **followed immediately by the parseable line on its own line**: + +```text +VERDICT: ship_it +``` + +or `VERDICT: iterate` or `VERDICT: block`. The line is consumed by `.claude/hooks/review-marker.sh` — incorrect formatting means no marker, no push. + +Mapping (same strict rubric as `pre-push-reviewer` — **`ship_it` requires zero findings at any severity**): + +- **`ship_it`** — `[MUST]`, `[SHOULD]`, and `[MAY]` are all empty: the docs are accurate, runnable, and in sync with the code. Marker auto-writes; push proceeds. +- **`iterate`** — any finding exists (including a single `[MAY]`), none block-level. The orchestrator fixes them, commits, and re-invokes you in fresh context until `ship_it`. +- **`block`** — a `[MUST]` wrong/misleading enough to need maintainer attention (e.g. documented security guidance that is unsafe, an architectural claim that is flatly false). + +Under this rubric `[MAY]` is a real commitment — "I'd fix this before merge," not "optional polish." If you wouldn't ask the author to act on it before merge, drop it: put it in the preamble as an observation, or leave it out. + +**Advisory mode emits NO verdict line** — just the findings above. + +## Framing + +A meticulous technical writer who is also a skeptical engineer: you don't trust a sentence describing the system until you've checked it against the code. Surface findings; the user (or orchestrator) decides what to act on. + +**Do not** edit the docs. **Do not** post comments on any PR. In gating mode your only side effect is the marker the hook writes on `ship_it`. If a docs change also touches code, that code still goes through `pre-push-reviewer` separately (run in parallel with you). diff --git a/.claude/agents/pre-push-reviewer.md b/.claude/agents/pre-push-reviewer.md index 919837cc..e0945e28 100644 --- a/.claude/agents/pre-push-reviewer.md +++ b/.claude/agents/pre-push-reviewer.md @@ -42,6 +42,7 @@ The diff source here is the local working state, computed as `git diff main...HE - **Testing** — new code on critical paths without tests, missing edge cases, mocks where integration would catch more. - **Documentation sync** — per AGENTS.md §Documentation Sync table. - **SDK sync** — per AGENTS.md §SDK Sync table. Did `internal/api/` change without `clients/ts/src/` consideration? + - **Docs prose** — prose *quality* (accuracy-vs-code, runnable examples, clarity, completeness) and the docs↔code sync check are the **`docs-reviewer`** subagent's job. It runs **in parallel with you as a mandatory pre-push gate**, with its own `tmp/docs-review-passed-` marker, so the push is already blocked until it reaches `ship_it`. Do **not** review prose or raise a "run `/docs-review`" `[SHOULD]` here — that gate fires on its own. (This supersedes the docs soft-gate in `.github/prompts/pr-review.md`: locally, docs review is a hard gate, not a nudge.) Don't line-edit prose yourself. You still keep the **Documentation sync** and **SDK sync** checks above as a code-completeness backstop. 6. Apply the noise filter from `pr-review.md` before finalizing: drop findings you wouldn't personally ask the author to change in-person. @@ -88,7 +89,7 @@ VERDICT: ship_it WaveHouse uses a stricter rule than `.github/prompts/pr-review.md`: **`ship_it` requires zero findings at any severity**. If there is anything left to do, the PR isn't shippable — "ship it, just do this one thing first" is iteration, not shipping. -- **`Ship it`** + `VERDICT: ship_it` — `[MUST]`, `[SHOULD]`, and `[MAY]` sections are all empty. Pre-push marker auto-writes, push proceeds. +- **`Ship it`** + `VERDICT: ship_it` — `[MUST]`, `[SHOULD]`, and `[MAY]` sections are all empty. Pre-push marker auto-writes, push proceeds. (Docs prose + docs↔code sync are gated independently by the parallel `docs-reviewer` and its own marker — see the Docs prose focus area — so they aren't your concern here.) - **`Iterate`** + `VERDICT: iterate` — any `[MUST]` / `[SHOULD]` / `[MAY]` finding exists, but none are block-level. The orchestrator fixes the findings and re-invokes this subagent (always in fresh context) until ship_it. - **`Block`** + `VERDICT: block` — a `[MUST]` that's CRITICAL/HIGH security, data-loss risk, broken core invariant, or otherwise needs human/maintainer attention (architectural disagreement, missing CI signal, etc.). Cannot proceed without addressing. diff --git a/.claude/commands/cover.md b/.claude/commands/cover.md index 299cedc9..021def65 100644 --- a/.claude/commands/cover.md +++ b/.claude/commands/cover.md @@ -8,6 +8,7 @@ Generate the coverage report and surface anything below threshold from `.testcov Suite to run: $ARGUMENTS Behavior: + - **no argument or "merge"**: just `make cov` (merges whatever Go + TS coverage exists under `tmp/coverage/` and gates against the thresholds) - **unit**: `make test-unit` (gates per-suite + writes `tmp/coverage/unit/`) - **integration**: `make test-integration` (requires Docker) @@ -18,6 +19,7 @@ Behavior: - **all**: `make test-all` (all four suites sequentially + `make cov`) After the run completes: + 1. Parse `tmp/coverage//coverage.txt` (Go) or `tmp/coverage/sdk/index.html` (SDK) for per-package coverage. 2. Identify packages below the suite's threshold from `.testcoverage.yml`. Report as a sorted list. 3. Surface the suite total + delta vs. the threshold. diff --git a/.claude/commands/docs-review.md b/.claude/commands/docs-review.md new file mode 100644 index 00000000..7b0722fa --- /dev/null +++ b/.claude/commands/docs-review.md @@ -0,0 +1,18 @@ +--- +description: Review docs prose (accuracy-vs-code, runnable examples, clarity, completeness) + code↔docs sync. No-arg = the gating pre-push review; a path/`all` = advisory. Complements misspell/markdownlint/links-validator. +argument-hint: "[path/glob | all] (default: branch docs review — gates the push)" +--- + +Run a documentation review — the accuracy / runnable-examples / clarity / completeness / doc-sync layer the deterministic tools (misspell, markdownlint, starlight-links-validator) can't cover. + +Scope: $ARGUMENTS + +Invoke the **`docs-reviewer`** subagent (fresh context, for objectivity) and pass it the scope above: + +- **empty** → **gating mode**: the branch's changes vs `main` (docs prose **and** code↔docs sync). This is the mandatory pre-push docs review — on `VERDICT: ship_it` the SubagentStop hook writes `tmp/docs-review-passed-`, which `.claude/hooks/agent-bash-gate.sh` requires before the push. Run it alongside `pre-push-reviewer` before pushing. +- **a path or glob** (e.g. `docs/src/content/docs/api.md`) → **advisory**: just those files, no verdict, no marker. +- **`all`** → **advisory**: the whole docs-prose set (`scripts/docs-prose.sh all`), no verdict, no marker. + +The subagent reads `.github/prompts/docs-review.md`, cross-checks each claim against the code/config, and returns `[MUST]` / `[SHOULD]` / `[MAY]` findings. It never edits docs and never posts PR comments. + +Relay the subagent's findings. In **gating mode**, loop — fix the findings, commit, re-invoke `docs-reviewer` in fresh context — until `ship_it`. In **advisory mode**, ask which to act on and offer to apply the ones the user picks. diff --git a/.claude/hooks/agent-bash-gate.sh b/.claude/hooks/agent-bash-gate.sh index 2cd69106..2c860912 100755 --- a/.claude/hooks/agent-bash-gate.sh +++ b/.claude/hooks/agent-bash-gate.sh @@ -6,7 +6,8 @@ # - gh pr edit --add-reviewer / --add-assignee # - gh api .../requested_reviewers (write verbs) # - gh pr review --approve / --request-changes -# - git push to a PR branch without a pre-push-reviewer review-passed marker +# - git push to a PR branch missing either pre-push review marker +# (pre-push-reviewer code review + docs-reviewer docs review) # # Universal git checks (ci-passed marker, no-verify, etc.) live in .githooks/ # and apply to humans and agents equally. Bypass surface acknowledged: an @@ -81,8 +82,12 @@ if printf '%s\n' "$stripped" | grep -qE '(^|[[:space:];|&]+)gh[[:space:]]+pr[[:s && block "Agents post inline review comments instead of --request-changes." fi -# git push to a PR branch requires a pre-push-reviewer review-passed marker. -# (The universal .githooks/pre-push handles ci-passed for everyone.) +# git push to a PR branch requires BOTH pre-push review markers — the +# pre-push-reviewer (code) marker AND the docs-reviewer (docs) marker. Both are +# unconditional: even a code-only change goes through docs review, because +# catching "code changed but the docs should have and didn't" is the docs +# reviewer's job. (The universal .githooks/pre-push handles ci-passed for +# everyone.) if git_subcmd 'push' && ! git_subcmd_is_help 'push'; then head_sha=$(git rev-parse HEAD 2>/dev/null || echo "") branch=$(git symbolic-ref --short HEAD 2>/dev/null || echo "") @@ -96,16 +101,27 @@ if git_subcmd 'push' && ! git_subcmd_is_help 'push'; then elif ! printf '%s' "$pr_view_out" | grep -qiE 'no (open )?pull request'; then block "Could not determine PR state for '${branch}': ${pr_view_out}" fi - if [ "$pr_state" = "OPEN" ] && [ ! -f "tmp/review-passed-${head_sha}" ]; then - cat >&2 <&2 <. The pre-push gate (in -# .claude/hooks/agent-bash-gate.sh) reads that marker to allow the subsequent -# `git push`. +# Both review subagents gate a push, each with its own HEAD-keyed marker: +# pre-push-reviewer → tmp/review-passed- (code review) +# docs-reviewer → tmp/docs-review-passed- (docs prose + code<->docs sync) +# When the subagent's last assistant message ends with `VERDICT: ship_it`, this +# hook writes the corresponding marker. The pre-push gate (in +# .claude/hooks/agent-bash-gate.sh) requires both markers to allow the +# subsequent `git push`. # # Why SubagentStop (not PostToolUse:Agent): the PostToolUse:Agent payload puts # the subagent's final text in `.tool_response.content[].text` (array of @@ -14,13 +16,13 @@ # both stable and what we actually need. Both events do fire on subagent # completion; we just use the one with the friendlier schema. # -# Why this hook exists at all: the orchestrator agent is denied direct writes -# to tmp/(ci|review)-passed-* (permission deny list + agent-bash-gate). Hooks -# run at Claude Code privilege level, NOT subject to the permission system, so -# this is the only path to creating the marker. The subagent's verdict is the -# gate; the orchestrator can't fake it because the subagent runs in fresh -# context with the canonical system prompt from -# .claude/agents/pre-push-reviewer.md. +# Why this hook exists at all: the orchestrator agent must not hand-write +# tmp/(ci|review|docs-review)-passed-* (policy in AGENTS.md §"Don't bypass the +# gates"). Hooks run at Claude Code privilege level, NOT subject to the +# permission system, so this is the only honest path to creating a marker. The +# subagent's verdict is the gate; the orchestrator can't fake it because each +# subagent runs in fresh context with the canonical system prompt from +# .claude/agents/.md. set -uo pipefail @@ -38,12 +40,17 @@ if ! command -v jq >/dev/null 2>&1; then fi # SubagentStop fires for every subagent completion (no matcher support per -# Claude Code docs), so we filter by `agent_type` in-script. +# Claude Code docs), so we filter by `agent_type` in-script and map it to the +# marker it gates. Any other subagent (Explore, Plan, …) is a no-op. if ! agent_type=$(printf '%s' "$input" | jq -r '.agent_type // empty' 2>/dev/null); then echo "review-marker: malformed SubagentStop payload; could not parse .agent_type — no marker written." >&2 exit 0 fi -[ "$agent_type" = "pre-push-reviewer" ] || exit 0 +case "$agent_type" in + pre-push-reviewer) marker_prefix="review-passed" ;; + docs-reviewer) marker_prefix="docs-review-passed" ;; + *) exit 0 ;; +esac if ! response=$(printf '%s' "$input" | jq -r '.last_assistant_message // empty' 2>/dev/null); then echo "review-marker: malformed SubagentStop payload; could not parse .last_assistant_message — no marker written." >&2 @@ -69,10 +76,10 @@ cd "${CLAUDE_PROJECT_DIR:-.}" 2>/dev/null || exit 0 head_sha=$(git rev-parse HEAD 2>/dev/null) [ -z "$head_sha" ] && exit 0 -if mkdir -p tmp && touch "tmp/review-passed-${head_sha}"; then - echo "📝 Pre-push review marker written: tmp/review-passed-${head_sha:0:8}" >&2 +if mkdir -p tmp && touch "tmp/${marker_prefix}-${head_sha}"; then + echo "📝 Pre-push marker written (${agent_type}): tmp/${marker_prefix}-${head_sha:0:8}" >&2 else - echo "review-marker: failed to write tmp/review-passed-${head_sha:0:8} — no marker written." >&2 + echo "review-marker: failed to write tmp/${marker_prefix}-${head_sha:0:8} — no marker written." >&2 fi exit 0 diff --git a/.github/prompts/docs-review.md b/.github/prompts/docs-review.md new file mode 100644 index 00000000..0d6a566f --- /dev/null +++ b/.github/prompts/docs-review.md @@ -0,0 +1,54 @@ +You are reviewing the **documentation** of the WaveHouse project — the prose itself, and whether it kept up with the code; not the code's correctness. Read AGENTS.md at the repo root first: §Documentation Sync maps each code area to the docs that describe it, §SDK Sync covers the client, and the architecture/config context tells you what the docs *should* say. + +**Scope** is the canonical docs-prose set resolved by `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` file EXCEPT `.claude/**`, `.github/**`, `CHANGELOG.md`, `AGENTS.md`, `CLAUDE.md`, `*.draft.md`/`*.old.md`, and `PERF-CLAIMS-REVIEW.md`. That is the Astro Starlight site under `docs/src/content/docs/` (`.md` + `index.mdx`) **plus** the user-facing governance docs — `README.md`, the SDK readme `clients/ts/README.md`, `CONTRIBUTING.md`, `SECURITY.md`, `CODE_OF_CONDUCT.md`, `SUPPORT.md` — and any doc added later (new files are covered automatically). `CODE_OF_CONDUCT.md` and `SUPPORT.md` are mostly boilerplate: only deep-review them when they changed or when a material change elsewhere warrants it. + +This review **complements** the deterministic layers that already run — do **not** duplicate them: + +- **misspell** (`make lint-prose`) — spelling + US/UK. Don't flag spelling or British spellings. +- **markdownlint** — Markdown *style* (heading increments, list markers, blank lines). Don't flag formatting. +- **starlight-links-validator** (`astro build`) — broken internal links + `#fragments`. Don't flag link validity. +- **astro check** — types + content-schema (frontmatter) errors. Don't flag those. + +Your job is everything those *can't* check: whether the docs are **accurate, runnable, clear, and complete**. That needs judgment and cross-referencing the code — which is exactly why it's an LLM review, not a linter. + +## What to read + +The scope (changed files, a path, or the whole site) is in the header above. + +1. **The docs in scope** — read them in full, as a newcomer would. A paragraph can go stale without being edited, so review the whole file, not just a diff. +2. **The code/config they describe** — this is the point of the review. Cross-check every concrete claim against the source of truth: `internal/` (behavior), `config.yaml` + `deployments/compose/*` (config keys, defaults, env vars), `internal/api/` routes + `clients/ts/src/` (API surface + SDK), the `Makefile` (commands), `cmd/` (CLI flags). A doc that contradicts the code is a `[MUST]`. +3. **Prior review comments** (if a PR) — don't re-raise what another reviewer already flagged. + +## Tone + +A meticulous technical writer who is also a skeptical engineer: you don't trust a sentence describing the system until you've checked it against the code. Reader-first — assume a competent engineer new to WaveHouse, and flag where they'd get lost, misled, or stuck. Be specific: cite the file/line, quote the problem, and propose the concrete fix (corrected fact or replacement wording). Don't invent complaints; if a doc is clear and correct, say so briefly. + +## Focus areas (in this order) + +1. **Accuracy vs. the code, and code↔docs sync** *(highest value)* — every concrete claim checked against the source: config keys and their defaults, env var names, CLI flags/subcommands, API routes + methods, request/response shapes, error codes, event formats, behavior under failure. Flag anything stale, wrong, or contradicted by `internal/` / `config.yaml` / `clients/ts/`. **Cite the code location you checked against** so the author can verify. **And the inverse** — walk the branch's code/config changes (`git diff main...HEAD`) against AGENTS.md §Documentation Sync + §SDK Sync: a changed API route, config key, event format, CLI flag, deployment, or SDK surface with *no* docs update is a `[MUST]`, even when no docs file changed ("the docs should have changed but didn't"). + +2. **Examples that actually run** — code samples, `curl` calls, CLI invocations, config snippets, SDK usage: would they work *as written* against the current system? Real flags, real fields, correct types, valid endpoints, imports that resolve. A copy-paste example that fails is a `[MUST]`. + +3. **Clarity & comprehension** — ambiguity, jargon/acronyms used before they're defined, unstated assumptions, steps out of order, a buried lede, pronouns with no clear referent, sentences a newcomer would have to re-read. Name the *specific* reader confusion, not "this is unclear." + +4. **Completeness** — missing prerequisites, setup steps, required config, failure/edge cases, or "what next" pointers. A documented happy path with no error path. A described feature with no example. + +5. **Consistency** — the same concept referred to by the same term throughout; consistent voice/person; parallel structure across sibling sections. (Flag wrong term *casing* like `clickhouse`→`ClickHouse` in prose; raw spelling is misspell's job.) + +6. **Structure & navigation** — heading hierarchy that matches the content's logical shape, sections short enough to scan, cross-references that point somewhere conceptually useful. + +## Output + +Tag every finding with exactly one severity at the start of the line: + +- `[MUST]` — wrong / contradicted-by-code, a broken example, or an omission that will block or mislead a reader. Fix before merge. +- `[SHOULD]` — a real clarity / completeness / consistency problem worth fixing, but not a blocker if the author pushes back with reason. +- `[MAY]` — minor wording or structure suggestion. Take or leave. + +Cite `file:line`, quote the offending text, and give the concrete fix. Group findings by severity, and open with a one-line headline — `N [MUST], N [SHOULD], N [MAY]` — plus the single most important thing to fix. + +## Noise filter + +Before finalizing, drop any finding you wouldn't personally raise to the author in person — quality over quantity. Re-read the "do not duplicate" list at the top and delete anything the deterministic tools already own. Don't flag self-evidently-fine prose just to have a finding. + +Surface findings for the reader (or orchestrator) to act on. Do **not** edit the docs and do **not** post comments on any PR. In the default (branch) pre-push scope this review **gates the push**: it ends with a `VERDICT:` line and the SubagentStop hook writes `tmp/docs-review-passed-` on `ship_it`, in parallel with the code-only `pre-push-reviewer`; for an explicit path or `all` it is advisory (no verdict, no marker). See `.claude/agents/docs-reviewer.md` for the mode/verdict mechanics. diff --git a/.github/prompts/pr-review.md b/.github/prompts/pr-review.md index 955c747f..96172a9b 100644 --- a/.github/prompts/pr-review.md +++ b/.github/prompts/pr-review.md @@ -44,7 +44,7 @@ Review against each of these, in this order: 4. **Testing** — new code without tests (especially on critical paths: auth middleware, ingest pipeline, policy evaluation, structured query builder, cache coherence, dedup). Missing edge-case coverage. Mocks where a real integration test would catch more (per the "no mocking DB" rule in the test conventions). Unit tests that don't actually exercise the code path they claim to. -5. **Documentation & doc-sync** — AGENTS.md has a hard rule that code changes affecting API / config / architecture / event format / deployment must update the corresponding docs, `CHANGELOG.md` (under `[Unreleased]`), and the compose files / `config.yaml` defaults. The table in AGENTS.md §"Documentation Sync" is authoritative — diff changed files against that map and flag every missed sync. +5. **Documentation & doc-sync** — AGENTS.md has a hard rule that code changes affecting API / config / architecture / event format / deployment must update the corresponding docs, `CHANGELOG.md` (under `[Unreleased]`), and the compose files / `config.yaml` defaults. The table in AGENTS.md §"Documentation Sync" is authoritative — diff changed files against that map and flag every missed sync. This check is about whether code changes *updated* the docs — not whether the docs *prose* is correct and clear. Prose quality (accuracy-vs-code, runnable examples, clarity, completeness) **and code↔docs sync** are the **`docs-reviewer`** subagent's job — canonical rubric in `.github/prompts/docs-review.md`. **Locally that's a hard pre-push gate** (`docs-reviewer` runs in parallel with its own marker), so don't hand-review prose here. In a context that can't spawn subagents or see local markers (e.g. the cloud PR review), raise a `[SHOULD]` recommending `/docs-review` be run instead. Either way, don't line-by-line prose-review in this pass. ## Output discipline @@ -68,6 +68,8 @@ Verdict rules (matched to the styleguide): - `Iterate` — one or more `[MUST]` findings that aren't `Block`-level, or multiple `[SHOULD]` findings that collectively need a pass. - `Ship it` — no `[MUST]`s and few or no `[SHOULD]`s. `[MAY]` findings alone don't preclude `Ship it`. +**Docs review:** prose quality + code↔docs sync are gated by the `docs-reviewer` subagent. **Locally** that's a hard pre-push gate (its own marker, run in parallel with this review), so there's nothing to add here. **In the cloud PR review** (no subagents, no local markers): if the PR changed docs prose and no docs review was run/folded in, don't return `Ship it` — prefer `Iterate` with the single action "run `/docs-review`". That nudge is the best a markerless context can do; a docs-prose change shouldn't ship un-reviewed. + What not to comment on: - Anything the linter already catches (gofumpt, govet, staticcheck, gosec, gocritic, errcheck, etc. — see `.golangci.yml`). CI enforces those. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5b2b2688..a7add71f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,8 @@ on: permissions: contents: write + # The docs-deploy tail steps post a sticky preview-URL comment on PRs. + pull-requests: write # PR pushes: cancel in-flight, only the latest commit matters. Main pushes: # queue serially so every commit gets its own run. @@ -112,3 +114,137 @@ jobs: echo '```' } >> "$GITHUB_STEP_SUMMARY" fi + + # ── Docs deploy ────────────────────────────────────────────────── + # `make ci` already built the site (build-all → build-docs renders + # Mermaid via Chromium + validates links → docs/dist/). These tail + # steps reuse that artifact instead of rebuilding, and run only when + # the WHOLE pipeline above is green: this head step is gated on + # success() explicitly, and every step below chains off its outputs, + # so any earlier failure short-circuits the deploy. Cloudflare's own + # Workers Builds can't build this site (no headless browser at build + # time), so we ship from here. See docs/wrangler.jsonc. + + # Only deploy when docs-affecting files actually changed, so unrelated + # Go-only PRs/merges don't upload previews or redeploy prod. Computed + # from the GitHub API (the checkout is shallow, so `git diff` against + # the base can't see it). Skipped on fork PRs — no secrets there. + - name: Detect docs changes + id: docs_changed + if: >- + success() && + (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + echo "result=true" >> "$GITHUB_OUTPUT"; exit 0 # manual → always + fi + if [ "${{ github.event_name }}" = "pull_request" ]; then + files="$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${{ github.event.pull_request.number }}/files" \ + --paginate --jq '.[].filename' || true)" + else + files="$(gh api "repos/${GITHUB_REPOSITORY}/compare/${{ github.event.before }}...${{ github.sha }}" \ + --jq '.files[].filename' || true)" + fi + # Empty (API hiccup / new branch) → deploy to be safe. + if [ -z "$files" ] || printf '%s\n' "$files" | grep -qE '^(docs/|pnpm-lock\.yaml|pnpm-workspace\.yaml|\.github/workflows/ci\.yml|\.github/actions/setup-env/)'; then + echo "result=true" >> "$GITHUB_OUTPUT" + else + echo "result=false" >> "$GITHUB_OUTPUT" + fi + + # Soft-gate on the secrets so a not-yet-configured repo (or a run + # before they're added) warns and skips instead of failing CI. + - name: Check deploy secrets + id: cfsecrets + if: steps.docs_changed.outputs.result == 'true' + shell: bash + env: + TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + ACCT: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + run: | + if [ -n "$TOKEN" ] && [ -n "$ACCT" ]; then + echo "ok=true" >> "$GITHUB_OUTPUT" + else + echo "ok=false" >> "$GITHUB_OUTPUT" + echo "::warning::CLOUDFLARE_API_TOKEN / CLOUDFLARE_ACCOUNT_ID not set — skipping docs deploy." + fi + + # Production: push (or manual dispatch) on main → publish the active + # version served on wavehouse.dev. Not best-effort: a failure here + # should surface as a red CI run. + - name: Deploy docs (production) + if: >- + steps.cfsecrets.outputs.ok == 'true' + && (github.event_name == 'push' || github.event_name == 'workflow_dispatch') + && github.ref == 'refs/heads/main' + working-directory: docs + shell: bash + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + run: pnpm exec wrangler deploy + + # PRs: upload a non-active version and capture its preview URL. A real + # upload failure (e.g. a misscoped CLOUDFLARE_API_TOKEN → "Authentication + # error [code: 10000]") reds the PR instead of hiding — the retry loop + # first absorbs transient Cloudflare API blips so a one-off hiccup + # doesn't. (Still gated behind the whole `make ci` above, so an unrelated + # test flake there can also block it.) + - name: Upload docs preview + id: preview + if: steps.cfsecrets.outputs.ok == 'true' && github.event_name == 'pull_request' + working-directory: docs + shell: bash + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} + run: | + log="$RUNNER_TEMP/wrangler-upload.log" + # Retry absorbs transient CF API blips; a persistent failure falls + # through to exit 1 and reds the PR. + n=0 + until pnpm exec wrangler versions upload 2>&1 | tee "$log"; do + n=$((n + 1)) + if [ "$n" -ge 3 ]; then + echo "::error::wrangler versions upload failed after $n attempts — see log above." + exit 1 + fi + echo "::warning::wrangler versions upload failed (attempt $n/3) — retrying in 10s." + sleep 10 + done + url="$(grep -iE 'Version Preview URL' "$log" | grep -oE 'https://[A-Za-z0-9.-]+\.workers\.dev' | head -1 || true)" + [ -z "$url" ] && url="$(grep -oE 'https://[A-Za-z0-9.-]+\.workers\.dev' "$log" | head -1 || true)" + echo "url=$url" >> "$GITHUB_OUTPUT" + if [ -n "$url" ]; then + { echo "### 📚 Docs preview"; echo "$url"; } >> "$GITHUB_STEP_SUMMARY" + else + echo "::warning::Upload succeeded but no preview URL was parsed from the wrangler output." + fi + + # One sticky comment, updated in place (matched by the marker) so + # re-pushes don't spam the PR. gh + GITHUB_TOKEN, same pattern as the + # triage/housekeeping workflows — no extra action dependency. + - name: Comment docs preview URL + if: steps.preview.outputs.url != '' + continue-on-error: true + shell: bash + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR: ${{ github.event.pull_request.number }} + URL: ${{ steps.preview.outputs.url }} + SHA: ${{ github.event.pull_request.head.sha }} + run: | + marker='' + body="$(printf '%s\n📚 **Docs preview** is live: %s\n\n_Updated for commit %s_' "$marker" "$URL" "${SHA:0:7}")" + cid="$(gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" --paginate \ + --jq 'map(select(.body | startswith(""))) | .[0].id // empty' | head -1 || true)" + if [ -n "$cid" ]; then + gh api -X PATCH "repos/${GITHUB_REPOSITORY}/issues/comments/${cid}" -f body="$body" >/dev/null + else + gh api "repos/${GITHUB_REPOSITORY}/issues/${PR}/comments" -f body="$body" >/dev/null + fi + echo "Commented preview URL on PR #${PR}: ${URL}" diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc new file mode 100644 index 00000000..f5870101 --- /dev/null +++ b/.markdownlint-cli2.jsonc @@ -0,0 +1,34 @@ +{ + // markdownlint-cli2 CLI config for the WaveHouse monorepo: which files to lint. + // + // Rule configuration lives in .markdownlint.json (NOT here) so that editor + // markdownlint extensions — which read .markdownlint.json directly — stay in + // sync with the CLI. See that file for why MD013 / MD033 / MD024 / MD041 are + // tuned. Biome owns JS / TS / JSON; markdownlint owns Markdown — no overlap. + // Driven from the root Makefile (`make lint` / `make fix`). + // + // markdownlint checks Markdown *style* only — it does NOT resolve links or + // routes (it lints one file at a time, with no model of the site). Internal + // link + #fragment validation is owned by starlight-links-validator at + // `astro build` (CI, via `make ci` → build-docs), the single source of truth + // for broken links. See docs/astro.config.mjs. + // + // NOTE: .mdx is intentionally NOT linted — markdownlint parses CommonMark, not + // MDX's JSX/import syntax (docs/src/content/docs/index.mdx). + // + // markdownlint-cli2 globs with dot:true, so `**/*.md` descends into hidden + // dirs — including .worktrees/, where this repo nests git worktrees. Honor + // .gitignore so a `make lint` / `make fix` from the *main* checkout never + // reaches into sibling worktrees, tmp/, or vendored Markdown — and `fix:md` + // (`--fix`) never rewrites another branch's files. The explicit ignores below + // stay as a fallback for the case where .gitignore is absent. + "gitignore": true, + "globs": ["**/*.md"], + "ignores": [ + "**/node_modules/**", + "**/dist/**", + "**/.astro/**", + "**/.pnpm/**", + "**/.worktrees/**" + ] +} diff --git a/.markdownlint.json b/.markdownlint.json index 9bdf8b20..f0dfe2e0 100644 --- a/.markdownlint.json +++ b/.markdownlint.json @@ -1,4 +1,8 @@ { + "default": true, + "MD013": false, + "MD024": { "siblings_only": true }, + "MD033": false, "MD041": false, - "MD013": false + "MD060": false } diff --git a/.vscode/settings.json b/.vscode/settings.json index 3aa8c0b9..fe4c4de5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -60,9 +60,38 @@ "editor.tabSize": 2 }, "[markdown]": { - "editor.tabSize": 2 + "editor.tabSize": 2, + // Apply markdownlint's auto-fixes on save — mirrors `make fix` / `pnpm run + // fix:md`. The DavidAnson.vscode-markdownlint extension and the CLI both read + // the same .markdownlint.json (rules) + .markdownlint-cli2.jsonc (globs), so + // the editor and CI stay in lockstep. + "editor.codeActionsOnSave": { + "source.fixAll.markdownlint": "explicit" + } }, + // ── Markdown link validation (VS Code built-in — Microsoft) ──── + // The only editor-side link checker we use is VS Code's own built-in + // Markdown validation — no third-party link extension. It accurately + // validates same-page #anchors and reference-style links (kept on as + // warnings). But it resolves root-absolute links against the workspace + // folder, so Astro/Starlight routes like /api or /configuration#authentication + // look "broken" to it even though they're valid pages, and there is no + // setting to remap that (microsoft/vscode#222020). So `fileLinks` validation + // is turned off — otherwise every /route link is a false positive. + // + // Cross-page links + fragments are validated authoritatively in CI by + // starlight-links-validator at `astro build` (docs/astro.config.mjs, wired + // through `make ci` → build-docs) — that is the single source of truth for + // "are the links good". + "markdown.validate.enabled": true, + "markdown.validate.fileLinks.enabled": "ignore", + "markdown.validate.fileLinks.markdownFragmentLinks": "ignore", + "markdown.validate.fragmentLinks.enabled": "warning", + "markdown.validate.referenceLinks.enabled": "warning", + "markdown.validate.unusedLinkDefinitions.enabled": "warning", + "markdown.validate.duplicateLinkDefinitions.enabled": "warning", + // ── Search exclusions ────────────────────────────────────────── "search.exclude": { "data/": true, diff --git a/AGENTS.md b/AGENTS.md index c9007e8b..f859f5ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -122,7 +122,7 @@ make clean-all # Full reset: above + data/ + docker volumes make dev-docs # Hot-reload Astro dev server on :4321 make build-docs # Production build → docs/dist/ make preview-docs # Wrangler preview of the production build (auto-builds if dist/ missing) -make branding-docs # Regenerate logo/favicon/OG assets from docs/scripts/branding/mark.svg +make branding-docs # Regenerate logo/favicon/OG assets from docs/src/assets/branding/mark.svg ``` Verbose test output: `V=1 make test`. Extra flags: `make test ARGS="-run TestFoo"`. @@ -133,7 +133,7 @@ Tooling notes: - Most dev tools (`gotestsum`, `gofumpt`, `goimports`, `govulncheck`, `go-test-coverage`, `deadcode`, `gsa`, `goda`) are pinned in `go.mod` via native `tool` directives and invoked with `go tool ` — no manual install needed. - `golangci-lint` is pinned in the Makefile (currently v2.11.4) and auto-installed to `.bin/_/` on first `make lint` (or via `make tools`). Not in `go.mod` — its dependency tree conflicts with the main module. - `pnpm` (>= 11.1) and `Node.js` (22 LTS — pinned via `.nvmrc` at the repo root, matches CI) must be on your PATH; the SDK, E2E test harness, and docs site all shell out to `pnpm`. `make tools` runs a single root `pnpm install --frozen-lockfile`, which installs all three workspace packages (`clients/ts/`, `tests/e2e/sdk/`, `docs/`). -- **Node workspace**: the SDK (`clients/ts/`, `@wavehouse/sdk`), E2E harness (`tests/e2e/sdk/`, `wavehouse-e2e`), and docs site (`docs/`, `wavehouse-docs`) are pnpm workspace packages, driven directly from the root `Makefile` via `pnpm --filter` — no sub-Makefiles. The user-facing targets are verb-first and live in their natural `make help` sections: `build-ts` / `dev-ts` / `test-ts` / `clean-ts` for the SDK, and `build-docs` / `dev-docs` / `preview-docs` / `branding-docs` / `clean-docs` (plus the hidden `install-playwright-docs` helper) for docs. TS formatting/linting is workspace-wide via Biome (one `biome.json`), invoked by `make fmt` / `make lint` / `make fix`. +- **Node workspace**: the SDK (`clients/ts/`, `@wavehouse/sdk`), E2E harness (`tests/e2e/sdk/`, `wavehouse-e2e`), and docs site (`docs/`, `wavehouse-docs`) are pnpm workspace packages, driven directly from the root `Makefile` via `pnpm --filter` — no sub-Makefiles. The user-facing targets are verb-first and live in their natural `make help` sections: `build-ts` / `dev-ts` / `test-ts` / `clean-ts` for the SDK, and `build-docs` / `dev-docs` / `preview-docs` / `branding-docs` / `clean-docs` (plus the hidden `install-playwright-docs` helper) for docs. TS/JS/JSON formatting & linting is workspace-wide via Biome (one `biome.json`, covering the SDK, E2E harness, and docs — `.astro` templates and Markdown are out of Biome's scope). Markdown across the whole repo is linted by markdownlint-cli2 (rules in `.markdownlint.json`, file globs in `.markdownlint-cli2.jsonc`; `.mdx` excluded) — that's Markdown *style*. Docs **prose** is linted separately by the `lint-prose` / `fix-prose` targets via misspell (curated common-typo + US-spelling/UK→US enforcement over `docs/src/content/**`, `.md` *and* `.mdx`; a pinned `.bin/` binary, distinct from the misspell analyzer golangci-lint runs on Go source — same fork, different entry point; autofixable via `make fix`). The split is style vs. words: markdownlint owns style, misspell owns spelling, Biome owns JS/TS/JSON — no overlap. (A full-dictionary spell-checker, cspell, was trialled and dropped: on these jargon-dense docs it flagged ~64 legitimate terms and zero real typos — an unbounded dictionary tax for no signal. Catching novel typos — and judging accuracy-vs-code, clarity, and completeness — is left to LLM review: the `docs-reviewer` subagent — a mandatory pre-push gate, run via `/docs-review` (see §Agent PR Discipline → Docs review) — which weighs a word in context against the code.) All run under `make lint` / `make fix` (Biome also under `make fmt`). - `GNU Make 4+` is required (uses `--output-sync=target`); macOS ships BSD Make 3.81 which will not parse the Makefile. See `docs/src/content/docs/development.md` § Prerequisites for the full setup checklist. - **Worktrunk** (`wt`) reads `.config/wt.toml`. On `wt switch --create `, post-start runs `wt step copy-ignored` (seeds `.bin/` + `node_modules/` from main) then `make tools` to finish bootstrap. Personal overrides in `~/.config/worktrunk/config.toml`. @@ -251,24 +251,21 @@ Agents CAN re-request bot reviewers by mentioning them in PR comments (`gh pr co ### Pre-push self-review is mandatory on PR branches -Before pushing to any branch with an open PR, agents must invoke the `pre-push-reviewer` subagent in fresh context. The subagent reviews: +Before pushing to any branch with an open PR, agents must invoke **two review subagents in fresh context, in parallel** — both are mandatory gates, each with its own marker: -- The full PR diff against `main` (merge-base) -- The latest commit specifically -- All open PR comments and reviews (top-level + inline) -- CI status / failing checks -- Linked issues' acceptance criteria +- **`pre-push-reviewer`** (code) reviews: the full PR diff against `main` (merge-base); the latest commit specifically; all open PR comments and reviews (top-level + inline); CI status / failing checks; linked issues' acceptance criteria. +- **`docs-reviewer`** (docs) reviews: docs prose for accuracy-vs-code, runnable examples, clarity, and completeness, **plus code↔docs sync** — code that changed but whose docs didn't (per §Documentation Sync). It runs on **every** push, even code-only ones: docs may not change but *should*, and catching that is the point. (See §Docs review for scope.) -The subagent's verdict is one of `ship_it`, `iterate`, or `block`. **`ship_it` requires zero findings at any severity** (`[MUST]`, `[SHOULD]`, `[MAY]` sections all empty). Anything in the findings list — including `[MAY]` — forces `iterate`. The rule is: if there's anything left to do, the PR isn't shippable. "Ship it, just do this one thing first" is iteration, not shipping. +Each subagent's verdict is one of `ship_it`, `iterate`, or `block`. **`ship_it` requires zero findings at any severity** (`[MUST]`, `[SHOULD]`, `[MAY]` sections all empty). Anything in the findings list — including `[MAY]` — forces `iterate`. The rule is: if there's anything left to do, the PR isn't shippable. "Ship it, just do this one thing first" is iteration, not shipping. **Both** reviewers must reach `ship_it`. -When the subagent's response ends with the parseable line `VERDICT: ship_it`, `.claude/hooks/review-marker.sh` writes `tmp/review-passed-` and the next `git push` succeeds. On `VERDICT: iterate` or `VERDICT: block`, no marker — the orchestrator agent **loops**: address every finding, commit, re-invoke `pre-push-reviewer` in fresh context, repeat until `ship_it`. Never push with open findings. +When a subagent's response ends with the parseable line `VERDICT: ship_it`, `.claude/hooks/review-marker.sh` writes its marker — `tmp/review-passed-` for `pre-push-reviewer`, `tmp/docs-review-passed-` for `docs-reviewer`. `git push` succeeds only when **both** markers exist for HEAD. On `VERDICT: iterate` or `VERDICT: block`, that reviewer writes no marker — the orchestrator agent **loops**: address every finding, commit, re-invoke the reviewer(s) in fresh context, repeat until both say `ship_it`. Never push with open findings. -The orchestrator agent cannot override the subagent's system prompt (it's the fixed file content of `.claude/agents/pre-push-reviewer.md`), and the subagent runs in a clean conversation context, so it doesn't share the orchestrator's bias toward its own work. +The orchestrator agent cannot override either subagent's system prompt (the fixed file content of `.claude/agents/pre-push-reviewer.md` / `.claude/agents/docs-reviewer.md`), and each runs in a clean conversation context, so they don't share the orchestrator's bias toward its own work. ### Don't bypass the gates - `--no-verify` on `git commit` / `git push` exists for human WIP / draft pushes. Agents should not use it. -- Markers (`tmp/ci-passed-tree-*`, `tmp/review-passed-*`) are written by `make ci` and the `pre-push-reviewer` SubagentStop hook. Don't `touch` / `Write` / `Edit` them by hand — if you feel tempted, the marker is wrong-shaped for the situation you're in. Run `make ci`, invoke the subagent, get the verdict. +- Markers (`tmp/ci-passed-tree-*`, `tmp/review-passed-*`, `tmp/docs-review-passed-*`) are written by `make ci` and the `review-marker.sh` SubagentStop hook (for both `pre-push-reviewer` and `docs-reviewer`). Don't `touch` / `Write` / `Edit` them by hand — if you feel tempted, the marker is wrong-shaped for the situation you're in. Run `make ci`, invoke the subagent(s), get the verdict. These are policy, not mechanically enforced. Bash can write a file a dozen ways; an agent can edit `.claude/hooks/agent-bash-gate.sh` itself. Trust beats whack-a-mole regex. @@ -282,6 +279,14 @@ wt switch pr: # worktrunk + gh CLI; or `gh pr checkout ` fa Then invoke `pre-push-reviewer`. Findings stay local — agents must not post comments on the PR manually; surface them to the user, who decides what to act on. +### Docs review + +Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed but whose docs didn't) are reviewed by the **`docs-reviewer`** subagent, not the code-focused `pre-push-reviewer`. The canonical rubric is `.github/prompts/docs-review.md`. It complements the deterministic prose tools — misspell, markdownlint, starlight-links-validator — reviewing only what they can't, and it never edits docs or posts PR comments. + +**Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHANGELOG.md`, `AGENTS.md`, `CLAUDE.md`, `*.draft.md`/`*.old.md`, `PERF-CLAIMS-REVIEW.md`. So it covers the Starlight site under `docs/src/content/` **and** the governance docs (`README.md`, the SDK readme `clients/ts/README.md`, `CONTRIBUTING.md`, `SECURITY.md`, `CODE_OF_CONDUCT.md`, `SUPPORT.md`) — new docs are picked up automatically. `CODE_OF_CONDUCT.md`/`SUPPORT.md` are deep-reviewed only on change or material suspicion. + +**It is a hard pre-push gate**, run in parallel with `pre-push-reviewer` (see §Pre-push self-review). Invoked with the **default (branch) scope** it emits a `VERDICT:` line; on `ship_it` the `review-marker.sh` SubagentStop hook writes `tmp/docs-review-passed-`, which the push gate requires — unconditionally, on every PR-branch push (even code-only ones). Run it via **`/docs-review`**; with **no arg** that's the gating review (branch scope), while an explicit **path/glob** or **`all`** is **advisory** (no `VERDICT:`, no marker) for ad-hoc audits. The whole dev team runs Claude Code and this command is tracked in-repo, so everyone runs it themselves; there is intentionally **no PR/cloud path** for docs review. + ## Documentation Sync Every code change should update the corresponding docs in the same PR. A code change without its doc update is incomplete. @@ -377,12 +382,11 @@ internal/query/ → Structured query AST + SQL builder internal/testutil/ → Shared test helpers (NopLogger, etc.) tests/ → Integration & E2E tests tests/integration/ → Go integration tests (//go:build integration; ClickHouse testcontainer) -tests/e2e/ → E2E test stack +tests/e2e/ → E2E test stack (scripts/orchestrator boots a ClickHouse testcontainer + the wavehouse-cov binary) tests/e2e/fixtures/ → Idempotent ClickHouse DDL scripts for test tables -tests/e2e/compose.yaml → Docker Compose with profiles (ClickHouse always; WaveHouse via --profile app) tests/e2e/sdk/ → E2E integration tests via TypeScript SDK (Vitest) -deployments/compose/ → Docker Compose files -deployments/docker/ → Dockerfiles +deployments/compose/ → Docker Compose files (standalone.yaml, dependencies.yaml) +deployments/Dockerfile → Runtime image (+ Dockerfile.goreleaser for release builds) docs/ → Project documentation .vscode/ → Workspace settings (gopls build flags, recommended extensions) ``` @@ -401,7 +405,7 @@ docs/ → Project documentation - **Issue triage** (`triage.yml`): GitHub Models classifies new/edited issues and applies `area/*` + `security` + `breaking-change` labels. - **Code review** (advisory; the `Admin approval` required status check + the ruleset are the actual merge gate): handled by external marketplace apps (CodeRabbit, Copilot) configured at the org/repo level, not by in-repo workflows. Inline findings post as review threads that `required_review_thread_resolution: true` blocks merge on until resolved. - **Dependabot auto-merge** (`dependabot-automerge.yml`): patch/minor bumps auto-approve + auto-merge; major bumps hold for human review. CI still gates the actual merge. Patch/minor bypass `Admin approval` (the workflow + CI passing is the trust model); major bumps fall through to admin review like any human PR — this closed a hole where a bot's APPROVED review (e.g. CodeRabbit) could merge a major bump without admin involvement (see #130). -- **Docs site deploy** (`wavehouse.dev`): driven by Cloudflare's Workers Builds (the native Git integration on the CF side), not a GitHub Actions workflow — so no `CLOUDFLARE_API_TOKEN` lives in the repo. Push to `main` runs `npx wrangler deploy` from `docs/` and updates `wavehouse.dev` within ~2 minutes; pushes to PR branches run `npx wrangler versions upload`, which publishes a per-version preview at `-wavehouse-docs.wave-rf.workers.dev`. Wrangler config (custom domain, observability, source maps, preview URLs) lives in `docs/wrangler.jsonc`; the build command (`pnpm install --frozen-lockfile && pnpm build`) and `docs/` root directory are configured on the CF dashboard side. The worker (`docs/worker/index.ts`, delegating to `cloudflare-md-router`) deploys alongside the static assets so `Accept: text/markdown` content negotiation works in production. +- **Docs site deploy** (`wavehouse.dev`): a tail step of the CI job (`.github/workflows/ci.yml`), **not** Cloudflare's Workers Builds. Workers Builds can't build this site — `rehype-mermaid` renders diagrams to themed SVG at build time via headless Chromium, and the Workers Builds image has no browser (and no root to apt-install one). `make ci` already builds `docs/dist/` on a runner with a cached Chromium, so the deploy reuses that artifact and runs only once the whole pipeline is green: push to `main` runs `wrangler deploy` (production → `wavehouse.dev`); PR branches run `wrangler versions upload`, publishing a per-version preview at `-wavehouse-docs.wave-rf.workers.dev` posted as a sticky PR comment. Deploys are skipped when no docs-affecting files changed and on fork PRs. **Requires `CLOUDFLARE_API_TOKEN` + `CLOUDFLARE_ACCOUNT_ID` repo secrets**, and Cloudflare Workers Builds must stay **disconnected** from the `wavehouse-docs` Worker (else it double-deploys and fails the browser-dependent build on every push). Wrangler config (custom domain, observability, source maps, preview URLs) lives in `docs/wrangler.jsonc`. The worker (`docs/worker/index.ts`, delegating to `cloudflare-md-router`) deploys alongside the static assets so `Accept: text/markdown` content negotiation works in production. ## Governance Files diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d251fcc..08b4f327 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + + ## Unreleased ### Removed @@ -23,9 +25,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Schema discovery and DLQ stats are now admin-only** (`RequireAdmin`), replacing the prior token-only `RequireAuthenticated` gate (removed). The `"*"` any-role wildcard remains removed from `policy.Evaluate` and pipe allowlists (column allow-lists unaffected). - **Fail-closed by structure, not a flag.** `Evaluate(nil)`/`IsAdmin(nil)` deny everyone — including the admin role — so deleting the policy from KV is a total lockout on its own (a fresh or emptied deployment is bootstrapped from the policy file, not an implicit `admin` grant); the `Store.SetFailClosed` retention hack and the `allowAnon` admit-decision plumbing are removed. - **Every authorization denial emits a structured `slog` WARN** (`internal/api/errors.go`). One shared `writeAuthzDenied` path logs `reason`, `role_observed` vs `role_resolved`, `roles_allowed`, the chi `route` pattern (low-cardinality, no concrete path params), `method`, `status`, and a `gate` tag that names the check that denied — `admin` (the `RequireAdmin` gate, including `/v1/schema` and `/v1/dlq/stats`, which carry no `/admin` prefix), `policy` (the per-table evaluator, which also logs the `table` + `action`), or `pipe` (which also logs the pipe name). The denial gates (`RequireAdmin` and the ingest/structured-query/pipes handlers) take an injected `*slog.Logger` (wired from `main`, like the policy/pipes stores) rather than reaching for `slog.Default()`, and log with the request context — so the record inherits the process logger's format/destination and, under the OTel logger, its `trace_id`/`span_id`, making a misconfigured role or policy visible without reproducing the request. +- **First-boot trial policy for the standalone Docker `compose` stack** (`deployments/compose/dev-policy.yaml` (new), `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.md`): the `docker compose` quickstart now seeds a permissive `public` policy on first boot — mounted read-only from `dev-policy.yaml` and wired via `WH_POLICY_FILE_PATH=/app/policy.yaml` — so `clone → ingest → query → stream` works **tokenless** out of the box. The grant is deliberately narrow: `public` read/write on the two demo tables (`clicks`, `events`) only, **not** admin — no raw SQL, no policy/pipe CRUD, no schema/DLQ admin, no other tables — so the file escalates nothing if it ever rides into a real deployment (those demo tables won't exist there). WaveHouse stays fail-closed (no policy = every request denied, including admin); this only seeds NATS KV once, after which KV is authoritative and edits go through `PUT /v1/admin/policy`. Tune it — or drop the `default_role` to require auth — before production (see `access-control.md`). Also documents the existing `clickhouse.http_scheme` key (default `http`) in the configuration reference. ### Changed +- **`docs-reviewer` is now a mandatory, always-on pre-push gate, run in parallel with `pre-push-reviewer`** (`scripts/docs-prose.sh` (new), `.claude/hooks/review-marker.sh`, `.claude/hooks/agent-bash-gate.sh`, `.claude/agents/docs-reviewer.md`, `.claude/agents/pre-push-reviewer.md`, `.github/prompts/docs-review.md`, `.github/prompts/pr-review.md`, `.claude/commands/docs-review.md`, `AGENTS.md`, `docs/src/content/docs/claude-code.md`): the docs-prose review was previously advisory (no marker, soft-nudged by a `pre-push-reviewer` `[SHOULD]`). It now emits a `VERDICT: ship_it|iterate|block` line in its default (branch) scope; the `review-marker.sh` SubagentStop hook writes `tmp/docs-review-passed-` on `ship_it`, and `agent-bash-gate.sh` requires **both** that marker and the existing `tmp/review-passed-` before any push to an open-PR branch — unconditionally, even for code-only changes, because the reviewer also checks **code↔docs sync** (code that changed but whose docs didn't, per §Documentation Sync). Scope widened from "Starlight site + top-level `*.md`" to a denylist resolved by `scripts/docs-prose.sh`: every tracked `.md`/`.mdx` except `.claude/**`, `.github/**`, `CHANGELOG.md`, `AGENTS.md`, `CLAUDE.md`, `*.draft.md`/`*.old.md`, `PERF-CLAIMS-REVIEW.md` — so the README (incl. the SDK's `clients/ts/README.md`), `CONTRIBUTING.md`, `SECURITY.md`, `CODE_OF_CONDUCT.md`, and `SUPPORT.md` are now covered and new docs are picked up automatically (`CODE_OF_CONDUCT.md`/`SUPPORT.md` deep-reviewed only on change). An explicit `/docs-review ` or `/docs-review all` stays advisory (no `VERDICT:`, no marker). The `pre-push-reviewer` docs-prose soft-gate is removed (the hard gate replaces it); the soft-gate nudge in `.github/prompts/pr-review.md` is retained only for the markerless cloud PR-review path. +- **Lint/format coverage extended to the docs package + repo-wide Markdown** (`biome.json`, `package.json`, `.markdownlint.json`, `.markdownlint-cli2.jsonc`, `Makefile`, `AGENTS.md`): Biome's `files.includes` now covers `docs/**` JS/TS/JSON (astro config, mermaid theme, sidebar, worker, dev scripts) on top of the SDK and E2E harness — `.astro` templates and Markdown remain out of Biome's scope by design. Markdown across the whole repo is now linted by `markdownlint-cli2` (new root devDependency), wired into `make lint` / `make fix` next to Biome; rules live in `.markdownlint.json` (so editor extensions stay in sync) and file globs in `.markdownlint-cli2.jsonc`. `MD013` (line length), `MD033` (inline HTML — the README's theme-aware ``), and `MD060` (table column style) are disabled as ill-fitting here; `MD024` (duplicate headings) is exempted in `CHANGELOG.md` only, since Keep a Changelog repeats `### Added` / `Changed` / `Fixed` per version. `.mdx` is not linted (markdownlint parses CommonMark, not MDX's JSX). - **Relicensed from MIT to Apache-2.0** (`LICENSE`, `clients/ts/LICENSE` (new), `NOTICE` (new), `README.md`, `CONTRIBUTING.md`, `SUPPORT.md`, `clients/ts/README.md`, `clients/ts/package.json`, `deployments/Dockerfile`, `deployments/Dockerfile.goreleaser`, `docs/src/components/Hero.astro`, `docs/src/content/docs/index.mdx`, `docs/src/content/docs/why-wavehouse.md`): the whole project moves from the MIT License to the Apache License 2.0 — a permissive→permissive change (Wave RF owns the copyright) that adds an explicit patent grant. Both `LICENSE` files carry the canonical Apache-2.0 text, a top-level `NOTICE` asserts `Copyright 2026 Wave RF`, and every textual reference is flipped: the README badge + text, CONTRIBUTING, SUPPORT, both Dockerfiles' OCI `licenses` label, the docs site's hero/stat card, the JSON-LD schema.org `license` URL, and the comparison table. - **Ingest worker batches and flushes per table** (`internal/ingest/worker.go`, `internal/ingest/worker_test.go`, `docs/src/content/docs/ingest-pipeline.md` (new), `docs/src/config/sidebar.ts`, `tests/e2e/sdk/*` , `tests/e2e/fixtures/`): closes #191. The worker previously kept a **single shared batch across all tables**, so a high-volume table tripping the 500-row size trigger could strand a low-volume table's rows in a batch that then waited out the 5s timer (and vice-versa) — the root cause of the `batching.test.ts` CI flake. - **Per-table batching.** A single JetStream consumer now feeds a `dispatchLoop` that parses just enough to route each message to a per-table goroutine (`tableLoop`), lazily spawned on first sight of a table. Each table has its own batch, deadline timer, and size trigger, so one table's traffic never delays another's. State is single-owner/lock-free (no mutexes on the hot path); flushes hand the goroutine a private row snapshot so the race detector stays clean. @@ -39,10 +44,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **SDK (breaking).** The `sys` namespace now exposes only a content-free server-online check. `wh.sys.health()` calls `GET /v1/health` and returns `Result` (was `Result` against `/health`) — branch on `error`, not `data`. **`wh.sys.ready()` was removed**: readiness is a per-call ClickHouse query that belongs to the load balancer / reverse proxy, not the client, and `/readyz` may be filtered at the proxy — probe `/readyz` directly for k8s/LB. The now-unused `Health` and `Ready` types were dropped from the public exports. - **SDK DX.** `Result` gained an `ok: boolean` discriminant so callers can branch with `if (result.ok)` (mirroring `fetch`'s `Response.ok`) instead of checking `error === null` — cleaner for content-free results like `health()`. Additive and backward-compatible: `data`/`error` are unchanged, so existing checks keep working. - **Deferred (issue #144 part 2):** per-dependency drill-down probes (NATS, ClickHouse, schema, DLQ) and the richer aggregate `/readyz` body were intentionally left out of this release to keep the change small. -- **Docs site auto-deploys to `wavehouse.dev` via Cloudflare Workers Builds, and is driven from the root `Makefile` through the pnpm workspace** (`Makefile`, `docs/wrangler.jsonc`, `docs/scripts/branding/` (moved from `scripts/branding/`), `AGENTS.md`, `docs/src/content/docs/development.md`, `docs/scripts/screenshot.mjs`, `.github/actions/setup-env/action.yml`). Three threads, one PR: +- **Docs site auto-deploys to `wavehouse.dev` from the CI job, and is driven from the root `Makefile` through the pnpm workspace** (`Makefile`, `.github/workflows/ci.yml`, `docs/wrangler.jsonc`, `docs/scripts/branding/` (moved from `scripts/branding/`), `AGENTS.md`, `docs/src/content/docs/development.md`, `docs/scripts/screenshot.mjs`, `.github/actions/setup-env/action.yml`). Three threads, one PR: - **Driven from the root Makefile via the pnpm workspace.** `docs/` (package `wavehouse-docs`) is a pnpm workspace package built and served straight from the root `Makefile` with `pnpm --filter wavehouse-docs run