From 768dacc963f5c52e5705e9c416fed9b897a014d8 Mon Sep 17 00:00:00 2001 From: mrabine Date: Thu, 12 Nov 2026 18:09:52 +0100 Subject: [PATCH] Setup agent --- .agents/skills/add-gtest/SKILL.md | 22 +++++++++++++++ .agents/skills/benchmark/SKILL.md | 46 +++++++++++++++++++++++++++++++ .agents/skills/compile/SKILL.md | 25 +++++++++++++++++ .agents/skills/coverage/SKILL.md | 32 +++++++++++++++++++++ .agents/skills/create-pr/SKILL.md | 22 +++++++++++++++ .agents/skills/release/SKILL.md | 19 +++++++++++++ .agents/skills/test/SKILL.md | 35 +++++++++++++++++++++++ .claude/agents/explorer.md | 29 +++++++++++++++++++ .claude/agents/reviewer.md | 27 ++++++++++++++++++ .claude/hooks/format.sh | 17 ++++++++++++ .claude/rules/cpp-errors.md | 18 ++++++++++++ .claude/rules/cpp-style.md | 37 +++++++++++++++++++++++++ .claude/rules/fabric-api.md | 18 ++++++++++++ .claude/rules/hot-path.md | 33 ++++++++++++++++++++++ .claude/rules/tests.md | 22 +++++++++++++++ .claude/settings.json | 31 +++++++++++++++++++++ .claude/skills/add-gtest | 1 + .claude/skills/benchmark | 1 + .claude/skills/compile | 1 + .claude/skills/coverage | 1 + .claude/skills/create-pr | 1 + .claude/skills/release | 1 + .claude/skills/test | 1 + AGENTS.md | 35 +++++++++++++++++++++++ CLAUDE.md | 1 + 25 files changed, 476 insertions(+) create mode 100644 .agents/skills/add-gtest/SKILL.md create mode 100644 .agents/skills/benchmark/SKILL.md create mode 100644 .agents/skills/compile/SKILL.md create mode 100644 .agents/skills/coverage/SKILL.md create mode 100644 .agents/skills/create-pr/SKILL.md create mode 100644 .agents/skills/release/SKILL.md create mode 100644 .agents/skills/test/SKILL.md create mode 100644 .claude/agents/explorer.md create mode 100644 .claude/agents/reviewer.md create mode 100755 .claude/hooks/format.sh create mode 100644 .claude/rules/cpp-errors.md create mode 100644 .claude/rules/cpp-style.md create mode 100644 .claude/rules/fabric-api.md create mode 100644 .claude/rules/hot-path.md create mode 100644 .claude/rules/tests.md create mode 100644 .claude/settings.json create mode 120000 .claude/skills/add-gtest create mode 120000 .claude/skills/benchmark create mode 120000 .claude/skills/compile create mode 120000 .claude/skills/coverage create mode 120000 .claude/skills/create-pr create mode 120000 .claude/skills/release create mode 120000 .claude/skills/test create mode 100644 AGENTS.md create mode 100644 CLAUDE.md diff --git a/.agents/skills/add-gtest/SKILL.md b/.agents/skills/add-gtest/SKILL.md new file mode 100644 index 00000000..4d6e6607 --- /dev/null +++ b/.agents/skills/add-gtest/SKILL.md @@ -0,0 +1,22 @@ +--- +name: add-gtest +description: Add and register a gtest file for a join class. Use when creating tests for new or untested code. +--- + +# Add a gtest + +Input: module and class. + +1. Read the header and a neighbouring test (e.g. `core/tests/tcp_acceptor_test.cpp`) for the exact layout and license header. +2. List the class's public methods; create `/tests/_test.cpp` with one `TEST (, )` each (plus `create` (`construct` if the class has a `create` method)/`move`/`copy`), mimicking the neighbouring file for fixture naming and the `@brief Test method.` form, each covering success, errors, boundaries and overloads. Concurrent code gets a multi-threaded section for TSan. +3. Register it in `/tests/CMakeLists.txt` next to its neighbours: + + ```cmake + add_executable(.gtest _test.cpp) + target_link_libraries(.gtest ${JOIN_} GTest::gtest_main) + add_test(NAME .gtest COMMAND .gtest) + install(TARGETS .gtest RUNTIME DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME}/test) + ``` +4. Build and run it in dev-loop mode (`gcc-debug`, that target only) with skills `compile` and `test`. + +If a test reveals a bug, report the failing assertion and propose a diff first; do not change the code under test unprompted. diff --git a/.agents/skills/benchmark/SKILL.md b/.agents/skills/benchmark/SKILL.md new file mode 100644 index 00000000..bf882358 --- /dev/null +++ b/.agents/skills/benchmark/SKILL.md @@ -0,0 +1,46 @@ +--- +name: benchmark +description: Check join performance versus main with json-benchmark, pack-benchmark and queue-benchmark. Use at closing when benchmarked code changed. +--- + +# Benchmark a change + +Required at closing when the change touches benchmarked code: `data/` parsers and writers (`json-benchmark` for JSON, `pack-benchmark` for MessagePack `pack.hpp`, both for shared code such as `value*`, `sax*`), `core/` queues and memory backends (`queue.hpp`, `memory.hpp`: `queue-benchmark`). Bump/slab allocators have no benchmark: report **not verified** or measure ad hoc. Done means no degradation versus `main` beyond the run-to-run spread. If no suitable machine is available, report the item as **not verified** (indicative WSL2 numbers may be given but do not satisfy it) and ask the user how to proceed. + +## 1. Ask first + +Before running anything, ask the user: **is a real machine available, ideally both aarch64 and x86_64?** Memory ordering and SIMD behave differently on each, and numbers from a hypervisor are not trustworthy: `queue-benchmark` pins threads (`pthread_setaffinity_np`), which cannot control physical core placement under WSL2, and the `run_bench.sh` of `json-benchmark` and `pack-benchmark` refuse to start unless the governor is `performance` and boost is off, then wait for the CPU to settle, `ALLOW_NOISY=1` overriding it. If only WSL2 is available, say that results are indicative only, relative, and not publishable. The repos are expected next to join (`../`); if one is missing, clone `https://github.com/joinframework/.git` there, never into the join tree. + +## 1b. Put the machine in performance mode + +Read the current state first: `cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor` and `cat /sys/devices/system/cpu/cpufreq/boost` (Intel with `intel_pstate` uses `/sys/devices/system/cpu/intel_pstate/no_turbo` instead; not verified here). Expected: governor `performance`, boost off (`0`; `no_turbo` = `1`). Note the original values. + +```bash +echo performance | sudo tee /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor +echo 0 | sudo tee /sys/devices/system/cpu/cpufreq/boost # AMD / acpi-cpufreq +echo 1 | sudo tee /sys/devices/system/cpu/intel_pstate/no_turbo # Intel intel_pstate +``` + +Run the governor command and the boost command for this CPU with `sudo` only when justified, and say why. If `sudo` cannot run here (password prompt, no rights), give them exactly and ask the user to run them with the `! ` prefix so they execute in this session. Do not use `ALLOW_NOISY=1` to skip the check unless the user agrees to noisy numbers. When the campaign is over, remind the user of the original values so they can restore them. If the machine has no cpufreq interface (WSL2, some VMs), say the mode cannot be set and the numbers are indicative only. + +## 2. Compare, same machine, same session + +The repos fetch join from `main` by default (`FetchContent`, `GIT_TAG main`). To measure the branch, point them at the local checkout (`FETCHCONTENT_SOURCE_DIR_JOIN`, honoured by all three): + +```bash +cmake -S . -B /-branch -DCMAKE_BUILD_TYPE=Release -DFETCHCONTENT_SOURCE_DIR_JOIN= +``` + +Check each tree measures the intended join: `grep FETCHCONTENT_SOURCE_DIR_JOIN /CMakeCache.txt` must show its path, otherwise the numbers are GitHub `main`. json-benchmark and pack-benchmark pin a dconv tag: if the change needs an unreleased dconv, also pass `-DFETCHCONTENT_SOURCE_DIR_DCONV=` to both trees. + +Baseline: the same with a join `origin/main` worktree (`git -C fetch origin main && git -C worktree add --detach /base origin/main`) in `/-main`. Build both, then run the **binaries of each tree directly**, baseline and branch back to back, several times, alternating, with results written to the scratchpad. Do not use `run_bench.sh` for the comparison: each rebuilds `build/` against GitHub `main` and regenerates the committed `plots/`. Leave no changes in the benchmark repos; remove the worktree at the end. + +- **json-benchmark** (its `run_bench.sh`: conformance, then 3 repetitions, min reported, `taskset -c 2`, `GLIBC_TUNABLES=glibc.malloc.mmap_threshold=33554432:glibc.malloc.trim_threshold=4294967296`): `/benchmarks/performance/bench__join` with the same `taskset` and `GLIBC_TUNABLES`, keep the minimum; datasets `canada`, `citm_catalog`, `twitter`. Also run `/benchmarks/conformance/conformance --output /-conformance.json`: a conformance drop is a regression. `sonic-cpp` is built unconditionally with `-march=native`; building on aarch64 is not verified. +- **pack-benchmark** (same `run_bench.sh` protocol as json-benchmark): `/benchmarks/performance/bench__join`, same `taskset`, `GLIBC_TUNABLES` and conformance; datasets `canada`, `citm_catalog`, `twitter` (`.msgpack`). +- **queue-benchmark** (its `run_bench.sh`: 10 repetitions, means): e.g. `/benchmarks/mpmc/bench_mpmc_throughput --benchmark_filter="Join.*4/4/" --benchmark_repetitions=10 --benchmark_display_aggregates_only=true --benchmark_out=/-mpmc.json`. Throughput, bulk and latency for `Spsc`, `Mpsc`, `Mpmc`, `LocalMem` and `ShmMem`. + +Only the join entries matter for a regression check; the other libraries are context. + +## 3. Report + +Give: machine (CPU model, architecture, governor/boost, kernel, native or WSL2), repetitions, and a before/after table for the affected benchmarks with the relative change. Call a change real only if it exceeds the run-to-run spread; otherwise say it is within noise. When a delta is close to the spread, rerun with 5 repetitions per side and decide with google benchmark's `compare.py` (`/_deps/googlebenchmark-src/tools/compare.py benchmarks
.json .json`, needs `scipy`; merge each side's runs into one JSON first): with 3 per side its Mann-Whitney test cannot reach p < 0.05. A mechanism read from the diff (fewer atomics, false sharing removed) is a hypothesis until `perf stat -e cycles,instructions,cache-misses,branch-misses` shows it (needs `kernel.perf_event_paranoid` <= 1). Never claim a gain you did not measure, and say plainly when the machine made the numbers unreliable. diff --git a/.agents/skills/compile/SKILL.md b/.agents/skills/compile/SKILL.md new file mode 100644 index 00000000..83714dfa --- /dev/null +++ b/.agents/skills/compile/SKILL.md @@ -0,0 +1,25 @@ +--- +name: compile +description: Configure and build join with the CMake presets, as CI does. Use to compile a change or fix build errors. +--- + +# Compile + +Dev loop vs closing: see `AGENTS.md` "Definition of done". Dev loop: `gcc-debug` only, one target. Closing: gcc and clang, `debug` + `release` + `tsan` + `asan`; announce it, it is slow. + +```bash +cmake --preset +cmake --build --preset -j [--target .gtest] +``` + +Presets: `gcc-` / `clang-` + `debug`, `release`, `tsan`, `asan`; trees in `build//`. Fix warnings (`-Werror`), never silence them. `JOIN_ENABLE_IO_URING` differs per preset (see `CMakePresets.json`), so a path can be compiled by one compiler and not the other. + +## What CI builds on every PR + +1. `clang-format` at the version pinned in `build.yml` over the tree (gates everything; `exclude-regex: 'build'` also skips paths containing `build`, e.g. `value_builder*`, so format those locally). Check `clang-format --version`: if it differs, ask the user to install it; a format diff seen only in CI points to the version gap. +2. gcc and clang x amd64 and arm64, in `joinframework/join-ci`: `release` then `debug`; `tsan` and `asan` on amd64. +3. CodeQL on `gcc-debug`. + +## Report + +Presets built, with the first error or warning in full on failure. Missing dependencies: see the README, and ask the user to install them. diff --git a/.agents/skills/coverage/SKILL.md b/.agents/skills/coverage/SKILL.md new file mode 100644 index 00000000..d6bdbf47 --- /dev/null +++ b/.agents/skills/coverage/SKILL.md @@ -0,0 +1,32 @@ +--- +name: coverage +description: Check join coverage is not degraded versus main (gcc and clang). Use at closing. +--- + +# Coverage + +Coverage must not drop on **gcc and clang**. Use the `debug` presets (only ones with coverage on) and CI's `gcovr` options. `gcc-debug` has io_uring and `clang-debug` does not: compare each compiler only with itself. + +```bash +C=gcc # repeat with C=clang +GCOV=gcov; [ "$C" = clang ] && GCOV='llvm-cov gcov' +cmake --preset $C-debug && cmake --build --preset $C-debug +find build/$C/debug -name '*.gcda' -delete +ctest --preset $C-debug +gcovr --root . --gcov-executable "$GCOV" --exclude-pattern-prefix LCOV --exclude-branches-by-pattern '.*' \ + --exclude '.*/tests/.*' --exclude '.*/samples/.*' --json-summary /$C-${SIDE:-after}.json --print-summary build/$C/debug +``` + +Baseline: do the same on `origin/main` in a worktree (`git fetch origin main && git worktree add --detach /base origin/main`; configure, build, ctest, gcovr, same preset) with `SIDE=before`, into `$C-before.json`. Remove the worktree (`git worktree remove --force /base`) at the end, also when a step failed. + +Compare total line coverage and, above all, every changed file. Any drop fails: add tests (or `LCOV_EXCL_*` for deliberately unreachable code, sparingly) and re-measure. + +Needs `gcovr` and `llvm-cov` matching clang, and root for the privileged tests (skipped or failing otherwise, which hides their coverage). When a tool is missing or the change touches privileged code, save the steps above, after `set -e`, to `/coverage.sh` and run it in CI's image (`docker pull joinframework/join-ci:latest` first if it lacks `gcovr`), on copies so the host `build/` is untouched: + +```bash +rsync -a --exclude build ./ /head/ +docker run --rm --privileged --sysctl net.ipv6.conf.all.disable_ipv6=0 -v : -w /head \ + joinframework/join-ci:latest bash -c "bash /coverage.sh; rc=\$?; chown -R $(id -u):$(id -g) ; exit \$rc" +``` + +Same for the baseline with `-e SIDE=before -w /base`. `--privileged` is root-equivalent (skill `test`). If neither works, report coverage as **not verified**, never "not degraded". diff --git a/.agents/skills/create-pr/SKILL.md b/.agents/skills/create-pr/SKILL.md new file mode 100644 index 00000000..275d991a --- /dev/null +++ b/.agents/skills/create-pr/SKILL.md @@ -0,0 +1,22 @@ +--- +name: create-pr +description: Branch, commit, push and open a PR with the join conventions. Use when asked to commit, push or open a PR. +--- + +# Branch, commit, push + +**Ask the user before each of: creating the branch (propose the name), committing (show files and message), pushing (branch and remote), opening the PR (title and base).** One approval covers one action. Never merge, force-push or push to `main`. + +1. **Inspect**: `git status`, `git diff`, current branch. Stage nothing unrelated, nothing under `build/`. +2. **Validate**: the dev loop of `AGENTS.md` "Definition of done" passed; the closing checks run only when the user says the change is ready to merge. Say what was not verified when asking. +3. **Branch**: on `main`, `git switch -c `. Stay on a suitable branch; never commit on `main`. + - `feature/`: additions (class, API, option, tool). + - `bugfix/`: real bug, root cause fixed, regression test added. + - `hotfix/`: small quick fix, minimal diff. + + Ask once if bugfix vs hotfix is unclear. +4. **Version**: bump `VERSION` in every PR, unless it is already ahead of the latest tag at an equal or higher level: minor for a feature or an API change, patch otherwise (bugfix, hotfix, tools, CI, docs, agent config), major only on request. Minor or major also sets `PROJECT_NUMBER` = `.` in `doxyfile`. +5. **Commit**: explicit paths (no `git add -A`), one commit. Message: one capitalized imperative line, about 50 chars, no period, no body, no PR number, no trailer: `Fix netlink dump race`, or `Msgpack binary value type` for a new feature. +6. **Push**: `git push -u origin `. Never rewrite pushed history; report a rejected push. +7. **PR**: title = commit line, no body. Ask first; with `gh`: `gh pr create --base main --title "" --body ""`. Otherwise give `https://github.com/joinframework/join/compare/main...<branch>?expand=1`. +8. **Report** branch, commit, presets run and the link; do not claim CI is green. diff --git a/.agents/skills/release/SKILL.md b/.agents/skills/release/SKILL.md new file mode 100644 index 00000000..4676c70d --- /dev/null +++ b/.agents/skills/release/SKILL.md @@ -0,0 +1,19 @@ +--- +name: release +description: Tag and publish a join release from main. Use when asked to release or tag a version. +--- + +# Release + +**Ask the user before creating the tag and again before pushing it.** Never move, delete or re-push an existing tag; never push anything else. + +1. **Check**: + - On `main`, clean tree, up to date: `git fetch origin --tags && git status` and `git rev-parse HEAD origin/main` equal. + - `VERSION` was bumped by a merged PR (skill `create-pr`), is greater than the latest tag (`git tag -l --sort=-v:refname | head -1`) and `v$(cat VERSION)` does not exist locally or on `origin`. + - `PROJECT_NUMBER` in `doxyfile` = `<major>.<minor>` of `VERSION`. + - CI (`cd` workflow) is green on this commit: ask the user, or check with `gh run list --workflow cd.yml --commit $(git rev-parse HEAD)` if available. + + If a check fails, stop and say why. +2. **Tag**: lightweight tag on `HEAD` (= `origin/main`, the squash-merged PR commit), like the existing ones: `git tag v$(cat VERSION)`. +3. **Push**: `git push origin v$(cat VERSION)`. `release.yml` checks the tag against `VERSION`, builds (scan, tsan, asan, pack) and publishes the GitHub release with the `.deb` packages and generated notes. +4. **Report** the tag, commit and `https://github.com/joinframework/join/actions/workflows/release.yml`; do not claim the release is published until the user confirms the workflow passed. diff --git a/.agents/skills/test/SKILL.md b/.agents/skills/test/SKILL.md new file mode 100644 index 00000000..68956d00 --- /dev/null +++ b/.agents/skills/test/SKILL.md @@ -0,0 +1,35 @@ +--- +name: test +description: Run join tests (ctest or one gtest), including sanitizers and root-only tests. Use to validate a change or reproduce a failure. +--- + +# Test + +Build first with skill `compile`. Dev loop vs closing: see `AGENTS.md` "Definition of done". At closing, the `coverage` skill follows, and `benchmark` when the change touches benchmarked code. + +```bash +ctest --preset <preset> --output-on-failure [-R '^<name>\.gtest$'] +build/<compiler>/<kind>/<module>/tests/<name>.gtest --gtest_filter='Suite.test' --gtest_repeat=50 +``` + +`tsan` for races, atomics, reactor/proactor; `asan` for memory, UB, lifetimes. Repeat flaky or concurrent tests under `tsan` before calling them fixed. The `tsan`/`asan` test presets set `TSAN_OPTIONS`/`ASAN_OPTIONS`/`UBSAN_OPTIONS` (`CMakePresets.json`): export the same values when running a binary directly. + +## What CI tests on every PR + +gcc and clang x amd64 and arm64, in `joinframework/join-ci` (`--privileged`, IPv6): `release` then `debug` ctest; on amd64 `gcovr` coverage, then `tsan` and `asan`. Coverage is generated for gcc and clang, but only the gcc report is published (`deploy-coverage`, which can fail the run). + +Privileged tests (`CAP_NET_RAW`, veth, netns) run for real in CI; without root older tests fail locally and newer ones skip (new privileged tests use a fixture that probes the privilege and calls `GTEST_SKIP`, e.g. `PingTest`): ask the user to run them as root, see `AGENTS.md`. A skip is not a pass. `JOIN_ENABLE_IO_URING` differs per preset, so a path can be exercised by one compiler and not the other. + +## Run in the CI image (Docker) + +On a machine with Docker, the privileged tests can run in the CI image `joinframework/join-ci:latest` (same flags as CI: `--privileged`, IPv6 enabled). `--privileged` is root-equivalent: run it yourself only when justified, otherwise give the command to the user with the `! ` prefix; either way, analyze the log. The repo is copied into the container so the host `build/` is untouched. Example (not verified; change the preset as needed): + +```bash +docker run --rm --privileged --sysctl net.ipv6.conf.all.disable_ipv6=0 -v "$PWD":/src:ro joinframework/join-ci:latest bash -c 'mkdir /work && tar --exclude=./build -C /src -cf - . | tar -C /work -xf - && cd /work && git config --global --add safe.directory /work && cmake --preset gcc-debug && cmake --build --preset gcc-debug && ctest --preset gcc-debug --output-on-failure' 2>&1 | tee <scratchpad>/gcc-debug.log +``` + +Run the same steps for each preset required at closing. Without Docker, ask the user to run the root tests directly (see `AGENTS.md`). + +## Report + +Presets run and pass/fail counts. Report environmental failures or skips with the output; never claim success you did not observe. diff --git a/.claude/agents/explorer.md b/.claude/agents/explorer.md new file mode 100644 index 00000000..4ec0d035 --- /dev/null +++ b/.claude/agents/explorer.md @@ -0,0 +1,29 @@ +--- +name: explorer +description: "Read-only: map where a new join feature fits before coding. Use before implementing a feature." +tools: Read, Grep, Glob, Bash +--- + +You explore **join** before a feature is developed. Strictly read-only: never edit, write, commit or push. Bash is for inspection only (`git log`, `git show`, `grep`, `ls`), never builds. Read `AGENTS.md` and the `.claude/rules/` that match the area first. + +## Method + +1. Restate the feature in one sentence and name the module it belongs to (`core`, `crypto`, `data`, `fabric`, `services`; dependencies only flow `services` -> `fabric` -> `crypto` -> `core` and `services` -> `data` -> `core`). +2. Find the **closest existing analogues** (e.g. `Dhcp` for a new fabric protocol, `BasicBumpAllocator` for an allocator) and read them: public API shape, policy parameters, ownership, error reporting, sync/async forms. +3. Locate everything the change touches: headers, sources, `CMakeLists.txt`, the matching `tests/` files, samples, README sections, and the callers. +4. Classify the code as **hot path or not** (`hot-path.md`): allocation, locking, atomics and cache-line layout, progress guarantees, what could be a policy or computed at build time. +5. Check history with `git log` / `git show` on the analogues for earlier decisions, and the constraints in `.claude/rules/` (e.g. `fabric-api.md`). +6. List what would have to be tested (one file per class, one test per public method) and what the closing verification will exercise (privileges needed, io_uring, sanitizers). + +Stay narrow: read what answers the question, do not survey the whole tree. Ground every claim in a `path:line`; say "not verified" for anything you did not read. + +## Output (keep it under about 60 lines) + +- **Fit**: module, files to add or change, 1-2 sentences. +- **Analogues**: the classes to imitate, with `path:line` and what to copy. +- **Design options**: 2-3 at most, with a recommendation and why (performance, progress guarantee, policy vs runtime, API shape). +- **Impact**: callers, CMake, tests to create, README or docs, build options or dependencies. +- **Risks**: concurrency, lifetime, ABI/API breakage, privileged tests, platform limits. +- **Open questions** for the user, only those that change the design. + +Do not propose a diff and do not start implementing; the caller decides and develops. diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md new file mode 100644 index 00000000..67e939e7 --- /dev/null +++ b/.claude/agents/reviewer.md @@ -0,0 +1,27 @@ +--- +name: reviewer +description: "Read-only: review a join change for correctness, concurrency, memory, hot paths and conventions. Use before a PR." +tools: Read, Grep, Glob, Bash +--- + +You review C++20 changes in **join**. Strictly read-only: never edit, write, commit, push or comment on a PR. Bash is for inspection (`git diff`, `git log`, `git show`, `grep`) and, only if asked, building or testing in a scratch tree. + +## Scope + +Default: `git diff HEAD` (staged + unstaged) plus `git status --short --untracked-files=all`, reading new files in full. Otherwise `git diff main...<ref>` for a branch or range. Read the surrounding code and matching tests, not just the hunks, plus `AGENTS.md` and the applicable `.claude/rules/`. + +## Check, in priority order + +1. **Correctness**: logic, off-by-one, unchecked syscalls, `errno` clobbered, wrong error category, overflow/sign, endianness. +2. **Concurrency**: races, wrong memory orders (reason for aarch64), lock order, callback/listener lifetime vs reactor/proactor thread, `isReactorThread ()` deadlock guard (`resource_deadlock_would_occur`), io_uring buffer-ring use-after-free. +3. **Memory**: dangling views, overruns, allocator/arena lifetime, RAII gaps on early return, fd leaks. +4. **Hot path**: apply `.claude/rules/hot-path.md` to the diff (allocation, locks, progress guarantee, cache-line layout, policies, build-time computation, unmeasured performance claims). +5. **Conventions**: `.claude/rules/*`. +6. **Tests** (`tests.md`): one file per class, one test per public method, error paths and boundaries, registered in CMake, no weakened assertions, deterministic under TSan/ASan. +7. **Agent config** (`AGENTS.md`, `.agents/`, `.claude/`, `CLAUDE.md`): `AGENTS.md` and `.agents/` stay tool-agnostic (no dependency on `.claude/` files or Claude-only features); Claude-specific content stays in `.claude/` and `CLAUDE.md`; `CLAUDE.md` only imports `AGENTS.md`; every file stays short. + +Skip what `clang-format` fixes and speculative refactors. + +## Output + +Findings by severity, each with `path:line`, one-sentence defect, a concrete failure scenario, and a proposed fix as a small diff (not applied). Mark **confirmed** (traced in code) or **plausible**; drop what you cannot ground. If nothing survives, say "No findings" with what you checked and could not check. End with a one-line verdict. diff --git a/.claude/hooks/format.sh b/.claude/hooks/format.sh new file mode 100755 index 00000000..27ff1c0a --- /dev/null +++ b/.claude/hooks/format.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +re='"file_path": *"([^"]*\.(hpp|cpp))"' +sre='"session_id": *"([^"]*)"' +in=$(cat) +[[ $in =~ $sre ]] && mark=${TMPDIR:-/tmp}/format-hook-${BASH_REMATCH[1]} +[[ $in =~ $re ]] || exit 0 +file=${BASH_REMATCH[1]} +[[ $file == "$CLAUDE_PROJECT_DIR"/* ]] || exit 0 +v=$(sed -n "s/.*clang-format-version: *'\([0-9]*\)'.*/\1/p" "$CLAUDE_PROJECT_DIR"/.github/workflows/build.yml | head -1) +cf=clang-format-$v +command -v "$cf" >/dev/null || cf=clang-format +command -v "$cf" >/dev/null || { echo "format hook: clang-format is missing, nothing was formatted. Ask the user to install it (AGENTS.md, Definition of done)." >&2; exit 2; } +[ -f "$file" ] || exit 0 +"$cf" -i "$file" || { echo "format hook: $cf failed on $file" >&2; exit 2; } +have=$("$cf" --version | sed -n 's/.*version \([0-9]*\)\..*/\1/p') +[ -z "$v" ] || [ "$have" = "$v" ] || [ -e "$mark" ] || { touch "$mark"; echo "{\"hookSpecificOutput\":{\"hookEventName\":\"PostToolUse\",\"additionalContext\":\"format hook: formatted with clang-format $have, CI uses $v; ask the user to install clang-format $v.\"}}"; } +exit 0 diff --git a/.claude/rules/cpp-errors.md b/.claude/rules/cpp-errors.md new file mode 100644 index 00000000..c52a9113 --- /dev/null +++ b/.claude/rules/cpp-errors.md @@ -0,0 +1,18 @@ +--- +paths: + - "**/*.hpp" + - "**/*.cpp" +--- + +# C++ errors, types and idioms + +## Errors +- Non-throwing operations set the thread-local `join::lastError` (`std::error_code`) and return `-1` / `nullptr` / `false`: `lastError = std::error_code (errno, std::generic_category ());` or `lastError = make_error_code (Errc::InvalidParam);`. +- `generic_category` is the default for POSIX errno; the proactor/io_uring code uses `system_category`. Follow the neighbouring file. +- Constructors and one-time setup throw `std::system_error (errno, <category of the neighbouring file>, "<syscall> failed")` (or rethrow `std::system_error (lastError, "...")` after a non-throwing call), documented with `@throw`. Exceptions and `std::function` are to be avoided, never banned outright: use or propose them only with a clear justification, and not on the data path described in `hot-path.md`. +- Capture `errno` right after the failing call. + +## Idioms +- Named casts only: `static_cast` (common), `reinterpret_cast` at the OS/wire boundary, rarely `const_cast`; no C casts or `dynamic_cast`. `nullptr`, never `NULL`. `uint8_t`, `size_t` unprefixed. +- Prefer join's own `Function`, `Variant`, `View`, `Mutex` + `ScopedLock`, `Thread`, `Queue`, allocators. Not used here: `std::variant`, `std::mutex`, `std::lock_guard`; `std::span`/`std::optional` only in the data API (SAX handlers, `Value`, readers/writers). Used: `std::string_view`, `std::atomic`, `std::unique_ptr`, `std::shared_ptr` (`Ptr` aliases) and `std::function` (legacy manager listeners, thread entry points) off the hot path; new listeners use `Function`. +- Locks via `ScopedLock`, never manual `lock`/`unlock`. Network byte order via `htons`/`ntohl`...; bit reinterpretation via `std::bit_cast`, host-order tests via `std::endian`. diff --git a/.claude/rules/cpp-style.md b/.claude/rules/cpp-style.md new file mode 100644 index 00000000..5e5b1d45 --- /dev/null +++ b/.claude/rules/cpp-style.md @@ -0,0 +1,37 @@ +--- +paths: + - "**/*.hpp" + - "**/*.cpp" +--- + +# C++ style + +`.clang-format` is enforced (hook, or `clang-format` at the CI version); never hand-format. Errors and idioms: `cpp-errors.md`. + +## Layout +- C++20, no extensions. Space before every `(` (`foo (a)`, `TEST (S, n)`; except `operator~(T)`). Allman braces on every control statement, no one-line functions or lambdas. Constructor initializers one per line, leading comma. Digit separators `1'000'000`. +- Header: MIT license, `#ifndef JOIN_<MODULE>_<NAME>_HPP` (no `#pragma once`; the `*_uring*`/`*_epoll*` backend headers have none and are quote-included by their front header), content in indented `namespace join`. Backends guarded by `JOIN_HAS_IO_URING` / `_NUMA` / `_XDP`. +- Include groups, in order: `// dconv.` (data only), `// libjoin.`, `// Libraries.`, `// C++.`, `// C.` (kernel `<linux/...>` too); `<join/...>` for join headers. +- Templates are implemented in class in the header (only the proactor / io_ring_buffer backends define out of class). Other classes go in `src/<name>.cpp` (`using join::X;` after the includes), except small or hot-path classes kept in the header (`Backoff`, clocks, `PackWriter`): follow the neighbours. Helpers nest in the owning class; helper namespaces are `details`. +- Each out-of-class definition (`.cpp`, proactor backend headers) has this banner: + +```cpp +// ========================================================================= +// CLASS : Mutex +// METHOD : lock +// ========================================================================= +``` + +## Naming +- `PascalCase` types, enum classes and values, template parameters, aliases; `camelCase` functions, accessors `get`/`set`/`is`/`has`. +- Members `_camelCase`, default-initialised when they have a sentinel (`int _handle = -1;`), no `m_`. `this->` only for base members (dependent bases in templates, `std::streambuf` in the stream buffers). Public constants `camelCase`, private `static constexpr` `_camelCase`; traits `snake_case_v`; macros `JOIN_UPPER`. +- Bitmask enums: `enum class X : uint32_t`, values `1L << n`, full set of free bit operators. + +## Classes +- Order: `public`, `protected`, `private`; constructors, special members, destructor, API. +- Special members explicit (`= delete` copy then move, or `= default`); moved-from objects stay valid and empty. `explicit` single-argument constructors, `override` always, `virtual ~X` only on real bases, `final` on new leaves (legacy `Arp`, `Ping` are not). +- `noexcept` when it cannot throw, `constexpr` wherever possible, `[[maybe_unused]]` on unused parameters, `static_assert` with message. No `[[nodiscard]]`. + +## Doxygen +- Every header declaration: `/**`, ` * @brief ...` (lowercase), then `@param`/`@return`/`@throw` lines, ` */`. One short line per tag, a noun phrase ending with a period; no rationale (it goes in the reply or commit). Wording: `create instance.`, `destroy instance.`, `copy constructor.`, `move assignment operator.`, `@param other other object to copy.`, `@return current object.`. Class members: `/// text.` above; enum values and plain struct fields: trailing `/**< ... */`. +- No comments inside method bodies except `// LCOV_EXCL_LINE` and `LCOV_EXCL_START`/`STOP`. diff --git a/.claude/rules/fabric-api.md b/.claude/rules/fabric-api.md new file mode 100644 index 00000000..ae8a609a --- /dev/null +++ b/.claude/rules/fabric-api.md @@ -0,0 +1,18 @@ +--- +paths: + - "fabric/**" + - "core/include/join/reactor.hpp" + - "core/include/join/proactor*.hpp" +--- + +# fabric / event-driven API conventions + +- New protocol and socket leaf classes are `final` (Ndp, Dhcp clients/servers); legacy `Arp` and `Ping` are not. No user-overridable virtual callbacks. +- Listeners are mutated through `reactor.invoke`: single-slot `setXxxListener (cb)` / `unsetXxxListener ()` returning `int` (`Ndp`, `Dhcp`), or `uint64_t addXxxListener (cb)` / `void removeXxxListener (id)` (`NeighborManager`, `RouteManager`, `InterfaceManager`). Single-slot callbacks are `join::Function` (`Ndp`, `Dhcp`); the legacy managers use `std::function`. New listeners use `Function`. +- The final class registers with the reactor through the base's register helper once fully constructed, not in the base constructor (`BasicNdp`, `BasicDhcp`). +- Sync / async shape: `op ()` never waits; `op (Result&, timeout)` waits and fills the reference. The sync form fails with `std::errc::resource_deadlock_would_occur` (EDEADLK) when `_reactor.isReactorThread ()`. +- Keep the `Protocol` template parameter for consistency with `Dhcp`, even if only one protocol exists. +- Common behaviour lives in the base class (e.g. neighbor discovery in `BasicNdp`), role-specific code in the final classes. +- The lib is moving from the reactor to the proactor (NDP first): do not add new reactor-only code paths without checking with the user. +- Every feature exists on both epoll and io_uring, or its scope is reduced explicitly. +- Caller-owned buffers and endpoints are never copied; they stay valid until the handler runs. Signatures forbid dangling temporaries (`asyncWriteTo` takes a non-const `Endpoint&`, `connect` a `const Endpoint&`). diff --git a/.claude/rules/hot-path.md b/.claude/rules/hot-path.md new file mode 100644 index 00000000..a490d736 --- /dev/null +++ b/.claude/rules/hot-path.md @@ -0,0 +1,33 @@ +--- +paths: + - "core/include/**" + - "core/src/**" + - "data/include/**" + - "data/src/**" +--- + +# Hot paths: performance and determinism first + +Targets: zero allocation, zero lock, bounded latency, low jitter. Data path: proactor/reactor submit-complete-wakeup, queues, allocators, timer wheel, statistics, socket I/O, parser/writer loops. Constructors, setup, teardown, configuration and listener management are control path (may allocate, throw, lock). Judge per method, not per file; ask the user if unsure. + +## No allocation +- Avoid `new`, `malloc`, `std::string`, growing `std::vector`, `std::function`, `std::shared_ptr`, exceptions: no flat ban, but any of them needs a strong justification stated up front. Memory comes from join allocators (`LocalMem`, bump, slab) sized at construction; value trees use the bump allocator. +- Parser helpers: buffer-based `noexcept` APIs only; no `std::string`/`u16string` overloads, `codecvt`, locale. + +## No lock, progress guarantees +- No `Mutex`, condition, `ScopedLock` or blocking syscall (locks stay in setup, `ThreadPool`, `Condition`); use atomics and the lock-free queues. +- Prefer **wait-free**, then **lock-free**. Wait-free: `Spsc::tryPush`/`tryPop` (blocking `push`/`pop` spin with `Backoff`). Lock-free: the slab free list (CAS + `SlabTaggedIndex` generation). `Mpsc`/`Mpmc` are sequence-slot queues, not strictly lock-free (a preempted producer can stall the consumer; `Mpmc` bulk push spins on `_seq`). Document the real guarantee, never a stronger one. +- Weakest topology that fits (SPSC < MPSC < MPMC); one writer per cache line before CAS contention. CAS loops bounded or provably progressing; ABA prevented by a generation tag (`SlabTaggedIndex`; `_seq` is a slot ticket, not ABA protection); document reclamation. +- Explicit memory orders: `acquire`/`release` pairs, `relaxed` for pure counters, `seq_cst` only with a reason. Reason for aarch64; validate with `tsan`. + +## False sharing +- Independently written atomics on their own line: `alignas (64)` (`QueueSync`, `BasicStats`, `_wakeupState`). Group fields written by one thread; keep read-mostly fields (`_capacity`) apart. +- Cross-thread slots and commands are `struct alignas (64)`, padded with `(64 - (sizeof (...) % 64)) % 64`. When changing a shared layout, `static_assert` it (`sizeof`/`alignof`/`offsetof`) and state which thread writes which field. Keep the literal `64`. + +## Policies and build time +- Variability is a policy parameter, not a runtime flag, virtual call or `std::function`: `IoPolicy`, `SyncPolicy` (`SyncBinding`), `ClockPolicy`; data uses mode parameters (`JsonReadMode`, `JsonWriteMode`). A policy is a stateless struct of `static constexpr` members or tags; optional members detected with `requires` (`has_x_v`), defaulted by an `x_v` specialisation (`io_policy.hpp`); select with `if constexpr` or specialisation. `Function<Sig, Capacity>` is the non-allocating callable. +- Compute at build time: `constexpr`/`consteval` tables (`digitPairs`, `hexDigits`, `pow10`), traits, `static_assert`, sizes and masks derived from template parameters. No function-local statics with dynamic initialization (local `static constexpr` tables are fine). No `strtod`/`std::to_string` on the data path (`data/` uses dconv; `core` does not depend on it). Never recompute a compile-time value in a loop. + +## Determinism and proof +- Loops bounded by input or constant; recursion depth-limited. No hidden syscalls, locale or `errno` side effects. `JOIN_UNLIKELY` on error branches, `JOIN_LIKELY` on the expected path, `JOIN_ALWAYS_INLINE` only on measured inner functions. +- Claim a gain only after measuring in `release` (skill `benchmark`); never trade correctness for speed without an equivalence test (`roundtrip_test.cpp`, `parse_test.cpp`, stress tests). Report what, how, before/after. diff --git a/.claude/rules/tests.md b/.claude/rules/tests.md new file mode 100644 index 00000000..9bf7bcb7 --- /dev/null +++ b/.claude/rules/tests.md @@ -0,0 +1,22 @@ +--- +paths: + - "**/tests/**" +--- + +# Tests + +- GoogleTest/GMock. **One file per class**, snake_case + `_test.cpp` (`TcpAcceptor` -> `tcp_acceptor_test.cpp`); template policy variants follow the existing split (`local_bump_test.cpp`, `heap_bump_test.cpp`). +- **One test per public method**, named after it: `TEST (TcpAcceptor, accept)`. Constructors and special members: `create` (`construct` if the class has a `create` method), `move`, `copy`. Suite is the class name; with `TEST_F`, a fixture named `<Class>` or `<Class>Test` (both exist: copy the neighbouring file). All paths of a method (success, errors, boundaries, overloads) live in its single test. Extra tests for non-method concerns (`scopedLock`, benchmarks, `inheritConstruct`) are allowed as separate `TEST`s. A new, removed or renamed public method changes its test in the same change. +- Each test has a three-line `/**` / ` * @brief Test <method> method.` / ` */` block (the dominant form in the existing tests). Prefer `ASSERT_*`; `EXPECT_*` only when later lines stay meaningful. Assert moved-from and boundary states explicitly. +- Register in the module `tests/CMakeLists.txt`, like the neighbours: + +```cmake +add_executable(<name>.gtest <name>_test.cpp) +target_link_libraries(<name>.gtest ${JOIN_<MODULE>} GTest::gtest_main) +add_test(NAME <name>.gtest COMMAND <name>.gtest) +install(TARGETS <name>.gtest RUNTIME DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME}/test) +``` + +- New code paths need tests (coverage is tracked). Tests are deterministic under TSan/ASan: prefer futures/conditions over sleeps (some existing tests sleep; do not add more); no shared global state. +- Tests needing root, veth or netns use a fixture that probes the privilege and calls `GTEST_SKIP` when it is missing (e.g. `PingTest`; `NdpLinkTest` predates the rule and does not skip). +- Never weaken or delete an assertion to make a test pass. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..a8b4ca4e --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,31 @@ +{ + "attribution": { + "commit": "", + "pr": "" + }, + "permissions": { + "deny": [ + "Bash(git push --force:*)", + "Bash(git push --force-with-lease:*)", + "Bash(git push -f:*)", + "Bash(git push origin main:*)", + "Bash(git push -u origin main:*)", + "Bash(git push origin HEAD:main:*)", + "Bash(git merge:*)", + "Bash(gh pr merge:*)" + ] + }, + "hooks": { + "PostToolUse": [ + { + "matcher": "Edit|Write", + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR\"/.claude/hooks/format.sh" + } + ] + } + ] + } +} diff --git a/.claude/skills/add-gtest b/.claude/skills/add-gtest new file mode 120000 index 00000000..821cbc44 --- /dev/null +++ b/.claude/skills/add-gtest @@ -0,0 +1 @@ +../../.agents/skills/add-gtest \ No newline at end of file diff --git a/.claude/skills/benchmark b/.claude/skills/benchmark new file mode 120000 index 00000000..a6318e41 --- /dev/null +++ b/.claude/skills/benchmark @@ -0,0 +1 @@ +../../.agents/skills/benchmark \ No newline at end of file diff --git a/.claude/skills/compile b/.claude/skills/compile new file mode 120000 index 00000000..0f8e80b4 --- /dev/null +++ b/.claude/skills/compile @@ -0,0 +1 @@ +../../.agents/skills/compile \ No newline at end of file diff --git a/.claude/skills/coverage b/.claude/skills/coverage new file mode 120000 index 00000000..a6c2678a --- /dev/null +++ b/.claude/skills/coverage @@ -0,0 +1 @@ +../../.agents/skills/coverage \ No newline at end of file diff --git a/.claude/skills/create-pr b/.claude/skills/create-pr new file mode 120000 index 00000000..d5b42b72 --- /dev/null +++ b/.claude/skills/create-pr @@ -0,0 +1 @@ +../../.agents/skills/create-pr \ No newline at end of file diff --git a/.claude/skills/release b/.claude/skills/release new file mode 120000 index 00000000..14f8a38a --- /dev/null +++ b/.claude/skills/release @@ -0,0 +1 @@ +../../.agents/skills/release \ No newline at end of file diff --git a/.claude/skills/test b/.claude/skills/test new file mode 120000 index 00000000..20211d17 --- /dev/null +++ b/.claude/skills/test @@ -0,0 +1 @@ +../../.agents/skills/test \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..fccf5c14 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,35 @@ +# AGENTS.md + +**join**: modular C++20 networking runtime for Linux. Modules `core`, `crypto`, `data`, `fabric`, `services` (each `include/join/`, `src/`, `tests/`, optional `samples/`); dependencies `services` -> `fabric` -> `crypto` -> `core`, `services` -> `data` -> `core`. + +## Build and test + +Presets `gcc|clang` x `debug|release|tsan|asan`, trees in `build/<compiler>/<kind>`. `-Werror` is on. + +```bash +cmake --preset gcc-debug && cmake --build --preset gcc-debug && ctest --preset gcc-debug --output-on-failure +``` + +## Agent files + +Skills in `.agents/skills/<name>/SKILL.md`: compile, test, coverage, benchmark, add-gtest, create-pr, release; `<scratchpad>` in them is a temporary directory outside the repository. Run `clang-format` on changed C++ files at the version pinned in `.github/workflows/build.yml`. + +Changes to `AGENTS.md` and `.agents/` keep them tool-agnostic and short; `.claude/` is Claude Code only. + +## Definition of done + +- **Dev loop**: `gcc-debug`, the relevant test only, once the batch is reviewed (not after each edit); no clang, `release`, sanitizers or coverage. Renames: diff and `clang-format` only. Design still moving: code and benchmarks only. +- **Closing** (once, when the user says ready to merge), gcc and clang: scope limited to the request; tests added; `debug`, `release`, `tsan`, `asan` pass; `coverage` not degraded; `benchmark` not degraded if benchmarked code changed (`data/`, `core/` queues and memory backends); `README.md` updated if affected, else say none needed. + +Report presets run, results, coverage before/after and README status; never call a partial pass done. A missing tool or wrong version: ask before installing (`sudo` if justified), else give the command with the `! ` prefix, and report the item **not verified**. Root-only tests and system settings (the user is a sudoer): `sudo` if justified with output to a log, or the CI image (skill `test`), or ask the user to run them with `! `; then analyze the log. + +## Working agreements + +- English for code, comments, commits, PRs, docs and branches. +- Brief answers. Change only what was requested; extras as separate proposals. +- Off the data path, plain code first, measured; tricks, fast paths or new helpers only as a measured, proposed option. +- Show one diff at a time; edit only after an explicit yes covering that diff ("ok" after an either/or question is not a choice). Reading, building, measuring and scratchpad work need no approval. +- Prefer join's primitives (`Mutex`, `Condition`, `Semaphore`, `Queue`, `ThreadPool`, `Function`, `Variant`, `View`, allocators, clocks (`Monotonic`, `Rdtsc`...) and their `Timer`...) over `std::` or new dependencies; check `core/include/join/` first. +- Samples stay minimal: few lines, one `try/catch`. +- Ask before any state-changing git command (except `<scratchpad>` worktrees for measurements) and each PR (skill `create-pr`); read-only ones are fine. Never force-push, push to `main` or merge. +- Never edit `build/` or the generated `core/include/join/version.hpp`. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..43c994c2 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +@AGENTS.md