feat(review): set up Crafter.Review's pre-push review
All checks were successful
Flatpak / flatpak (push) Successful in 13m7s
All checks were successful
Flatpak / flatpak (push) Successful in 13m7s
Crafter.Review reviews a frozen commit range before it is pushed: the approach, a static proof of the code's bugs, and every claim in the messages and in the documentation the range changes, one claude process per lens and unit, or per whole-range pass, and the owner approves the exact frozen hash before the push. This sets it up for this repository. .review-gates/config.json turns on three lenses in the commits mode, at xhigh: repo-better-solution, repo-bug-prover and repo-facts. The functional lens is off: it exercises a web application through a browser, and this repository's product is a command-line tool. The security lens is off by the owner's ruling: crafter-build stores no secret of its own, and while its Flatpak runs with the user's SSH agent (--socket=ssh-auth) and the whole host filesystem (--filesystem=host), everything it runs is code its user chose to run. The rules CI keeps around its token and signing key go to the approach lens's section instead. gate0 lists the commands run by hand before a freeze. build.sh comes first, because bin/crafter-build compiles project.cpp against the interfaces it was bootstrapped with, and lint comes after the self-host build, because lint's AST rules parse with the module PCMs a build leaves and lint builds only the std PCM itself. .review-gates/lenses/ holds this repository's section of each lens's charter, appended to the text Crafter.Review ships: that AGENTS.md is absent from the frozen tree, the layers and the consumers of interfaces/, the execution contexts a bug can hide in, where libc++ and clang are readable from the review's sandbox, and what a "passes the suite" claim traces to. The /prepush skill that crafter-review init writes stays untracked. It is generated from the installed Crafter.Review and refreshed by every init, so a committed copy would lag the binary it describes. One init per checkout restores it, which AGENTS.template.md now says, together with the rules freeze holds every commit of a range to: signed (requireSigned), with the owner's address as its author and committer (authors), and no AI attribution trailer (aiTrailers forbid).
This commit is contained in:
parent
a3fe1bdda7
commit
e2bd0b14ee
6 changed files with 139 additions and 0 deletions
2
.gitignore
vendored
2
.gitignore
vendored
|
|
@ -7,3 +7,5 @@ share/crafter-build/
|
|||
packaging/flatpak/ci-manifest.yaml
|
||||
# written by every build and test run from AGENTS.template.md
|
||||
/AGENTS.md
|
||||
.review-gates/push/
|
||||
.review-gates/trees/
|
||||
|
|
|
|||
28
.review-gates/config.json
Normal file
28
.review-gates/config.json
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
{
|
||||
"base": "origin/master",
|
||||
"rangeFrom": "upstream",
|
||||
"authors": [
|
||||
"jorijnvdgraaf@catcrafts.net"
|
||||
],
|
||||
"aiTrailers": "forbid",
|
||||
"requireSigned": true,
|
||||
"lensSections": ".review-gates/lenses",
|
||||
"modes": {
|
||||
"commits": {
|
||||
"kind": "commits",
|
||||
"lenses": [
|
||||
"repo-better-solution",
|
||||
"repo-bug-prover",
|
||||
"repo-facts"
|
||||
],
|
||||
"effort": "xhigh"
|
||||
}
|
||||
},
|
||||
"gate0": [
|
||||
"./build.sh",
|
||||
"./bin/crafter-build",
|
||||
"./bin/crafter-build test",
|
||||
"./bin/crafter-build format --check",
|
||||
"./bin/crafter-build lint"
|
||||
]
|
||||
}
|
||||
42
.review-gates/lenses/repo-better-solution.md
Normal file
42
.review-gates/lenses/repo-better-solution.md
Normal file
|
|
@ -0,0 +1,42 @@
|
|||
## This repository: Crafter.Build
|
||||
|
||||
A C++26 modules build system that builds itself. The stated design is
|
||||
`README.md` (the manual) and `AGENTS.template.md` (the contributor guide;
|
||||
`AGENTS.md` is written from it by a crafter-build build or test run, not
|
||||
by `build.sh`, and is not in the tree).
|
||||
Its "Design rules" and "Security considerations" are the owner's
|
||||
decisions: a range that departs from one without saying so is a finding;
|
||||
whether one should change is the owner's call, not a defect of the range.
|
||||
Among them, for CI, which holds the workflow's token and the signing key
|
||||
its runner mounts: no secret is added to the workflow, and the
|
||||
environment is never echoed in it.
|
||||
|
||||
The layers: the library (`interfaces/`, `implementations/`), which the
|
||||
executable wraps and every `project.cpp` is compiled against; this
|
||||
repository's own `project.cpp`; `build.sh`, the bootstrap that builds the
|
||||
tool without the tool; the Flatpak (`packaging/flatpak/`), which bundles
|
||||
the LLVM users build with; CI (`.forgejo/workflows/ci.yaml`). A target
|
||||
difference branches on `cfg.target` at run time, never on the host; an
|
||||
output path goes through `BinDir()`, `BuildDir()` or `PcmDir()`; whatever
|
||||
changes what gets compiled or bundled reaches `VariantId()`.
|
||||
|
||||
Search first: the partitions' own helpers (`RunBuildCommand`,
|
||||
`RunCommandChecked`, `Progress::Diagnostic`, `ApplyStandardArgs`,
|
||||
`HostTarget`, `GitProject`, `LocalProject`); the Crafter.* libraries the
|
||||
tree already uses, cloned under `~/.cache/crafter.build/external/`; libc++
|
||||
and clang as the host installs them (`/run/host/usr/include/c++/v1`,
|
||||
`/run/host/usr/lib/clang/`); git, cmake and flatpak-builder.
|
||||
|
||||
Commit shape here: a fix, its test and its README change are separate
|
||||
commits when each stands on its own, in that order, and a bug fix ships
|
||||
with a test that fails before it. That split is not a later commit
|
||||
repairing an earlier one.
|
||||
|
||||
Compatibility: the `project.cpp` of every sibling repository beside this
|
||||
checkout (`../Crafter.*/project.cpp`) and of every user compiles against
|
||||
what `interfaces/` exports and is loaded by the binary; the CLI, the
|
||||
`CRAFTER_BUILD_*` variables, the cache an older version left under
|
||||
`~/.cache/crafter.build/` and the `bin/` layout CI globs are contracts too.
|
||||
Users get a push to master through the Flatpak's `beta` branch and a tag
|
||||
through `stable`. A change an existing `project.cpp` does not survive is a
|
||||
finding unless the range documents, in the README, what users must change.
|
||||
46
.review-gates/lenses/repo-bug-prover.md
Normal file
46
.review-gates/lenses/repo-bug-prover.md
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
## This repository: Crafter.Build
|
||||
|
||||
The execution contexts to enumerate, beside the charter's:
|
||||
- `project.cpp` is compiled to a shared object and loaded into the running
|
||||
crafter-build, resolving `Crafter::` symbols from the executable
|
||||
(`--export-dynamic`); its static initialisers run at load. Its sibling
|
||||
projects (`GitProject`, `LocalProject`) load the same way into the same
|
||||
process, and its `#pragma crafter dependency` targets are built for the
|
||||
host first, on every load.
|
||||
- The clang++, clang and llvm-ar the tool spawns, and the git and cmake it
|
||||
runs, are shell commands (`popen`, `std::system` in `:Platform`), many in
|
||||
parallel; such a step fails on its exit code, never because it printed.
|
||||
Running a test or a built artifact is a shell command too (`:Test`,
|
||||
`:Clang`). A path, file name or flag that reaches one with a space or a
|
||||
shell metacharacter in it is an input to attack. Other steps run inside
|
||||
the process, among them glslang's shader compiles, on a thread of their
|
||||
own per stale shader (`:Shader`), and lint's parse through the libclang
|
||||
the linter dlopens (`:Lint`).
|
||||
- Several crafter-build processes share `~/.cache/crafter.build/` at once:
|
||||
the std and Crafter.Build PCMs per target and march (the
|
||||
`ConcurrentCacheRace` test), and the external clones with their `.lock`
|
||||
files.
|
||||
- Three builds of the tool: the bootstrap from `build.sh` (without
|
||||
`CRAFTER_BUILD_HAS_ASSET`), the self-hosted binary on the host's LLVM, and
|
||||
the Flatpak on its own LLVM under `/app`, in a sandbox whose `/usr` is the
|
||||
Freedesktop SDK's. Their toolchains can be a release apart.
|
||||
- Targets: the host triple, a sysroot (aarch64, musl), `x86_64-w64-mingw32`
|
||||
under wine, `wasm32-wasi*` under wasmtime (or wasmer for `-r`), and
|
||||
browser builds through the runtime in `wasi-runtime/`.
|
||||
- Incremental builds: a first build, a rebuild after an edit, a rebuild
|
||||
after a toolchain or flag change and a build after `clean` start from
|
||||
different states; whatever changes what gets compiled has to reach
|
||||
`VariantId()`, or a stale artifact is reused.
|
||||
- Tests: each its own subprocess, in parallel, with a 60 s timeout. A test
|
||||
that edits or touches files stages a copy under
|
||||
`fs::temp_directory_path()`; one that only builds its fixture builds it in
|
||||
place, into the fixture's own `bin/` and `build/` in the checkout.
|
||||
|
||||
Semantics to read on disk rather than recall: libc++ and clang as the host
|
||||
installs them (`/run/host/usr/include/c++/v1`,
|
||||
`/run/host/usr/share/libc++/v1/std.cppm`,
|
||||
`/run/host/usr/lib/clang/<major>/include`), the dependencies as cloned
|
||||
(`~/.cache/crafter.build/external/<Name>-<hash>/`), the vendored
|
||||
`lib/DirStackFileIncluder.h`. The Flatpak's own LLVM is not readable from
|
||||
here: a semantic that may differ between LLVM releases is unverified for
|
||||
the Flatpak build.
|
||||
18
.review-gates/lenses/repo-facts.md
Normal file
18
.review-gates/lenses/repo-facts.md
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
## This repository: Crafter.Build
|
||||
|
||||
- A claim that a tree passes `build.sh`, the self-host build, the test
|
||||
suite, `format --check` or `lint` traces to the Gate 0 log among the
|
||||
evidence paths INPUTS names: each command with its exit code and the sha
|
||||
it ran at. A claim of a run anywhere else (the Flatpak through
|
||||
`run flatpak`, CI, wine, qemu, node, a browser) traces to a capture of
|
||||
that run that names the commit, or is unverifiable.
|
||||
- A test a message names exists as a `cfg.AddTest("<Name>")` line in
|
||||
`project.cpp` and a `tests/<Name>/main.cpp` whose checks pin what the
|
||||
message says.
|
||||
- `AGENTS.md` is generated from `AGENTS.template.md` and the dependencies'
|
||||
`AGENTS.consumer.md` and is not in the tree: a claim about it is checked
|
||||
against those sources.
|
||||
- A `<release>` entry in
|
||||
`packaging/flatpak/net.catcrafts.CrafterBuild.metainfo.xml` is the
|
||||
release notes users read: every sentence in it is a claim about the code
|
||||
since the previous tag.
|
||||
|
|
@ -19,6 +19,7 @@ crafter-build is a C++26 modules build system. A project is described by a `proj
|
|||
| `lib/` | Vendored third-party headers (nlohmann/json, glslang's DirStackFileIncluder). Do not edit. |
|
||||
| `share/crafter-build/`, `bin/`, `build/`, `lib/*.a` | Generated and gitignored. |
|
||||
| `packaging/flatpak/`, `.forgejo/workflows/ci.yaml` | The Flatpak manifest with its metainfo, repo and ref files, and the Forgejo Actions CI that builds and publishes it. |
|
||||
| `.review-gates/` | The pre-push review's `config.json` and this repository's lens sections (`lenses/`), read by Crafter.Review. Its `push/` and `trees/` are generated and gitignored. |
|
||||
|
||||
Partitions and what they own:
|
||||
|
||||
|
|
@ -142,6 +143,8 @@ These were decided deliberately. Do not undo them without the maintainer's say-s
|
|||
- Separate commits for the fix, its test and the README change when each stands on its own.
|
||||
- Update the README in the same PR when user-visible behaviour changes.
|
||||
- `format`, `lint` and `test` green before pushing. CI builds the Flatpak and runs the suite inside it on PRs, publishes the `beta` branch of the Catcrafts repo on every push to `master`, and the `stable` branch plus a release on an `X.Y.Z` tag.
|
||||
- Every push of new commits goes through Crafter.Review's pre-push review first, and goes out only once the owner approves the frozen hash. Run `flatpak run net.catcrafts.CrafterReview init` once in a fresh checkout or worktree: it writes the `/prepush` skill that drives the review, and the skill stays untracked. `freeze` refuses merge commits, so the merge of a reviewed branch into `master` is pushed without a freeze of its own.
|
||||
- Every commit of a pushed range is GPG-signed, has `jorijnvdgraaf@catcrafts.net` as its author and committer, and carries no AI attribution trailer in its message, neither `Co-Authored-By:` nor `Assisted-by:`. `freeze` refuses a range with a commit that breaks any of these (`requireSigned`, `authors` and `aiTrailers` in `.review-gates/config.json`).
|
||||
- Never commit `bin/`, `build/`, `share/crafter-build/`, `lib/*.a`, `AGENTS.md` or fixture build outputs (all gitignored), and never edit the vendored files in `lib/`.
|
||||
- Keep `CRAFTER_BUILD_MARCH=x86-64-v2` in the manifest for the shipped binary: it has to run on any x86_64 machine. User projects still default to `native`.
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue