Repository navigation
fix(lint): parse Markdown and MDX in WH001/WH002, autofix .mdx prose - #760
EricAndrechek wants to merge 57 commits into
Conversation
The repo's Actions cache sat at the 10 GB cap and evicted continuously. Entries are readable only from the saving ref and main, so each PR's own Go caches were dead weight that displaced main's warm ones. Go modules, Go build objects, the pnpm store and Playwright now save on main pushes and restore-only elsewhere; main restores gomod without restore-keys so superseded modules don't accumulate. housekeeping gains a cache-cleanup job on PR close that deletes the PR refs' caches. Refs #535 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…failed GOTOOLCHAIN=auto means max(local, go.mod), so a newer local Go (or runner image) builds and lints on a different toolchain than CI, and golangci-lint, built with go.mod's Go, panics on it. The Makefile now exports GOTOOLCHAIN derived from go.mod's go directive, and setup-env exports the same value for CI steps that run Go outside make. The setup-env comment that described auto as resolving go.mod exactly is corrected, and development.md records that bumping the go directive needs a golangci-lint built with at least that Go. pnpm-install ran with --reporter=silent, which prints nothing at all on failure, so a supply-chain policy violation surfaced as a bare Error 1. It now captures pnpm's default output and replays it only when the install fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pnpm prerequisite row omitted make verify, lint, fmt, fix, ci and test-all, all of which install pnpm deps (verify is what the pre-commit hook runs). The Go tool row missed two of nine pinned tools, the verify row advertised a manual -j, and the project tree skipped deployments/nats. CONTRIBUTING's doc-sync list also lacked the CHANGELOG row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WH001 joins only what micromark calls a paragraph (markdownlint's tokens for .md, an MDX parse for .mdx), so a four-space nested list item is no longer mistaken for indented code. WH002 asks markdownlint's own CommonMark tokens whether a fence sits inside an HTML block. The MDX fixer now runs both rules through the markdownlint library and unwraps .mdx prose too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Address pre-review: the PR-scoped churn came from PRs whose key missed main's, not every PR; two live generations are about 7.8 GiB so this refs #535. Cleanup gets its own concurrency group and tolerates a failed delete; pnpm prune runs on main only; docs and comments follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ten tests in tests/integration bring up servers or containers of their own (a ClickHouse, a NATS server, a Redis, processes on random ports) and spend most of their time waiting on leases, handovers and restarts. Run serially they made up most of the package's 337s on a 4-core machine. Marked t.Parallel, they overlap each other and the multi-process roles test, which already ran in parallel. Each was checked for shared state first: per-test tables and databases on the shared ClickHouse, no t.Setenv, no captured default logger, no fixed ports. TestShardClaims_ScaleOneThreeTwo and the tests that share the wired app stay sequential. Refs #701 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stop pnpm's node_modules purge prompt from hanging the captured install, document the GOTOOLCHAIN override and the golangci-lint minor-version coupling, and update the CI comments that still described GOTOOLCHAIN=auto. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The pre-push review gate could attest to code no reviewer read. review-marker.sh keyed the marker to whatever HEAD the session's launch directory had when the reviewer stopped, so a commit made while a reviewer ran inherited its ship_it, and a review of a sibling worktree marked the launch directory's branch instead. Reviewers now end with `REVIEWED: <sha>` naming the commit they pinned before reading anything. The hook writes the marker into each worktree on that commit, and only when that worktree's HEAD reflog shows it on the commit from the start of the review (the subagent transcript's first timestamp) until now. Same-commit entries such as `git reset` or `git stash` don't count as moves. A report delivered through the subagent hand-back leaves only closing text in last_assistant_message, so the hook reads the hand-back's message from the transcript. Every decision goes to stderr and to tmp/review-marker.log, which the push gate prints when it blocks. agent-bash-gate.sh checked HEAD of the session's launch directory for every push. It now follows the push to where it runs (the tool call's cwd, a preceding cd/pushd, git -C), resolves each refspec source to the commit it publishes, and looks for that commit's markers in every worktree of the repository. It blocks when it can't tell: a computed directory or refspec, GIT_DIR, --all/--mirror, a glob refspec, a wrapped push, or a payload without cwd. A push into another repository with no reviewer manifest is outside the policy. Pushes through `git -C` and from a detached HEAD were not gated at all before. skip-pre-push-review.sh writes to the worktree root when run from a subdirectory. .claude/hooks/review-gate.test.sh drives both hooks with synthetic events in a scratch repository with sibling worktrees; it is a make verify leaf (test-review-gate). Because the pre-commit hook runs it with GIT_DIR exported, it clears git's repository variables, names every repository with -C, and refuses to write unless git sees the scratch directory as outside any repository and creates each scratch repository where asked. /prepush, the reviewer agents, AGENTS.md and the Claude Code docs page describe the REVIEWED line and the rule not to move HEAD while reviewers run. Closes #454 Closes #565 Closes #659 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Integration job had grown to about 8 minutes and was CI's critical path. make test-integration now runs two parts, test-integration-app (tests/integration, with -parallel 8 since its parallel tests mostly wait) and test-integration-backends (internal/cache, natsspike and internal/mq's tagged tests), at once. CI runs one matrix job per part: the changes job reads INTEGRATION_PARTS from the Makefile (scripts/ci/integration-parts.sh) and hands the coverage job the fragments and producer jobs to wait for, so a part added to the Makefile gets its own job, and CI and a local run execute the same commands. Each part keeps its own build-cache suffix, so no part saves a cache holding only its own test binaries. internal/mq's integration tests were chosen by a -run name pattern, so a new prefix would have been skipped silently. scripts/ci/tagged-tests.sh run selects the tests that only build with the integration tag, and tagged-tests.sh check, run before every part, fails while a package with tagged tests belongs to no part. tagged-tests.test.sh checks the selection against go test -list on a throwaway module (a verify leaf). natsspike stays in the suite: it pins the nats-server behavior the external NATS topology rests on, and it runs beside internal/cache. Closes #701 Closes #692 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
make fmt only checks; make fix applies. The pnpm row now leads with make tools and covers dev-ts, check-docs and build-all, the hooks opt-out is documented, and AGENTS.md/CONTRIBUTING.md summaries match the Makefile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds fixtures for each #525 shape (nested and four-space-continued list items), each newly caught WH002 case (a fence glued to prose inside an open block, a fence under a self-closing tag), the MDX parse, and the fixer (fixpoint, list-item refusal, unparseable file). The fixture that asserted WH002 stays silent on `<Foo>` / prose / fence now asserts the interop report: that silence was the under-report #499 describes. Docs that described the line-shape rules, the four-space blind spot and hand-unwrapping .mdx are brought in line, and the CHANGELOG records it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Make 3.81 ignores a makefile-set --output-sync, it only interleaves output. The hooks opt-out is clone-wide and undone by the next make tools. make verify, not make fmt, is what CI runs. CONTRIBUTING's doc-sync list now points at AGENTS.md instead of duplicating it, and stale tree comments are fixed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Move the close-time cleanup into its own workflow (no always-skipped check, no concurrency trick), make it fail when a delete fails, and correct what a PR still saves (CodeQL, golangci, Astro). Docs: measurements move to the sizing section, units and stale sizes refreshed, prune comment no longer overclaims, AGENTS.md and the dev guide describe the new policy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The cheap filter in agent-bash-gate.sh strips quotes one line at a time, so a heredoc body, a multi-line quoted string or a comment that mentions `git push` reached the tokenizer, which correctly found no push, and the gate then refused the command as one it couldn't follow. It now refuses only when the tokens hold a `git … push` the walk didn't gate: behind an unknown wrapper, or inside a command substitution. The gate runs before the command line does, so in `git commit … && git push` it judged the commit before the new one. A push that follows a HEAD-moving git command (commit, merge, rebase, reset, checkout, switch, cherry-pick, revert, am, pull) on the same line is now blocked with "push in a separate command". Pushes inside if/while/until/for … do are followed instead of refused, and the matching refspec `:` (or `+:`) is blocked like --all instead of being read as a delete. review-marker.sh uses the hand-back message whenever the final message lacks either the VERDICT or the REVIEWED line, retries the transcript read once, and resolves an abbreviated REVIEWED sha that names exactly one commit. A ship_it without a REVIEWED line, which is what a session started before this change produces because reviewer definitions load at session start, logs how to recover, and the gate's block message says the same and now shows the latest decisions from every worktree's log. pre-push-reviewer pins its commit in step 1 with `git -C <path> rev-parse HEAD`. The docs say the gate checks the commit each refspec publishes (its tip, or HEAD with no refspec), and the remaining copies that described a VERDICT-only, HEAD-keyed marker now describe REVIEWED plus VERDICT and the reviewed commit. The test covers each change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… coupling Prefer go.mod's toolchain line, require x.y.z, document that a new minor needs a matching golangci-lint (it refuses to load its config, not panics), the go bump command, the pnpm prompt, and the Go prerequisite row. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-ups to the integration split. - scripts/ci/integration-parts.sh read only the first INTEGRATION_PARTS line, so a later `+=` or a second assignment changed what make runs but not CI's matrix or the coverage job's wait list. It now refuses anything but a single `INTEGRATION_PARTS := ...` line, and its --check mode, run by check-integration-parts before every part, fails when its list differs from make's own. integration-parts.test.sh covers the shapes it must refuse (a verify leaf). - scripts/ci/tagged-tests.sh missed tagged examples and tests whose signature wraps across lines. It now takes every top-level func named Test*, Fuzz* or Example*, and its test compares the selection with every entry `go test -list` prints, with an example and a wrapped signature among the fixtures. - Docs: AGENTS.md's CI summary, the ci.yml comments that still called e2e the long pole or counted three fragments, development.md's account of the parts and of which tests run in parallel, where a package with both unit and tagged tests goes, a fresh cache suffix per job in the README's add-a-job recipe, estimates flagged in the timing table, and a transition note for purging the retired -integration cache entries. Refs #701 Refs #692 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
markdownlint hands rules every `<!--` ... `-->` span masked, even one opened and closed in code spans paragraphs apart, so a join built from params.lines could write dots over real prose. The rules now rebuild the source from markdownlint's own micromark tokens (and fall back to reporting without a fix if that does not mask back exactly), and the MDX parse reads the same text, so `<!--` in a code span no longer makes a valid .mdx "not parseable". The fixer ignores those reports anyway. WH001 no longer joins a code-span line ending with spaces beside it or used as padding, image alt text or directive attributes; trims only spaces and tabs; and decides "line opens with a tag" and "holds an HTML comment" from tokens, not from a leading `<` or a marker in inline code. fix:md runs the .md passes even when the MDX fixer fails and exits with its status. Fixtures cover each case, render .mdx results through @mdx-js/mdx to prove nothing a reader sees changed, test the fixer's invariant directly, and run the shared constructs as .mdx too. Docs list every shape WH001 leaves alone and what `make fix` leaves for a human. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # Makefile # docs/src/content/docs/development.md
# Conflicts: # .github/workflows/README.md
# Conflicts: # .github/actions/setup-env/action.yml # .github/workflows/README.md # AGENTS.md
# Conflicts: # docs/src/content/docs/development.md
# Conflicts: # docs/src/content/docs/claude-code.md # docs/src/content/docs/development.md
Make 3.81 parses the Makefile and silently ignores --output-sync, so the cost is interleaved parallel output, not a parse failure. AGENTS.md still called it BSD and said it would not parse, and still described the e2e fixtures as DDL scripts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
agent-bash-gate.sh looked for an unfollowed push only when it had followed none, so a second push on the same line behind a wrapper or in a command substitution went unchecked once the first was gated. The walk now records each simple command it gates, and every other one is checked. GIT_DIR or GIT_WORK_TREE exported, declared or assigned earlier on the line now makes the push's repository unknown, the same as the inline form. The test requires jq up front, and development.md lists jq among the prerequisites, since make verify now runs it. pr-review-locally names the PR worktree's path in its reviewer prompts, because reviewers pin the worktree their prompt names, else their current directory. The reviewer agents say an unresolvable REVIEWED sha means no marker, and the remaining "marker for HEAD" phrasings say "for the commit being pushed". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A PR saves its own CodeQL dependency cache whenever main has no entry for the key, including after main's entry was evicted; the docs now say so with dated figures. Also restores the base wording of the housekeeping sentence in AGENTS.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Makefile and setup-env each derived the pin and had already drifted. scripts/ci/go-toolchain.sh now owns the derivation and the x.y.z check, with a fixture test as a verify leaf; the Makefile passes the pin to its parse-time go list, and development.md's Go version hints match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The missing-suffix error, the input description and the header comment still allowed reusing another flavor's suffix when the flags match. Two jobs on one key race to save it, which is why the integration parts each take their own. Also restores the blank line the merge dropped before the test-review-gate rule. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The #535 sentences had landed between the two-generation budget and the sentence it explains, so "cached once" read as a consequence of PRs no longer saving. Move them to the end of the paragraph, and give the CodeQL row one date and an order for "on main" and "gone". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The parse-time go list used the derived pin directly, so `make GOTOOLCHAIN=local` still downloaded go.mod's toolchain for it. $(GOTOOLCHAIN) equals the pin unless overridden on the command line, and passing it explicitly still covers Make before 4.4, whose $(shell) doesn't see exported variables. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In .mdx, WH001 joined the lines inside a `<pre>` element, so `make fix` and the save hook changed what renders (inside `<script>`, the code). CommonMark reads `<pre>`, `<script>`, `<style>` and `<textarea>` as an HTML block that runs to its closing tag, but MDX reads the tag as JSX and its text as an ordinary paragraph. In .mdx, WH001 now skips any paragraph that touches one of those blocks in markdownlint's CommonMark parse; prose inside `<div>`, `<Aside>` and other blocks a blank line ends is still joined. The MDX fixer tripped on the same blocks: for a fence inside `<pre>` it inserted blank lines that end nothing, so WH002 still fired, yet it wrote the file and printed "fixed". It now refuses that insert, says why, and leaves the file as it was, as AGENTS.md already said it did. Both read the blocks through one helper, verbatimHtmlBlocks(). development.md no longer says `make fix` prints the generic rules' .mdx findings; only `make lint` reports those. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…lchain versions go-toolchain.sh matched directives only at column 0, so a go.mod with an indented go line yielded no pin, and its validation checked a prefix, so go1.26.6-custom passed. Allow leading whitespace and anchor the version at the end. Docs and changelog now say the pin is the toolchain line, else the go line. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…o-cache-main-only
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
The base branch was changed.
# Conflicts: # AGENTS.md # docs/src/content/docs/development.md
|
📚 Docs preview is live → https://3046c744-wavehouse-docs.wave-rf.workers.dev
|
Go keys this module's own packages in the build cache by absolute directory, so every worktree compiled and cached its own copy of each variant (normal, test, `-race`, `-cover -coverpkg=./...`), and Go only evicts entries unused for five days. `-trimpath` makes the entries path-independent. - **Makefile.** Exports `GOFLAGS=-trimpath`, keeping any `GOFLAGS` the caller passes (also when given on the `make` command line). A `GOFLAGS` that already names `-trimpath` is left alone, so sub-makes don't prepend it again and `GOFLAGS=-trimpath=false make …` opts out. CI runs the same targets and inherits it. No workflow or `.goreleaser.yaml` change: they have no direct `go build`/`go test` calls that matter here, and release-build paths are #537. - **Path anchors.** `natstest.repoFile` walks up from the working directory to `go.mod`, because tests in several packages import it. The roles integration test builds the binary from two levels above its package directory, which is where `go test` runs it. Nothing else uses `runtime.Caller`; `debug.Stack` in `internal/api/router.go` only feeds a log field. - **Debugging.** `make build`'s debug binary is exempt (`scripts/build.sh` passes `-trimpath=false`) and keeps absolute source paths for Delve. Other binaries are trimmed and need a source-path substitution; `development.md` has the `dlv` example. ## Test plan This branch's diff is identical to #765's (same 6 files, +40/−11, compared line by line), which CodeRabbit approved. Re-run here against `main`: the pre-commit `make verify` and a full `make ci` (unit suite, integration suite including `TestRoles`, and the builds). The three measurement items below were measured on #765 and not repeated. - [x] `go test -trimpath -count=1 ./internal/mq/...` passes (inside `make ci`'s unit suite). - [x] `go test -trimpath -tags integration -run TestRoles ./tests/integration/` passes (inside `make ci`'s integration suite). - [x] `make test-unit` passes under the new flags (inside `make ci`). The profile naming files by import path, and `gocover-cobertura` emitting repo-relative filenames that all exist, were measured on #765. - [x] `make verify` passes. - [ ] (measured on #765) Cache sharing, counting `compile` steps of `go test -c -x ./internal/api` in a second checkout of the same commit: 0 with `-trimpath`, 23 without. - [ ] (measured on #765) A recursive make probe shows `GOFLAGS` stable at depth 0, 1 and 2 with it unset, in the environment, on the command line, and set to `-trimpath=false`. - [ ] (measured on #765) `make build`'s binary has absolute source paths; `make build-release`'s has none. - [x] `make ci` passes. ## Related Issues Closes #763. Related: #537 (`-trimpath` for release builds). Part of #740. Supersedes #765 (rebuilt on `main`, no longer stacked on #760). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Wjv2nu841pxTMdUWSAPmDH Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
WH001 and WH002 guessed at document structure from line shape, and both gaps in #525 and #499 came from that. This takes #499's option 1 (the rules read a real parser's token stream) and builds option 3 on top of it, so
.mdxprose is unwrapped bymake fixagain without the generic rules going near it.parser: "micromark"tokens for.md, and for.mdxan MDX parse with the extensions the docs site compiles with (micromark-extension-mdxjs,-gfm,-directive,-math; newscripts/markdownlint-rules/lib/mdx.mjs). A nested list item indented four spaces, or an item continued at four spaces, is now a paragraph rather than "indented code" (lint(WH001): four-space-indented lines are skipped, so hard-wrapped nested list items are never reported or fixed #525). Tables, code, headings, math, asides, JSX and ESM are excluded by construction rather than by regex. It also stops joining across a line ending inside inline math, an MDX expression, a link title, image alt text or directive attributes (the old rule joined those, which pulls the next line into a TeX%or JS//comment), or inside a code span where spaces sit beside the break or the break is padding. Blockquotes, lines that open with a tag, and lines holding an HTML comment are still kept apart, now decided from tokens; the docs list every shape that is neither reported nor fixed.<!--…-->span masked, even one opened and closed in code spans paragraphs apart; a join built from those lines wrote dots over prose. The rules rebuild the source from markdownlint's own tokens, and report without a fix if the rebuild doesn't mask back exactly. The.mdxparse reads the same text, so a<!--in a code span no longer makes valid MDX "unparseable".htmlFlowtoken? That catches a fence glued to prose inside an open block (<Aside>/Run this first:/ fence) and a fence under a self-closing tag, where the old detector saw one side or neither.scripts/fix-mdx.mjs(renamed fromfix-mdx-fences.mjs, since it now unwraps prose too) runs WH002 and WH001 alone through the markdownlint library that markdownlint-cli2 ships, which takes its config as given: the "rule X only" pass the CLI can't express. It loops to a fixpoint, never inserts a blank line inside a list item, a blockquote or a<pre>,<script>,<style>or<textarea>HTML block (a blank line there changes the rendered HTML or does not end the block), keeps an unbalanced<pre>verbatim, and refuses to write unless block structure, every code/math/ESM/JSX/table block, and the non-whitespace text are unchanged.fix:mdnow runs the.mdpasses even when it fails, then exits with its status.Every constraint #499 lists still holds: the generic rules never run over
.mdx(config globs untouched), a "WH001 and WH002 only" pass goes through the library,.mdstill gets two passes, edits are built from unmasked text and never move an HTML comment off its line, and the hook keeps its containment check. One existing fixture changes its assertion: "does not fire when the backward scan would cross prose" pinned the under-report #499 describes, and now asserts one report, at the fence, from the interop check.No docs page needed a fix: with the new rules
make lintreports 0 issues across all 49 Markdown/MDX files. The three nested bullets #522 unwrapped by hand were the only ones. Adds@mdx-js/mdx,micromarkand itsgfm,directive,mathandmdxjsextensions, andremark-gfm,-directiveand-math(the last three for the render comparison in the tests) as dev dependencies.Test plan
make cigreen locally.make test-md-rules: 137 fixtures pass. They cover each lint(WH001): four-space-indented lines are skipped, so hard-wrapped nested list items are never reported or fixed #525 shape, each newly caught WH002 case, the comment-mask and code-span cases, MDX parsing, the fixer (a full page, fixpoint, hazard closed per MD022, list-item, blockquote and<pre>refusals, exit statuses, unparseable file left alone) and its invariant directly; every shared "leave alone" construct also runs as.mdx, and each.mdxresult is rendered through@mdx-js/mdx(with remark-gfm, -directive, -math) and compared with the input's rendering. Mutations of each guard each fail at least one fixture..github/workflows/README.mdas it was before build(lint): enforce WH001 everywhere, drop vendored skill #522: the new WH001 reports exactly the three nested bullets build(lint): enforce WH001 everywhere, drop vendored skill #522 found by hand (lines 336, 345, 350), which the old rule missed.pnpm fix:md, restores every file byte for byte. With the old rules and fixer, 23.mdparagraphs stay wrapped with no report and about 270.mdxparagraphs stay wrapped.make lint-md: 0 issues in 49 files.make fixtwice: both runs exit 0 and leavegit statusempty.make build-docs: 22 pages built,starlight-links-validatorreports all internal links valid (no.mdxfile changes on this branch).ship_it; docs reviewship_it.Remaining findings: a
<pre>opened mid-line in.mdprose is still joined (a pre-existing limitation, not a regression).Related Issues
Closes #525
Closes #499
Part of #740
Stacked on #759 (base
docs/dev-guide-drift)Contains the final heads of its parent stack and today's
main; no rebase is needed when the parents merge.🤖 Generated with Claude Code