feat(freeze): no authors in the config, no author check
A repository that takes commits from people it cannot list has no way to freeze a range today: a null `authors` falls back to `git config user.email`, so every foreign author or committer fails. Null, or no key, now means neither is checked; the freeze warns that it did not check them and records `authors` as null. A list is checked as before. An empty list, which no commit could pass, is a config error that points at null. Released as 1.1.0.
This commit is contained in:
parent
0d3d8ed2d2
commit
c6b4d78245
GPG key ID:
2937E59CDCC1BCFB
10 changed files with 52 additions and 49 deletions
|
|
@ -41,7 +41,7 @@ crafter-review config # edit .review-gates/config.json until it prints no
|
|||
|
||||
`.review-gates/config.json` is committed. It says:
|
||||
|
||||
- what `freeze` checks: `authors`, `aiTrailers`, `requireSigned`, and the branch a range is measured against (`base`, `rangeFrom`);
|
||||
- what `freeze` checks: `authors` (null: not checked), `aiTrailers`, `requireSigned`, and the branch a range is measured against (`base`, `rangeFrom`);
|
||||
- `modes.<name>`: the kind of artifact, the lenses it runs and their effort. Every lens is off until a mode lists it; `run --mode` picks one;
|
||||
- `lenses.<name>`: the environment a lens gets (`none`, `browser`, `supabase` or `browser+supabase`) and, if the repository has one, its `section` - a file of this repository's own appended to the lens's charter;
|
||||
- `functional`, `slots`, `supabase`: how the application is started and reached, one stack or several, and where its database answers;
|
||||
|
|
|
|||
|
|
@ -9,7 +9,6 @@ import :Core;
|
|||
import :Plan;
|
||||
import :Tools;
|
||||
import :PgWire;
|
||||
import :Process;
|
||||
import :Platform;
|
||||
import :Prompt;
|
||||
import std;
|
||||
|
|
@ -151,8 +150,8 @@ namespace Crafter::Review::Config {
|
|||
// when HEAD tracks nothing; base: always merge-base HEAD
|
||||
// <base>..HEAD, the whole pull request.
|
||||
{"rangeFrom", "upstream"},
|
||||
// Author and committer emails the freeze accepts; null: the
|
||||
// repository's `git config user.email`.
|
||||
// Author and committer emails the freeze accepts; null: neither
|
||||
// is checked.
|
||||
{"authors", nullptr},
|
||||
// forbid | allow: Assisted-by / Co-Authored-By trailers in the
|
||||
// range.
|
||||
|
|
@ -266,6 +265,8 @@ namespace Crafter::Review::Config {
|
|||
return a.is_string() && a.get_ref<const std::string&>().contains('@');
|
||||
}))) {
|
||||
out.emplace_back("authors must be null or a list of email addresses");
|
||||
} else if (authors.is_array() && authors.empty()) {
|
||||
out.emplace_back("authors is an empty list, which no commit passes - null leaves author and committer unchecked");
|
||||
}
|
||||
for (const std::string_view key : {"allowDirty", "gate0"}) {
|
||||
if (!AllStrings(Dyn::At(config, key))) out.push_back(std::format("{} must be a list of strings", key));
|
||||
|
|
@ -453,22 +454,13 @@ namespace Crafter::Review::Config {
|
|||
return copy;
|
||||
}
|
||||
|
||||
std::string GitUserEmail(const fs::path& repo) {
|
||||
const Process::Captured run = Process::Run({"git", "-C", repo.string(), "config", "user.email"});
|
||||
return run.returnCode == 0 ? Dyn::Strip(Dyn::UniversalNewlines(Dyn::DecodeReplace(run.out))) : std::string();
|
||||
}
|
||||
|
||||
std::vector<std::string> Authors(const Json& config, const fs::path& repo) {
|
||||
std::optional<std::vector<std::string>> Authors(const Json& config) {
|
||||
const Json& authors = Dyn::Get(config, "authors");
|
||||
if (Dyn::Truthy(authors)) {
|
||||
if (!authors.is_array()) return std::nullopt;
|
||||
std::vector<std::string> out;
|
||||
for (const Json& author : authors) out.push_back(Dyn::Str(author));
|
||||
return out;
|
||||
}
|
||||
const std::string email = GitUserEmail(repo);
|
||||
if (email.empty()) return {};
|
||||
return {email};
|
||||
}
|
||||
|
||||
std::map<std::string, fs::path> LensSectionsOf(const Json& config, const fs::path& repo) {
|
||||
// Only a file inside the repository, by the rule Validate refuses the
|
||||
|
|
|
|||
|
|
@ -211,20 +211,18 @@ namespace Crafter::Review::Freeze {
|
|||
fail(std::format("{} is not the toplevel of a git checkout", repo));
|
||||
return report;
|
||||
}
|
||||
std::vector<std::string> authors = options.authors;
|
||||
if (authors.empty()) authors = Config::Authors({{"authors", nullptr}}, repo);
|
||||
if (authors.empty()) {
|
||||
fail("no author identity: set `authors` in .review-gates/config.json or git config user.email");
|
||||
return report;
|
||||
}
|
||||
const std::optional<std::vector<std::string>>& authors = options.authors;
|
||||
const Base base = ResolveBase(repo, options.base, options.configBase, options.rangeFrom);
|
||||
if (!base.failure.empty()) {
|
||||
fail(base.failure);
|
||||
return report;
|
||||
}
|
||||
const std::string head = Artifact::TreeHead(repo);
|
||||
Json authorList = Json::array();
|
||||
for (const std::string& author : authors) authorList.push_back(author);
|
||||
Json authorList;
|
||||
if (authors) {
|
||||
authorList = Json::array();
|
||||
for (const std::string& author : *authors) authorList.push_back(author);
|
||||
}
|
||||
report["base"] = base.sha;
|
||||
report["baseFrom"] = base.how;
|
||||
report["upstream"] = base.upstream ? Json(*base.upstream) : Json();
|
||||
|
|
@ -261,11 +259,12 @@ namespace Crafter::Review::Freeze {
|
|||
commitList.push_back({{"sha", c.sha}, {"subject", c.subject}, {"author", c.author}, {"committer", c.committer}, {"sig", c.sig}});
|
||||
}
|
||||
report["commits"] = commitList;
|
||||
const std::string authorText = Dyn::Join(authors, ", ");
|
||||
if (!authors) warn("no authors in .review-gates/config.json: author and committer not checked");
|
||||
const std::string authorText = authors ? Dyn::Join(*authors, ", ") : std::string();
|
||||
for (const Commit& c : commits) {
|
||||
const std::string who = std::format("{} {}", Head12(c.sha), Dyn::Repr(Dyn::Slice(c.subject, 0, 50)));
|
||||
if (!std::ranges::contains(authors, c.author)) fail(std::format("{}: author {} not in {}", who, c.author, authorText));
|
||||
if (!std::ranges::contains(authors, c.committer)) fail(std::format("{}: committer {} not in {}", who, c.committer, authorText));
|
||||
if (authors && !std::ranges::contains(*authors, c.author)) fail(std::format("{}: author {} not in {}", who, c.author, authorText));
|
||||
if (authors && !std::ranges::contains(*authors, c.committer)) fail(std::format("{}: committer {} not in {}", who, c.committer, authorText));
|
||||
if (const std::optional<std::string> trailer = AiTrailer(c.body)) {
|
||||
const bool forbid = options.aiTrailers == "forbid";
|
||||
const std::string message = std::format("{}: AI attribution trailer {}{}", who, Dyn::Repr(Dyn::Strip(*trailer)),
|
||||
|
|
@ -472,7 +471,7 @@ namespace Crafter::Review::Freeze {
|
|||
options.replace = arguments.replace;
|
||||
options.worktree = arguments.worktree;
|
||||
options.requireSigned = arguments.requireSigned || Dyn::Truthy(Dyn::Get(config, "requireSigned"));
|
||||
options.authors = Config::Authors(config, repo);
|
||||
options.authors = Config::Authors(config);
|
||||
options.aiTrailers = Dyn::Str(Dyn::Get(config, "aiTrailers"));
|
||||
options.allowDirty = allowDirty;
|
||||
const Json& configBase = Dyn::Get(config, "base");
|
||||
|
|
|
|||
|
|
@ -36,10 +36,9 @@ export namespace Crafter::Review::Config {
|
|||
// The config for a dump: the database URL with its password masked, no
|
||||
// `_` bookkeeping keys.
|
||||
Json Redacted(const Json& config);
|
||||
// The emails the freeze accepts: the configured list, else the
|
||||
// repository's git identity.
|
||||
std::vector<std::string> Authors(const Json& config, const std::filesystem::path& repo);
|
||||
std::string GitUserEmail(const std::filesystem::path& repo);
|
||||
// The emails the freeze accepts as author and committer: the configured
|
||||
// list; none (null, or no `authors` key) - the freeze checks neither.
|
||||
std::optional<std::vector<std::string>> Authors(const Json& config);
|
||||
// The lens files no mode lists (what runs nowhere: the slots and init).
|
||||
std::vector<std::string> LensesOff(const Json& config);
|
||||
// {lens file: environment} for every lens of the table, as the config
|
||||
|
|
|
|||
|
|
@ -56,7 +56,8 @@ export namespace Crafter::Review::Freeze {
|
|||
bool replace = false;
|
||||
bool worktree = false;
|
||||
bool requireSigned = false;
|
||||
std::vector<std::string> authors;
|
||||
// The emails author and committer must be among; none: not checked.
|
||||
std::optional<std::vector<std::string>> authors;
|
||||
std::string aiTrailers = "forbid";
|
||||
std::vector<std::string> allowDirty;
|
||||
std::optional<std::string> configBase;
|
||||
|
|
|
|||
|
|
@ -27,6 +27,14 @@
|
|||
</provides>
|
||||
<content_rating type="oars-1.1"/>
|
||||
<releases>
|
||||
<release version="1.1.0" date="2026-11-02">
|
||||
<description>
|
||||
<p>New</p>
|
||||
<ul>
|
||||
<li>Leaving <code>authors</code> out of the config turns the freeze's author and committer check off, where it used to check <code>git config user.email</code>. An empty list is a config error.</li>
|
||||
</ul>
|
||||
</description>
|
||||
</release>
|
||||
<release version="1.0.1" date="2026-11-02">
|
||||
<description>
|
||||
<p>Fixes</p>
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ delta `@{upstream}..HEAD`, the rule for a repository that pushes to its
|
|||
own trunk; `base`: always the merge-base with `base`, the whole pull request
|
||||
for a repository whose branches become PRs — their commits must be linear,
|
||||
rebased, never a merge of the integration branch), `authors` (the emails the
|
||||
freeze accepts; null = `git config user.email`), `aiTrailers` (`forbid` or
|
||||
freeze accepts; null = neither author nor committer is checked), `aiTrailers` (`forbid` or
|
||||
`allow`), `requireSigned`, `allowDirty` (tracked
|
||||
files the local stack rewrites in place while the lenses run), `gate0` (the
|
||||
commands Gate 0 runs by hand, for the skill), `modes` (`modes.<name>`: the
|
||||
|
|
@ -209,8 +209,9 @@ default), `notes`.
|
|||
output where a commit message's "tested" claim can point at it
|
||||
(`--context-paths`).
|
||||
2. **Freeze** — `freeze` checks, all of them, before it
|
||||
exports anything: author AND committer are in `authors`; AI attribution
|
||||
trailers (`Assisted-by:`, `Co-Authored-By:`) by the `aiTrailers` rule; the
|
||||
exports anything: author AND committer are in `authors` (when it is
|
||||
set); AI attribution trailers (`Assisted-by:`, `Co-Authored-By:`) by the
|
||||
`aiTrailers` rule; the
|
||||
tracked tree is clean (the `allowDirty` paths aside, warned); no merge
|
||||
commit in the range (`git format-patch` drops them silently, so the
|
||||
artifact would not be the push); the range is a fast-forward of
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
name: prepush
|
||||
description: Pre-push gates for a repository set up for crafter-review (`crafter-review init`). Runs the pipeline's procedure end to end — Gate 0 build/tests + freeze (identity, trailers by the config's rule, clean tree, no merges, fast-forward), the application and the database under test brought up from the frozen tree, the driver run of the lenses the config turns on - every lens is off until it does (repo-better-solution → repo-functional with the Playwright MCP → repo-bug-prover and repo-security side by side → repo-facts, the commit truth) via crafter-review, the fix/rerun loop, the owner's approval of the hash, verify, push. Nothing is pushed unreviewed or unapproved.
|
||||
description: Pre-push gates for a repository set up for crafter-review (`crafter-review init`). Runs the pipeline's procedure end to end — Gate 0 build/tests + freeze (identity when `authors` is set, trailers by the config's rule, clean tree, no merges, fast-forward), the application and the database under test brought up from the frozen tree, the driver run of the lenses the config turns on - every lens is off until it does (repo-better-solution → repo-functional with the Playwright MCP → repo-bug-prover and repo-security side by side → repo-facts, the commit truth) via crafter-review, the fix/rerun loop, the owner's approval of the hash, verify, push. Nothing is pushed unreviewed or unapproved.
|
||||
---
|
||||
|
||||
Follow the pipeline's procedure, `prepush.md`, EXACTLY — read it now, in
|
||||
|
|
|
|||
|
|
@ -233,12 +233,11 @@ int main() {
|
|||
&& ex.at("modes").at("commits").at("lenses").size() == 5 && ex.at("modes").at("series").at("lenses").size() == 10 && ex.at("modes").at("reply").at("lenses").size() == 5
|
||||
&& ex.at("modes").at("commits").at("effort") == "xhigh" && ex.at("modes").at("series").at("effort") == "max" && ex.at("modes").at("reply").at("effort") == "max"
|
||||
&& std::ranges::none_of(ex.at("lenses"), [](const Json& entry) { return entry.contains("run"); }), Show(Config::Validate(ex)));
|
||||
const fs::path repo = td / "r";
|
||||
fs::create_directories(repo);
|
||||
Process::Run({"git", "-C", repo.string(), "init", "-q"});
|
||||
Process::Run({"git", "-C", repo.string(), "config", "user.email", "me@example.com"});
|
||||
Check("authors: the configured list, else git config user.email", Config::Authors({{"authors", {"a@b.c"}}}, repo) == std::vector<std::string>{"a@b.c"}
|
||||
&& Config::Authors({{"authors", nullptr}}, repo) == std::vector<std::string>{"me@example.com"});
|
||||
Check("authors: the configured list; null or no key - none, the freeze checks neither author nor committer (2026-11-02, not the git identity)",
|
||||
Config::Authors({{"authors", {"a@b.c"}}}) == std::vector<std::string>{"a@b.c"} && !Config::Authors({{"authors", nullptr}}) && !Config::Authors(Json::object())
|
||||
&& !Config::Authors(Config::Defaults()));
|
||||
problems = bad({{"authors", Json::array()}});
|
||||
Check("an empty authors list is the problem that says null turns the check off", problems.size() == 1 && problems[0].contains("empty list") && problems[0].contains("null"), Show(problems));
|
||||
std::ofstream(td / "u.json", std::ios::binary) << "{}";
|
||||
Check("login_fixture: the first candidate that exists", Config::LoginFixture({{"functional", {{"login", {{"fixtures", {"nope.json", "u.json"}}}}}}}, td) == "u.json"
|
||||
&& !Config::LoginFixture({{"functional", {{"login", {{"fixtures", Json::array()}}}}}}, td));
|
||||
|
|
|
|||
|
|
@ -79,14 +79,18 @@ int main() {
|
|||
if (options.date.empty()) options.date = "2026-10-31";
|
||||
return Freeze::Run(repo, out, options);
|
||||
};
|
||||
Json r = freeze({.base = base});
|
||||
Json r = freeze({.base = base, .authors = std::vector<std::string> {me}});
|
||||
std::string failures = Joined(r.at("failures"));
|
||||
Check("trailer (forbid) + foreign author both named, nothing exported; the author list came from git config", !r.at("ok").get<bool>() && failures.contains("Co-Authored-By")
|
||||
Check("trailer (forbid) + foreign author both named, nothing exported", !r.at("ok").get<bool>() && failures.contains("Co-Authored-By")
|
||||
&& failures.contains("catbot@bot.local") && !fs::exists(out) && r.at("authors") == Json::array({me}), failures);
|
||||
r = freeze({.base = base, .aiTrailers = "allow"});
|
||||
r = freeze({.base = base, .authors = std::vector<std::string> {me}, .aiTrailers = "allow"});
|
||||
Check("trailer (allow): a warning, the foreign author still fails", !r.at("ok").get<bool>() && Joined(r.at("failures")).contains("catbot")
|
||||
&& !Joined(r.at("failures")).contains("Co-Authored-By") && AnyContains(r.at("warnings"), "Co-Authored-By"));
|
||||
r = freeze({.base = base, .authors = {me, "catbot@bot.local"}, .aiTrailers = "allow"});
|
||||
r = freeze({.base = base});
|
||||
Check("no authors (null in the config): author and committer unchecked, said in a warning - only the trailer fails", !r.at("ok").get<bool>()
|
||||
&& Joined(r.at("failures")).contains("Co-Authored-By") && !Joined(r.at("failures")).contains("catbot") && r.at("authors").is_null()
|
||||
&& AnyContains(r.at("warnings"), "author and committer not checked"), Joined(r.at("failures")));
|
||||
r = freeze({.base = base, .authors = std::vector<std::string> {me, "catbot@bot.local"}, .aiTrailers = "allow"});
|
||||
Check("both emails in authors + allow -> the range freezes (3 commits)", r.at("ok").get<bool>() && r.at("files").size() == 3, Joined(r.at("failures")));
|
||||
g({"reset", "-q", "--hard", clean});
|
||||
r = freeze({.base = base, .replace = true});
|
||||
|
|
|
|||
Loading…
Reference in a new issue