feat(run): the repository under review is the freeze record's, not the working directory's
A directory can review repositories other than itself: a workbench whose config, env file and records serve several, freezing each with --repo and --out. The run took the working directory for the repository under review: the prompt named it, the calibration was kept under it, and --trees fell back to it. The run now reads the repository from the artifact's freeze.json - the prompt names it and the calibration is kept under it - and --trees defaults to what the record says: the worktree, else the repository when it is not the working directory, else `.`, the same rule that builds the printed run line. The working directory keeps its roles: the config, the env file, the slots, the lens processes' cwd and every logged path. A damaged record stops the run at preflight.
This commit is contained in:
parent
ea4d362a10
commit
4fc15fe1c2
9 changed files with 133 additions and 14 deletions
|
|
@ -98,6 +98,7 @@ Conventions:
|
|||
- **Messages that point at a pid.** In the sandbox, the run-lock message says "wait for it or stop it (`<invocation> stop <its scratch dir>`)", and `seats` adds a line naming `stop`; natively it says "wait for it or kill it". The pid inside a sandbox ignores SIGTERM.
|
||||
- **The dump's `driver` field and the first log line** read `crafter-review <version> (prompt v1)`. Nothing reads them.
|
||||
- **`freeze.json` records `gitVersion`.** When the hashes differ, `freeze --verify`'s MISMATCH names a git version change as the likely cause (Freeze).
|
||||
- **The repository under review is the freeze record's**, not the working directory's: `Run::reviewed` is named in the prompt and keys the calibration, and `--trees` defaults to the record's worktree, else its repository when that is not the working directory, else `.` (`Freeze::ReviewedBy`, `TreeOf`). The working directory stays `Run::repo`: the config, `.env`, the slots, the lens processes' cwd and every logged path. That is how a workbench reviews other repositories with `freeze --repo` (Freeze, Runs).
|
||||
- **`config` reports an absolute `functional.mcp.command` that is no program here**, such as `/usr/bin/npx` inside the Flatpak, whose `/usr` is the Sdk's. `config` then exits 1, and a run stops at preflight (Config).
|
||||
- **JSON edge cases.**
|
||||
- `NaN` and `Infinity` are refused when read: a claude stream, opencode or seat-probe line that holds one is skipped. They are written as `null`.
|
||||
|
|
|
|||
|
|
@ -84,6 +84,10 @@ crafter-review run --artifact .review-gates/push/<branch>/ --hash <sha256> \
|
|||
|
||||
The dump with every finding, note and coverage entry is `gate-state/commits-<hash8>-run-<n>.json`.
|
||||
|
||||
### Reviewing another repository
|
||||
|
||||
A directory can review repositories other than itself: a workbench whose config, `.env` and records serve several. From it, `crafter-review freeze --repo <path> --out <dir> --worktree`, then the printed run line. The run reads the repository under review from the artifact's `freeze.json`: the prompt names it, the calibration is kept under it, and `--trees` defaults to the recorded worktree. The config, the env file and the slots are the working directory's, the lenses run in it, and `--context-paths` are relative to it.
|
||||
|
||||
To stop a run, run `crafter-review stop <its scratch dir>`, or `crafter-review stop --hash <hash8>` for every live run of a hash. Its running passes are killed, no later gate starts and the dump is written. A run takes its passes with it however it ends, SIGKILL included: each pass runs under a supervisor that kills its whole tree, the MCP server and Chromium included. `flatpak kill net.catcrafts.CrafterReview` is the hard stop.
|
||||
|
||||
| Command | What it does |
|
||||
|
|
|
|||
|
|
@ -423,6 +423,30 @@ namespace Crafter::Review::Freeze {
|
|||
Dyn::Str(Dyn::At(report, "base")), trees, Dyn::JoinPath(Dyn::JoinPath(outRel, "gate-state"), "dispositions.json"), Dyn::Str(Dyn::At(report, "date")));
|
||||
}
|
||||
|
||||
std::string TreeOf(const Json& record, const fs::path& cwd) {
|
||||
const Json& worktree = Dyn::Get(record, "worktree");
|
||||
if (Dyn::Truthy(worktree)) return Dyn::Str(worktree);
|
||||
const Json& repo = Dyn::Get(record, "repo");
|
||||
if (!Dyn::Truthy(repo)) return ".";
|
||||
std::error_code error;
|
||||
return fs::weakly_canonical(fs::path(Dyn::Str(repo)), error) == fs::weakly_canonical(cwd, error) ? std::string(".") : Dyn::Str(repo);
|
||||
}
|
||||
|
||||
Reviewed ReviewedBy(const fs::path& artifact, const fs::path& cwd) {
|
||||
const fs::path file = artifact / "freeze.json";
|
||||
std::error_code error;
|
||||
if (!fs::exists(file, error)) return {cwd, "."};
|
||||
Json record;
|
||||
try {
|
||||
record = Dyn::LoadJsonFile(file);
|
||||
} catch (const std::exception& e) {
|
||||
throw std::runtime_error(std::format("{}: not a freeze record ({})", file.string(), e.what()));
|
||||
}
|
||||
if (!record.is_object()) throw std::runtime_error(std::format("{}: not a freeze record", file.string()));
|
||||
const Json& repo = Dyn::Get(record, "repo");
|
||||
return {Dyn::Truthy(repo) ? fs::path(Dyn::AbsPath(Dyn::Str(repo))) : cwd, TreeOf(record, cwd)};
|
||||
}
|
||||
|
||||
std::int32_t Main(const Arguments& arguments, const Json& config) {
|
||||
const std::string cwd = fs::current_path().string();
|
||||
const std::string repo = Dyn::AbsPath(arguments.repo && !arguments.repo->empty() ? *arguments.repo : cwd);
|
||||
|
|
@ -467,14 +491,7 @@ namespace Crafter::Review::Freeze {
|
|||
std::println("frozen: {} commit(s) {}..{} (base from {}) -> {}/ ({})", files.size(), Head12(report.at("base").get<std::string>()), Head12(report.at("head").get<std::string>()),
|
||||
report.at("baseFrom").get<std::string>(), rel, Dyn::Join(files, ", "));
|
||||
std::println("hash: {}", report.at("hash").get<std::string>());
|
||||
std::string tree;
|
||||
if (Dyn::Truthy(Dyn::Get(report, "worktree"))) {
|
||||
tree = report.at("worktree").get<std::string>();
|
||||
} else {
|
||||
std::error_code error;
|
||||
tree = fs::weakly_canonical(repo, error) == fs::weakly_canonical(cwd, error) ? std::string(".") : report.at("repo").get<std::string>();
|
||||
}
|
||||
const std::string line = RunLine(report, rel, tree);
|
||||
const std::string line = RunLine(report, rel, TreeOf(report, cwd));
|
||||
std::ofstream(fs::path(report.at("out").get<std::string>()) / "run-line.txt", std::ios::binary) << line << "\n";
|
||||
std::println("run (from the repository root, in a detached background shell; also in {}):\n {}", Dyn::JoinPath(rel, "run-line.txt"), line);
|
||||
return 0;
|
||||
|
|
|
|||
|
|
@ -238,7 +238,7 @@ namespace Crafter::Review::Run {
|
|||
return {0, {}};
|
||||
}
|
||||
const fs::path root = scratchRoot && !scratchRoot->empty() ? *scratchRoot : fs::path(ScratchRoot(std::nullopt));
|
||||
const Memory::Calibration calib(root / "memory.json", repo, DefaultCosts());
|
||||
const Memory::Calibration calib(root / "memory.json", artifact ? Freeze::ReviewedBy(*artifact, repo).repo : repo, DefaultCosts());
|
||||
std::optional<Memory::Calibration::Cost> best;
|
||||
for (const std::string& e : appEnvs) {
|
||||
Memory::Calibration::Cost cost = calib.CostOf(e);
|
||||
|
|
@ -307,7 +307,7 @@ namespace Crafter::Review::Run {
|
|||
}
|
||||
|
||||
// ---- the Run ---------------------------------------------------------------------------
|
||||
Run::Run() : repo(fs::current_path()), t0(Platform::Now()) {
|
||||
Run::Run() : repo(fs::current_path()), reviewed(repo), t0(Platform::Now()) {
|
||||
log = [this](const std::string& message) { Message(message); };
|
||||
appReady = [](std::string_view url, std::string_view path) { return AppReady(url, path); };
|
||||
probe = [](std::string_view url) { return PgWire::Probe(url); };
|
||||
|
|
@ -483,6 +483,8 @@ namespace Crafter::Review::Run {
|
|||
}
|
||||
logFile = scratch / "log.txt";
|
||||
log(std::format("crafter-review {} prompt v{}: commits {} @ {}", Version(), PromptVersion, Dyn::RelPath(a.artifact, repo.string()), hash));
|
||||
std::error_code canonical;
|
||||
if (fs::weakly_canonical(reviewed, canonical) != fs::weakly_canonical(repo, canonical)) log(std::format("repository under review: {} (read at {})", reviewed.string(), trees.front()));
|
||||
log(std::format("config: {}", Dyn::Truthy(Dyn::Get(cfg, "_path")) ? Dyn::Str(cfg.at("_path")) : std::string("defaults (no .review-gates/config.json)")));
|
||||
for (const Json& w : Dyn::Or(Dyn::Get(cfg, "_warnings"), Dyn::EmptyList())) log(std::format("config: warning: {}", Dyn::Str(w)));
|
||||
std::vector<std::string> lensParts;
|
||||
|
|
@ -560,7 +562,7 @@ namespace Crafter::Review::Run {
|
|||
systemText = Prompt::SystemText();
|
||||
systemPaths.clear();
|
||||
systemByLens.clear();
|
||||
runLayer = Prompt::RunLayer(Dyn::RelPath(a.artifact, repo.string()), a.base, a.date, trees, hash, lines, units, contextPaths, Basename(repo));
|
||||
runLayer = Prompt::RunLayer(Dyn::RelPath(a.artifact, repo.string()), a.base, a.date, trees, hash, lines, units, contextPaths, Basename(reviewed));
|
||||
// The application under test: the login fixture, the TOTP helper.
|
||||
const Json& fcfg = cfg.at("functional");
|
||||
loginFixture = Config::LoginFixture(cfg, repo);
|
||||
|
|
@ -570,7 +572,7 @@ namespace Crafter::Review::Run {
|
|||
schemas = {{"block", Schema::SchemaJson("block")}, {"whole", Schema::SchemaJson("whole")}};
|
||||
// Memory: every pass's tree measured, its peak recorded per repository
|
||||
// and environment; the budget per gate.
|
||||
calibration = std::make_unique<Memory::Calibration>(scratchRoot / "memory.json", repo, DefaultCosts());
|
||||
calibration = std::make_unique<Memory::Calibration>(scratchRoot / "memory.json", reviewed, DefaultCosts());
|
||||
if (!a.dryRun) {
|
||||
// A pass runs under the supervisor: its tree is claude's, the
|
||||
// supervisor itself left out, so a peak measures claude's tree
|
||||
|
|
@ -1685,7 +1687,14 @@ namespace Crafter::Review::Run {
|
|||
|
||||
// ---- the run ------------------------------------------------------------------------------
|
||||
std::int32_t Run::Execute() {
|
||||
trees = CommaList(a.trees.value_or("."));
|
||||
Freeze::Reviewed frozen;
|
||||
try {
|
||||
frozen = Freeze::ReviewedBy(a.artifact, repo);
|
||||
} catch (const std::runtime_error& e) {
|
||||
Die(e.what());
|
||||
}
|
||||
reviewed = frozen.repo;
|
||||
trees = CommaList(a.trees.value_or(frozen.tree));
|
||||
contextPaths = CommaList(a.contextPaths);
|
||||
for (const std::string& c : contextPaths) {
|
||||
if (!fs::exists(c)) Die(std::format("--context-paths: {} does not exist", c));
|
||||
|
|
|
|||
|
|
@ -76,6 +76,18 @@ export namespace Crafter::Review::Freeze {
|
|||
// The exact `run` command for a freeze report: built from its fields, never
|
||||
// retyped (2026-09-29).
|
||||
std::string RunLine(const Json& report, std::string_view outRel, std::string_view trees);
|
||||
// The tree the lenses read for a frozen artifact, as its record says: the
|
||||
// worktree (relative to the directory freeze ran from), else the
|
||||
// repository when it is not `cwd`, else `.`.
|
||||
std::string TreeOf(const Json& record, const std::filesystem::path& cwd);
|
||||
// What an artifact's freeze.json says it reviews: the repository, absolute
|
||||
// (`cwd` when the artifact has no record, or its record names none), and
|
||||
// the tree (TreeOf). A record that is not JSON throws.
|
||||
struct Reviewed {
|
||||
std::filesystem::path repo;
|
||||
std::string tree;
|
||||
};
|
||||
Reviewed ReviewedBy(const std::filesystem::path& artifact, const std::filesystem::path& cwd);
|
||||
|
||||
struct Arguments {
|
||||
std::optional<std::string> repo;
|
||||
|
|
|
|||
|
|
@ -175,7 +175,14 @@ export namespace Crafter::Review::Run {
|
|||
// {lens file: environment}, in the config's lens order: the dump, the
|
||||
// resume file and the scratch dir's marker write it in that order.
|
||||
Json environments = Json::object();
|
||||
// The working directory: the config, the env file and the slots are
|
||||
// its, the lens processes run in it, and every path logged is relative
|
||||
// to it.
|
||||
std::filesystem::path repo;
|
||||
// The repository under review, from the artifact's freeze record (the
|
||||
// working directory when it has none): the prompt names it, and the
|
||||
// calibration is kept under it.
|
||||
std::filesystem::path reviewed;
|
||||
std::vector<std::string> logs;
|
||||
double t0 = 0;
|
||||
std::optional<std::filesystem::path> logFile;
|
||||
|
|
|
|||
|
|
@ -27,7 +27,9 @@ hash at Gate 5.
|
|||
- The repository at the range HEAD: the checkout itself (`--trees .`; the
|
||||
driver verifies HEAD and a clean tree before every launch) or a detached
|
||||
worktree under `.review-gates/trees/<repo>-<hash8>/` (`freeze --worktree`;
|
||||
gitignored) — content, history and blame; the state before the range via
|
||||
gitignored; `--trees` defaults to what the freeze record says, so a
|
||||
workbench that froze another repository with `--repo` reads its worktree)
|
||||
— content, history and blame; the state before the range via
|
||||
`git show <base>:<path>`.
|
||||
- `--context-paths` for the evidence dirs the commit messages' claims rest on.
|
||||
- The lens charter (`process/lenses/repo-*.md`, system text
|
||||
|
|
|
|||
|
|
@ -170,6 +170,28 @@ int main() {
|
|||
g({"merge", "-q", "--no-ff", "-m", "merge side", "side"});
|
||||
const Json r3 = freeze({.base = base, .replace = true});
|
||||
Check("merge in the range refused", !r3.at("ok").get<bool>() && AnyContains(r3.at("failures"), "merge commit"));
|
||||
// What a run reads from the record: the repository under review and the
|
||||
// tree the lenses read (a workbench freezing another repository with
|
||||
// --repo: the record names it, not the working directory).
|
||||
const Json withTree = {{"repo", repo.string()}, {"worktree", "x/y"}};
|
||||
const Json repoOnly = {{"repo", repo.string()}};
|
||||
Check("TreeOf: the record's worktree, else the repository when it is not the working directory, else `.`",
|
||||
Freeze::TreeOf(withTree, td) == "x/y" && Freeze::TreeOf(repoOnly, td) == repo.string() && Freeze::TreeOf(repoOnly, repo) == "." && Freeze::TreeOf(Json::object(), td) == ".");
|
||||
const Freeze::Reviewed byRecord = Freeze::ReviewedBy(out, td);
|
||||
const fs::path none = td / "no-record";
|
||||
fs::create_directories(none);
|
||||
const Freeze::Reviewed byNone = Freeze::ReviewedBy(none, td);
|
||||
std::ofstream(none / "freeze.json", std::ios::binary) << "{broken";
|
||||
bool damaged = false;
|
||||
try {
|
||||
Freeze::ReviewedBy(none, td);
|
||||
} catch (const std::runtime_error& e) {
|
||||
damaged = std::string(e.what()).contains("not a freeze record");
|
||||
}
|
||||
Check("ReviewedBy: the record's repository and its tree; no record: the working directory and `.`; a damaged record throws, saying so",
|
||||
fs::weakly_canonical(byRecord.repo) == fs::weakly_canonical(repo) && byRecord.tree == Dyn::AbsPath(repo.string()) && Freeze::ReviewedBy(out, repo).tree == "." && byNone.repo == td
|
||||
&& byNone.tree == "." && damaged,
|
||||
std::format("{} {} {}", byRecord.repo.string(), byRecord.tree, byNone.tree));
|
||||
g({"worktree", "remove", "--force", wt.string()});
|
||||
fs::remove_all(td);
|
||||
return Failures ? 1 : 0;
|
||||
|
|
|
|||
|
|
@ -448,6 +448,51 @@ echo "{\"type\":\"result\",\"subtype\":\"success\",\"is_error\":false,\"result\"
|
|||
std::format("exit {}, {} call(s), {}", truth.returnCode, calls() - before, dumpT.is_object() ? Text(dumpT.at("overall")) : truth.out + truth.err));
|
||||
}
|
||||
|
||||
// A workbench (2026-10-31): a directory that reviews another
|
||||
// repository - its own config and env file, the artifact wherever
|
||||
// --out says, the lenses' tree the frozen worktree. The run reads the
|
||||
// repository under review from the freeze record: the prompt names
|
||||
// it, not the working directory, and --trees defaults to the
|
||||
// recorded worktree.
|
||||
{
|
||||
const fs::path bench = base / "bench";
|
||||
fs::create_directories(bench / ".review-gates");
|
||||
Write(bench / ".review-gates" / "config.json", R"({"lenses": {"repo-bug-prover": {"run": true}}})");
|
||||
const fs::path out = bench / "journal" / "push" / "y";
|
||||
const Process::Captured benchFreeze = Process::Run({Platform::SelfExe().string(), "freeze", "--repo", repo.string(), "--out", out.string(), "--worktree", "--base", baseSha},
|
||||
bench, std::chrono::seconds(60), {}, Env);
|
||||
Json benchRecord;
|
||||
try {
|
||||
benchRecord = Dyn::LoadJsonFile(out / "freeze.json");
|
||||
} catch (const std::exception&) {
|
||||
}
|
||||
const std::string benchHash = benchRecord.is_object() ? Text(benchRecord.at("hash")) : std::string();
|
||||
const std::string benchTree = benchRecord.is_object() ? Text(Dyn::Get(benchRecord, "worktree")) : std::string();
|
||||
Check("a workbench freezes another repository: the record names it, and the worktree is under the workbench's .review-gates/trees/",
|
||||
benchFreeze.returnCode == 0 && benchRecord.is_object() && Text(benchRecord.at("repo")) == Dyn::AbsPath(repo.string())
|
||||
&& benchTree == std::format(".review-gates/trees/repo-{}", benchHash.substr(0, 8)) && fs::is_directory(bench / benchTree),
|
||||
benchFreeze.out + benchFreeze.err);
|
||||
const fs::path benchScratch = base / "bench-scratch";
|
||||
const Process::Captured benchRun = Process::Run({Platform::SelfExe().string(), "run", "--artifact", "journal/push/y", "--hash", benchHash, "--base", baseSha, "--dispositions",
|
||||
"journal/push/y/gate-state/dispositions.json", "--date", Text(freezeJson.at("date")), "--claude-bin", fake.string(), "--scratch",
|
||||
benchScratch.string(), "--seats", "local", "--no-probe", "--dry-run"},
|
||||
bench, std::chrono::seconds(60), {}, Env);
|
||||
const fs::path benchDry = benchScratch / std::format("{}-run-1-dry", benchHash.substr(0, 8));
|
||||
// The run layer is in the system prompts (system-<sha8>.md).
|
||||
std::string systems;
|
||||
std::error_code benchError;
|
||||
for (fs::directory_iterator it(benchDry, benchError), end; !benchError && it != end; it.increment(benchError)) {
|
||||
const std::string name = it->path().filename().string();
|
||||
if (name.starts_with("system-") && name.ends_with(".md")) systems += Platform::ReadFile(it->path()).value_or("");
|
||||
}
|
||||
const std::string benchLog = Platform::ReadFile(benchDry / "log.txt").value_or("");
|
||||
Check("...and a dry run from the workbench without --trees (exit 2, as every dry run) reads the recorded worktree, names the repository under review in the run layer, not the workbench, and logs it",
|
||||
benchRun.returnCode == 2 && !systems.empty()
|
||||
&& systems.contains(std::format("Repository under review (repo), checked out at the range HEAD (the last commit of the artifact): {} -", benchTree)) && !systems.contains("(bench)")
|
||||
&& benchLog.contains(std::format("repository under review: {} (read at {})", Dyn::AbsPath(repo.string()), benchTree)),
|
||||
std::format("exit {}, {} bytes of system prompt; {}{}", benchRun.returnCode, systems.size(), benchRun.out, benchRun.err));
|
||||
}
|
||||
|
||||
// A stop between two gates: the next gate's pool starts aborted and
|
||||
// runs nothing of it.
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in a new issue