Compare commits
11 changed files with 59 additions and 1511 deletions
|
|
@ -241,8 +241,8 @@ its one token, `swarm-review-classifier` with scope `write:repository`, into
|
||||||
the store at `swarm/services/review-classifier/forge/token`. A pass at start
|
the store at `swarm/services/review-classifier/forge/token`. A pass at start
|
||||||
and every five minutes re-mints when the token is missing or no longer
|
and every five minutes re-mints when the token is missing or no longer
|
||||||
matches the forge. The controller refuses an existing account by that name
|
matches the forge. The controller refuses an existing account by that name
|
||||||
with any other email. The controller posts the reviews and pushes the try-out
|
with any other email. The controller posts the reviews with that token, and
|
||||||
branches with that token, and the classifier service never sees it.
|
the classifier service never sees it.
|
||||||
|
|
||||||
**Giving a repo access is a repo admin's step, by hand.** Nothing grants
|
**Giving a repo access is a repo admin's step, by hand.** Nothing grants
|
||||||
it automatically. In the repo's settings, add `review-classifier` as a
|
it automatically. In the repo's settings, add `review-classifier` as a
|
||||||
|
|
@ -254,10 +254,7 @@ collaborator:
|
||||||
its request-changes blocks the merge where the branch protection blocks on
|
its request-changes blocks the merge where the branch protection blocks on
|
||||||
rejected reviews. Where the branch protection restricts approvals to an
|
rejected reviews. Where the branch protection restricts approvals to an
|
||||||
allowlist, the account counts only once it's on that list, and read access
|
allowlist, the account counts only once it's on that list, and read access
|
||||||
is then enough for reviews.
|
is then enough.
|
||||||
|
|
||||||
The try-out page's **open a PR** pushes a branch to the repo itself, so it
|
|
||||||
needs **write** whatever the branch protection says.
|
|
||||||
|
|
||||||
The repo also needs a rules file on its default branch (see
|
The repo also needs a rules file on its default branch (see
|
||||||
[Repo configuration](#repo-configuration)). Without one, a request posts a
|
[Repo configuration](#repo-configuration)). Without one, a request posts a
|
||||||
|
|
@ -467,49 +464,10 @@ since a run also works on a merged or closed PR. Two routes feed them:
|
||||||
|
|
||||||
Every error is problem+json with the reason in `detail`.
|
Every error is problem+json with the reason in `detail`.
|
||||||
|
|
||||||
When it's ready, **open a PR** (`POST /api/review-classifier/open-pr`, with
|
When it's ready, **open a PR** has the service account push a branch that
|
||||||
the repo and the classifications) has the service account push a branch that
|
adds the classification to the repo's config, with you, the swarm UI user,
|
||||||
adds the classifications to the repo's rules, with you, the swarm UI user, as
|
as commit author. The page then links you to the forge's new-PR page for
|
||||||
commit author. The page then links you to the forge's compare page for that
|
that branch, where you log in as usual and open the PR yourself.
|
||||||
branch, where you log in as usual and open the PR yourself.
|
|
||||||
|
|
||||||
- **The branch** is `review-classifier/<you>-<8 hex digits>`, made from the
|
|
||||||
default branch's head with one commit, through the forge's contents API.
|
|
||||||
- **The file** is the rules file on the default branch, merged by name: a
|
|
||||||
classification named like an entry replaces it, a new name goes at the end,
|
|
||||||
and every other entry stays. A repo with no rules file gets
|
|
||||||
`.hyperhive/review-classifier.yml`. In a YAML file, the entries the merge
|
|
||||||
leaves alone keep their text, comments included. The merged file has to
|
|
||||||
pass the same checks the `rules` route makes.
|
|
||||||
- **The author** is the forge account named like your swarm UI login. A forge
|
|
||||||
account made by an SSO login carries the SSO username, so it's yours once
|
|
||||||
you've logged in to the forge. The commit carries its full name, or the
|
|
||||||
login when it has none, and its noreply address,
|
|
||||||
`<login, lowercased>@noreply.<forge domain>`. It's the address the forge
|
|
||||||
itself uses for an account that keeps its email private, so the commit
|
|
||||||
links to your account and your email stays out of the repo's history. The
|
|
||||||
committer is the service account.
|
|
||||||
- **The token** is the service account's own, read from the store for each
|
|
||||||
push (see [The service account](#the-service-account)), so the account
|
|
||||||
needs **write** access to the repo.
|
|
||||||
|
|
||||||
The route answers **201** with `branch`, `path` (the file written) and
|
|
||||||
`compare_url`, or:
|
|
||||||
|
|
||||||
- **400** for a body the try-out POST would refuse in its repo or
|
|
||||||
classifications
|
|
||||||
- **401** without the `Remote-User` header the gateway sets
|
|
||||||
- **403** when the forge refuses the push: the service account isn't a
|
|
||||||
collaborator with write access
|
|
||||||
- **404** for a repo the forge doesn't know, or one with no commit on its
|
|
||||||
default branch
|
|
||||||
- **409** when the forge has no account named like you yet, or the rules
|
|
||||||
file changed on the default branch during the push
|
|
||||||
- **422** for a default branch holding both rules files or one the `rules`
|
|
||||||
route refuses; classifications the file already holds unchanged; or a
|
|
||||||
merge that breaks a rule, such as more than ten classifications
|
|
||||||
- **503** when this host has no forge token, the service account's token
|
|
||||||
isn't minted yet, or the forge failed or didn't answer within two minutes
|
|
||||||
|
|
||||||
## Run store and retention
|
## Run store and retention
|
||||||
|
|
||||||
|
|
@ -617,3 +575,7 @@ These get settled, and filled in here, as the work reaches them:
|
||||||
`secret/swarm/services/review-classifier/forge/token`.
|
`secret/swarm/services/review-classifier/forge/token`.
|
||||||
- **Reviewer handoff.** How a later reviewer, human or agent, learns the
|
- **Reviewer handoff.** How a later reviewer, human or agent, learns the
|
||||||
classifier passed a PR before it starts its own review.
|
classifier passed a PR before it starts its own review.
|
||||||
|
- **Try-out branch access.** The service account's access for pushing try-out
|
||||||
|
branches, beyond what a repo admin gives it for reviews.
|
||||||
|
- **Commit author for try-out PRs.** How the classifier derives the commit
|
||||||
|
author's name and email from the swarm UI login.
|
||||||
|
|
|
||||||
|
|
@ -112,21 +112,10 @@ and the head commit the run read. A failed run shows its error, for example when
|
||||||
has no API key yet.
|
has no API key yet.
|
||||||
|
|
||||||
What you typed stays after a run, so you can edit the classification and run it
|
What you typed stays after a run, so you can edit the classification and run it
|
||||||
again. The page saves no classification and no result, and a run posts nothing
|
again. The page saves no classification and no result, and posts nothing to the
|
||||||
to the forge; like every table in the UI, the result table remembers its column
|
forge; like every table in the UI, the result table remembers its column
|
||||||
filters in the browser. It shows no token counts, cache counts or cost.
|
filters in the browser. It shows no token counts, cache counts or cost.
|
||||||
|
|
||||||
When a classification answers the way you want, **open a PR** proposes the
|
|
||||||
classifications on the page to the repo. The classifier's forge account pushes
|
|
||||||
a branch that adds them to the repo's rules file, with you as the commit's
|
|
||||||
author, and the page links to the forge's page for opening the PR from that
|
|
||||||
branch, where you log in as usual. A classification named like one the file
|
|
||||||
already holds replaces it, and the file's other rules stay. It needs no PR
|
|
||||||
number. If the forge has no account of yours yet, log in to the forge once
|
|
||||||
first. If the classifier's account may not push to the repo, the page says so:
|
|
||||||
a repo admin adds it as a collaborator with write access
|
|
||||||
([the service account](review-classifier.md#the-service-account)).
|
|
||||||
|
|
||||||
### Classifier settings
|
### Classifier settings
|
||||||
|
|
||||||
Under the try-out form, **classifier settings** shows the runtime, model and
|
Under the try-out form, **classifier settings** shows the runtime, model and
|
||||||
|
|
|
||||||
|
|
@ -13,18 +13,13 @@
|
||||||
// **load the repo's rules** fills the rows from the rules on the repo's default
|
// **load the repo's rules** fills the rows from the rules on the repo's default
|
||||||
// branch (`GET /api/review-classifier/rules`), to run as they are or to edit.
|
// branch (`GET /api/review-classifier/rules`), to run as they are or to edit.
|
||||||
//
|
//
|
||||||
// **open a PR** posts the rows to `POST /api/review-classifier/open-pr`, which
|
|
||||||
// has the classifier's forge account push a branch adding them to the repo's
|
|
||||||
// rules file, with the operator as commit author, and links to the forge's page
|
|
||||||
// for opening the PR from that branch.
|
|
||||||
//
|
|
||||||
// Under the form, the classifier's own settings: its model, effort and review
|
// Under the form, the classifier's own settings: its model, effort and review
|
||||||
// concurrency (`ClassifierSettings`).
|
// concurrency (`ClassifierSettings`).
|
||||||
//
|
//
|
||||||
// The inputs stay as typed after a run, so the operator can edit and run
|
// The inputs stay as typed after a run, so the operator can edit and run
|
||||||
// again. Apart from the settings, the page keeps nothing you typed and no
|
// again. Apart from the settings, the page keeps nothing you typed and no
|
||||||
// result, and a run posts nothing to the forge; the only thing the browser
|
// result, and posts nothing to the forge; the only thing the browser remembers
|
||||||
// remembers is the result table's column filters, as for every table in the UI.
|
// is the result table's column filters, as for every table in the UI.
|
||||||
import { useEffect, useState } from "preact/hooks";
|
import { useEffect, useState } from "preact/hooks";
|
||||||
import { ApiErrorPanel } from "@hive/shared/api-error-panel.js";
|
import { ApiErrorPanel } from "@hive/shared/api-error-panel.js";
|
||||||
import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js";
|
import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js";
|
||||||
|
|
@ -37,14 +32,13 @@ import { ClassificationEditor } from "./ClassificationEditor.js";
|
||||||
import { TryOutResult } from "./TryOutResult.js";
|
import { TryOutResult } from "./TryOutResult.js";
|
||||||
import {
|
import {
|
||||||
draftFromRule,
|
draftFromRule,
|
||||||
|
draftProblem,
|
||||||
|
duplicateName,
|
||||||
hasTypedText,
|
hasTypedText,
|
||||||
prNumber,
|
prNumber,
|
||||||
repoProblem,
|
repoProblem,
|
||||||
rowsProblem,
|
|
||||||
runProblem,
|
|
||||||
toBody,
|
toBody,
|
||||||
type ClassificationDraft,
|
type ClassificationDraft,
|
||||||
type OpenPrAnswer,
|
|
||||||
type OpenPull,
|
type OpenPull,
|
||||||
type RulesAnswer,
|
type RulesAnswer,
|
||||||
type TryOutResult as Result,
|
type TryOutResult as Result,
|
||||||
|
|
@ -53,7 +47,6 @@ import "./ReviewTryOutPage.css";
|
||||||
|
|
||||||
const TRY_OUT = "/api/review-classifier/try-out";
|
const TRY_OUT = "/api/review-classifier/try-out";
|
||||||
const RULES = "/api/review-classifier/rules";
|
const RULES = "/api/review-classifier/rules";
|
||||||
const OPEN_PR = "/api/review-classifier/open-pr";
|
|
||||||
const PULL_REPOS = "/api/pulls/repos";
|
const PULL_REPOS = "/api/pulls/repos";
|
||||||
const PULLS = "/api/pulls";
|
const PULLS = "/api/pulls";
|
||||||
// How long the repo field rests before its PRs are fetched, so typing a repo
|
// How long the repo field rests before its PRs are fetched, so typing a repo
|
||||||
|
|
@ -98,12 +91,6 @@ export function ReviewTryOutPage() {
|
||||||
);
|
);
|
||||||
const [confirmLoad, setConfirmLoad] = useState(false);
|
const [confirmLoad, setConfirmLoad] = useState(false);
|
||||||
|
|
||||||
const [opening, setOpening] = useState(false);
|
|
||||||
const [opened, setOpened] = useState<
|
|
||||||
(OpenPrAnswer & { repo: string }) | null
|
|
||||||
>(null);
|
|
||||||
const [openError, setOpenError] = useState<ProblemDetails | null>(null);
|
|
||||||
|
|
||||||
// What the two fields offer to complete to. The fields stay free text, so a
|
// What the two fields offer to complete to. The fields stay free text, so a
|
||||||
// repo or a PR the lists lack, a merged one say, can still be typed.
|
// repo or a PR the lists lack, a merged one say, can still be typed.
|
||||||
const [repoChoices, setRepoChoices] = useState<string[]>([]);
|
const [repoChoices, setRepoChoices] = useState<string[]>([]);
|
||||||
|
|
@ -218,8 +205,12 @@ export function ReviewTryOutPage() {
|
||||||
};
|
};
|
||||||
}, [runId]);
|
}, [runId]);
|
||||||
|
|
||||||
const openPrProblem = rowsProblem(repo, drafts);
|
const problem =
|
||||||
const problem = runProblem(repo, pr, drafts);
|
repoProblem(repo) ??
|
||||||
|
(prNumber(pr) === null ? "a PR number is a whole number above 0" : null) ??
|
||||||
|
drafts.map(draftProblem).find((p) => p !== null) ??
|
||||||
|
duplicateName(drafts) ??
|
||||||
|
null;
|
||||||
|
|
||||||
async function run(e: Event) {
|
async function run(e: Event) {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
|
|
@ -273,34 +264,6 @@ export function ReviewTryOutPage() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Pushes the rows on a new branch of the repo and keeps the link to open
|
|
||||||
// the PR from it.
|
|
||||||
async function openPr() {
|
|
||||||
const target = repo.trim();
|
|
||||||
setOpening(true);
|
|
||||||
setOpened(null);
|
|
||||||
setOpenError(null);
|
|
||||||
try {
|
|
||||||
const res = await fetch(OPEN_PR, {
|
|
||||||
method: "POST",
|
|
||||||
headers: { "content-type": "application/json" },
|
|
||||||
body: JSON.stringify({
|
|
||||||
repo: target,
|
|
||||||
classifications: drafts.map(toBody),
|
|
||||||
}),
|
|
||||||
});
|
|
||||||
if (!res.ok) {
|
|
||||||
setOpenError(await readApiError(res));
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
setOpened({ ...((await res.json()) as OpenPrAnswer), repo: target });
|
|
||||||
} catch (err: unknown) {
|
|
||||||
setOpenError({ detail: String(err) });
|
|
||||||
} finally {
|
|
||||||
setOpening(false);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Typed rows are the operator's work, so replacing them is asked first.
|
// Typed rows are the operator's work, so replacing them is asked first.
|
||||||
function askLoadRules() {
|
function askLoadRules() {
|
||||||
if (hasTypedText(drafts)) setConfirmLoad(true);
|
if (hasTypedText(drafts)) setConfirmLoad(true);
|
||||||
|
|
@ -316,9 +279,9 @@ export function ReviewTryOutPage() {
|
||||||
<Panel title="review try-out" icon="🧪">
|
<Panel title="review try-out" icon="🧪">
|
||||||
<p>
|
<p>
|
||||||
Run a review classification against a real PR and see how the model
|
Run a review classification against a real PR and see how the model
|
||||||
answers it. A run posts nothing to the forge, and nothing is saved:
|
answers it. Nothing is posted to the forge, and nothing is saved:
|
||||||
change the classification and run it again until it answers the way
|
change the classification and run it again until it answers the way
|
||||||
you want. Then open a PR to add it to the repo's rules.
|
you want.
|
||||||
</p>
|
</p>
|
||||||
<form class="ui-tryout-form" onSubmit={(e) => void run(e)}>
|
<form class="ui-tryout-form" onSubmit={(e) => void run(e)}>
|
||||||
<div class="ui-tryout-target">
|
<div class="ui-tryout-target">
|
||||||
|
|
@ -401,30 +364,8 @@ export function ReviewTryOutPage() {
|
||||||
>
|
>
|
||||||
{running ? "running…" : "run"}
|
{running ? "running…" : "run"}
|
||||||
</Button>
|
</Button>
|
||||||
<Button
|
|
||||||
onClick={() => void openPr()}
|
|
||||||
disabled={opening || openPrProblem !== null}
|
|
||||||
>
|
|
||||||
{opening ? "pushing…" : "open a PR"}
|
|
||||||
</Button>
|
|
||||||
</div>
|
</div>
|
||||||
{problem !== null ? <p class="ui-tryout-hint">{problem}</p> : null}
|
{problem !== null ? <p class="ui-tryout-hint">{problem}</p> : null}
|
||||||
{openError ? (
|
|
||||||
<ApiErrorPanel
|
|
||||||
context="the branch for the PR could not be pushed"
|
|
||||||
problem={openError}
|
|
||||||
/>
|
|
||||||
) : null}
|
|
||||||
{opened ? (
|
|
||||||
<p class="ui-tryout-loaded">
|
|
||||||
Pushed <code>{opened.branch}</code> to {opened.repo}, changing{" "}
|
|
||||||
<code>{opened.path}</code> with you as the author.{" "}
|
|
||||||
<a href={opened.compare_url} target="_blank" rel="noopener">
|
|
||||||
Open the PR on the forge
|
|
||||||
</a>
|
|
||||||
.
|
|
||||||
</p>
|
|
||||||
) : null}
|
|
||||||
</form>
|
</form>
|
||||||
</Panel>
|
</Panel>
|
||||||
<ConfirmDialog
|
<ConfirmDialog
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,5 @@
|
||||||
// Wire shapes of the review classifier's try-out endpoints
|
// Wire shapes of the review classifier's try-out endpoints
|
||||||
// (`POST /api/review-classifier/try-out`, `GET .../try-out/{id}`,
|
// (`POST /api/review-classifier/try-out`, `GET .../try-out/{id}`), and the
|
||||||
// `POST .../open-pr`), and the
|
|
||||||
// checks the form makes before a request is worth sending. The backend stays
|
// checks the form makes before a request is worth sending. The backend stays
|
||||||
// the real gate: these only keep an obviously bad form from a round trip.
|
// the real gate: these only keep an obviously bad form from a round trip.
|
||||||
|
|
||||||
|
|
@ -35,17 +34,6 @@ export interface RulesAnswer {
|
||||||
classifications: ClassificationBody[];
|
classifications: ClassificationBody[];
|
||||||
}
|
}
|
||||||
|
|
||||||
/** The open-PR route's answer (`POST /api/review-classifier/open-pr`): the
|
|
||||||
* branch the classifications were pushed on, authored by the operator. */
|
|
||||||
export interface OpenPrAnswer {
|
|
||||||
branch: string;
|
|
||||||
/** The rules file the commit wrote. */
|
|
||||||
path: string;
|
|
||||||
/** The forge's page comparing the branch with the default branch, where the
|
|
||||||
* operator opens the PR. */
|
|
||||||
compare_url: string;
|
|
||||||
}
|
|
||||||
|
|
||||||
/** An open PR, as `GET /api/pulls?repo=…` lists it. */
|
/** An open PR, as `GET /api/pulls?repo=…` lists it. */
|
||||||
export interface OpenPull {
|
export interface OpenPull {
|
||||||
number: number;
|
number: number;
|
||||||
|
|
@ -150,35 +138,6 @@ export function duplicateName(drafts: ClassificationDraft[]): string | null {
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** What keeps the rows from going to the repo as a PR, or null: the repo, then
|
|
||||||
* each row, then a shared name. Unlike a run, it needs no PR number. */
|
|
||||||
export function rowsProblem(
|
|
||||||
repo: string,
|
|
||||||
drafts: ClassificationDraft[],
|
|
||||||
): string | null {
|
|
||||||
return (
|
|
||||||
repoProblem(repo) ??
|
|
||||||
drafts.map(draftProblem).find((p) => p !== null) ??
|
|
||||||
duplicateName(drafts) ??
|
|
||||||
null
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
/** What keeps the rows from a run, or null: everything `rowsProblem` checks,
|
|
||||||
* then the PR number. The page shows this one reason for both of its buttons,
|
|
||||||
* so whatever disables opening a PR comes before the PR number it doesn't
|
|
||||||
* need. */
|
|
||||||
export function runProblem(
|
|
||||||
repo: string,
|
|
||||||
pr: string,
|
|
||||||
drafts: ClassificationDraft[],
|
|
||||||
): string | null {
|
|
||||||
return (
|
|
||||||
rowsProblem(repo, drafts) ??
|
|
||||||
(prNumber(pr) === null ? "a PR number is a whole number above 0" : null)
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
export function toBody(d: ClassificationDraft): ClassificationBody {
|
export function toBody(d: ClassificationDraft): ClassificationBody {
|
||||||
return {
|
return {
|
||||||
name: d.name.trim(),
|
name: d.name.trim(),
|
||||||
|
|
|
||||||
|
|
@ -1,143 +0,0 @@
|
||||||
// The review try-out form's pure logic: the repo and row checks, the gate on
|
|
||||||
// opening a PR from the rows, and the request body each row becomes. Run by
|
|
||||||
// `npm test`, which Node runs with its built-in test runner and type
|
|
||||||
// stripping, so the `.ts` module is imported as it is.
|
|
||||||
import assert from "node:assert/strict";
|
|
||||||
import { test } from "node:test";
|
|
||||||
import {
|
|
||||||
draftFromRule,
|
|
||||||
draftProblem,
|
|
||||||
duplicateName,
|
|
||||||
hasTypedText,
|
|
||||||
prNumber,
|
|
||||||
repoProblem,
|
|
||||||
rowsProblem,
|
|
||||||
runProblem,
|
|
||||||
thresholdValue,
|
|
||||||
toBody,
|
|
||||||
} from "../src/pages/review-tryout/TryOutTypes.ts";
|
|
||||||
|
|
||||||
// A row ready to send, with `over` replacing any of its fields.
|
|
||||||
function row(over = {}) {
|
|
||||||
return {
|
|
||||||
key: 0,
|
|
||||||
name: "no-ads",
|
|
||||||
description: "No ads.",
|
|
||||||
polarity: "negative",
|
|
||||||
threshold: "80",
|
|
||||||
...over,
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
test("a repo is owner/name with forge-safe characters in each part", () => {
|
|
||||||
assert.equal(repoProblem("hyperhive/hyperhive"), null);
|
|
||||||
assert.equal(repoProblem(" o/r.name_x-1 "), null);
|
|
||||||
for (const bad of ["", "o", "o/", "/r", "o/r/x", "o/..", "./r", "o/r s"]) {
|
|
||||||
assert.match(repoProblem(bad), /owner\/name/, bad);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
|
|
||||||
test("a row needs a name, then a description, then a whole 0-100 threshold", () => {
|
|
||||||
assert.equal(draftProblem(row()), null);
|
|
||||||
assert.match(draftProblem(row({ name: " " })), /a name/);
|
|
||||||
assert.match(
|
|
||||||
draftProblem(row({ name: " ", description: "" })),
|
|
||||||
/a name/,
|
|
||||||
"the name is checked first",
|
|
||||||
);
|
|
||||||
assert.match(
|
|
||||||
draftProblem(row({ description: " " })),
|
|
||||||
/no-ads needs a description/,
|
|
||||||
);
|
|
||||||
assert.match(
|
|
||||||
draftProblem(row({ description: "", threshold: "x" })),
|
|
||||||
/description/,
|
|
||||||
"the description is checked before the threshold",
|
|
||||||
);
|
|
||||||
for (const bad of ["", "101", "-1", "1.5", "x"]) {
|
|
||||||
assert.match(draftProblem(row({ threshold: bad })), /threshold/, bad);
|
|
||||||
}
|
|
||||||
assert.equal(draftProblem(row({ threshold: " 100 " })), null);
|
|
||||||
assert.equal(draftProblem(row({ threshold: "0" })), null);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("two rows with one trimmed name are refused", () => {
|
|
||||||
assert.equal(duplicateName([row(), row({ name: "other" })]), null);
|
|
||||||
assert.match(
|
|
||||||
duplicateName([row(), row({ name: " no-ads " })]),
|
|
||||||
/two classifications are named no-ads/,
|
|
||||||
);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("opening a PR is gated by the repo, then each row, then a shared name", () => {
|
|
||||||
const good = [row(), row({ name: "other" })];
|
|
||||||
assert.equal(rowsProblem("o/r", good), null);
|
|
||||||
|
|
||||||
// Each check on its own.
|
|
||||||
assert.match(rowsProblem("", good), /owner\/name/);
|
|
||||||
assert.match(rowsProblem("o/r", [row(), row({ name: "" })]), /a name/);
|
|
||||||
assert.match(rowsProblem("o/r", [row(), row()]), /two classifications/);
|
|
||||||
|
|
||||||
// The order: a bad repo wins over a bad row, and a bad row over a shared
|
|
||||||
// name.
|
|
||||||
assert.match(rowsProblem("o", [row({ name: "" })]), /owner\/name/);
|
|
||||||
assert.match(
|
|
||||||
rowsProblem("o/r", [row(), row({ threshold: "x" }), row()]),
|
|
||||||
/threshold/,
|
|
||||||
);
|
|
||||||
// Rows are checked in their order.
|
|
||||||
assert.match(
|
|
||||||
rowsProblem("o/r", [row({ description: "" }), row({ name: "" })]),
|
|
||||||
/no-ads needs a description/,
|
|
||||||
);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("opening a PR needs no PR number, which a run does", () => {
|
|
||||||
assert.equal(prNumber(""), null);
|
|
||||||
assert.equal(rowsProblem("o/r", [row()]), null);
|
|
||||||
assert.equal(prNumber(" 7 "), 7);
|
|
||||||
assert.equal(prNumber("0"), null);
|
|
||||||
assert.equal(prNumber("7a"), null);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("a row becomes the request's classification", () => {
|
|
||||||
assert.deepEqual(
|
|
||||||
toBody(
|
|
||||||
row({ name: " no-ads ", description: " No ads. ", threshold: " 60 " }),
|
|
||||||
),
|
|
||||||
{
|
|
||||||
name: "no-ads",
|
|
||||||
description: " No ads. ",
|
|
||||||
polarity: "negative",
|
|
||||||
threshold: 60,
|
|
||||||
},
|
|
||||||
);
|
|
||||||
assert.equal(thresholdValue("101"), null);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("a loaded rule becomes a row that sends back the same rule", () => {
|
|
||||||
const rule = {
|
|
||||||
name: "no-ads",
|
|
||||||
description: "No ads.",
|
|
||||||
polarity: "negative",
|
|
||||||
threshold: 80,
|
|
||||||
};
|
|
||||||
const d = draftFromRule(rule, 3);
|
|
||||||
assert.deepEqual(d, { key: 3, ...rule, threshold: "80" });
|
|
||||||
assert.deepEqual(toBody(d), rule);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("only a typed name or description counts as the user's work", () => {
|
|
||||||
assert.equal(hasTypedText([row({ name: " ", description: "" })]), false);
|
|
||||||
assert.equal(hasTypedText([row({ name: "", description: "x" })]), true);
|
|
||||||
});
|
|
||||||
|
|
||||||
test("the page's one hint names what disables opening a PR before the PR number", () => {
|
|
||||||
// No PR number and a bad row: the row is what disables both buttons.
|
|
||||||
assert.match(runProblem("o/r", "", [row({ name: "" })]), /a name/);
|
|
||||||
assert.match(runProblem("o", "", [row()]), /owner\/name/);
|
|
||||||
// Rows ready but no PR number: only the run is held back.
|
|
||||||
assert.match(runProblem("o/r", "", [row()]), /PR number/);
|
|
||||||
assert.equal(rowsProblem("o/r", [row()]), null);
|
|
||||||
assert.equal(runProblem("o/r", "7", [row()]), null);
|
|
||||||
});
|
|
||||||
|
|
@ -62,9 +62,7 @@ The swarm's control plane. The swarm UI and `swarmctl` are its clients.
|
||||||
requests, and the newest 100 finished runs, one document in the
|
requests, and the newest 100 finished runs, one document in the
|
||||||
`review-classifier-runs` KV bucket, which this daemon alone writes
|
`review-classifier-runs` KV bucket, which this daemon alone writes
|
||||||
(`POST /api/review-classifier/try-out`, `GET /api/review-classifier/try-out/{id}`,
|
(`POST /api/review-classifier/try-out`, `GET /api/review-classifier/try-out/{id}`,
|
||||||
`GET /api/review-classifier/rules`, `GET /api/review-classifier/runs`), and
|
`GET /api/review-classifier/rules`, `GET /api/review-classifier/runs`) →
|
||||||
the try-out page's branch for a PR, pushed as the classifier's forge account
|
|
||||||
(`POST /api/review-classifier/open-pr`) →
|
|
||||||
[`docs/swarm/review-classifier.md`](../docs/swarm/review-classifier.md).
|
[`docs/swarm/review-classifier.md`](../docs/swarm/review-classifier.md).
|
||||||
- **Review classifier settings** — the classifier's runtime, model, effort and how
|
- **Review classifier settings** — the classifier's runtime, model, effort and how
|
||||||
many requested reviews run at once, one document in the
|
many requested reviews run at once, one document in the
|
||||||
|
|
|
||||||
|
|
@ -123,19 +123,9 @@ pub struct DefaultBranchFiles {
|
||||||
pub branch: String,
|
pub branch: String,
|
||||||
/// The commit the files were read at.
|
/// The commit the files were read at.
|
||||||
pub commit: String,
|
pub commit: String,
|
||||||
/// The repo's page on the forge, as a browser reaches it.
|
/// Each asked-for path that exists there, with its bytes, in the order
|
||||||
pub web_url: Option<url::Url>,
|
/// asked.
|
||||||
/// Each asked-for path that exists there, in the order asked.
|
pub files: Vec<(String, Vec<u8>)>,
|
||||||
pub files: Vec<BranchFile>,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// One file of [`DefaultBranchFiles`].
|
|
||||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
|
||||||
pub struct BranchFile {
|
|
||||||
pub path: String,
|
|
||||||
/// The blob's sha, which the forge asks for to replace the file.
|
|
||||||
pub sha: String,
|
|
||||||
pub bytes: Vec<u8>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// One row of [`Client::issue_report`] — an open issue plus the two facts
|
/// One row of [`Client::issue_report`] — an open issue plus the two facts
|
||||||
|
|
@ -1355,26 +1345,19 @@ impl Client {
|
||||||
let mut trees = HashMap::new();
|
let mut trees = HashMap::new();
|
||||||
let mut files = Vec::new();
|
let mut files = Vec::new();
|
||||||
for path in paths {
|
for path in paths {
|
||||||
if let Some((sha, bytes)) = self.file_at(owner, repo, &commit, path, &mut trees).await?
|
if let Some(bytes) = self.file_at(owner, repo, &commit, path, &mut trees).await? {
|
||||||
{
|
files.push(((*path).to_owned(), bytes));
|
||||||
files.push(BranchFile {
|
|
||||||
path: (*path).to_owned(),
|
|
||||||
sha,
|
|
||||||
bytes,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
Ok(Some(DefaultBranchFiles {
|
Ok(Some(DefaultBranchFiles {
|
||||||
branch,
|
branch,
|
||||||
commit,
|
commit,
|
||||||
web_url: info.html_url,
|
|
||||||
files,
|
files,
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The blob sha and bytes of the file at `path` in `owner/repo` at
|
/// The file at `path` in `owner/repo` at `commit`, or `None` when no
|
||||||
/// `commit`, or `None` when no blob is there. `trees` holds the tree
|
/// blob is there. `trees` holds the tree listings already read, by sha.
|
||||||
/// listings already read, by sha.
|
|
||||||
async fn file_at(
|
async fn file_at(
|
||||||
&self,
|
&self,
|
||||||
owner: &str,
|
owner: &str,
|
||||||
|
|
@ -1382,7 +1365,7 @@ impl Client {
|
||||||
commit: &str,
|
commit: &str,
|
||||||
path: &str,
|
path: &str,
|
||||||
trees: &mut HashMap<String, Vec<GitEntry>>,
|
trees: &mut HashMap<String, Vec<GitEntry>>,
|
||||||
) -> Result<Option<(String, Vec<u8>)>> {
|
) -> Result<Option<Vec<u8>>> {
|
||||||
let mut tree = commit.to_owned();
|
let mut tree = commit.to_owned();
|
||||||
let mut segments = path.split('/').peekable();
|
let mut segments = path.split('/').peekable();
|
||||||
while let Some(name) = segments.next() {
|
while let Some(name) = segments.next() {
|
||||||
|
|
@ -1400,10 +1383,7 @@ impl Client {
|
||||||
return Ok(None);
|
return Ok(None);
|
||||||
};
|
};
|
||||||
if kind == "blob" {
|
if kind == "blob" {
|
||||||
return self
|
return self.blob(owner, repo, &sha).await.map(Some);
|
||||||
.blob(owner, repo, &sha)
|
|
||||||
.await
|
|
||||||
.map(|bytes| Some((sha, bytes)));
|
|
||||||
}
|
}
|
||||||
tree = sha;
|
tree = sha;
|
||||||
}
|
}
|
||||||
|
|
@ -2454,11 +2434,7 @@ mod tests {
|
||||||
assert_eq!(read.commit, "def456");
|
assert_eq!(read.commit, "def456");
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
read.files,
|
read.files,
|
||||||
[BranchFile {
|
[(RULES_YML.to_owned(), b"classifications: []\n".to_vec())]
|
||||||
path: RULES_YML.to_owned(),
|
|
||||||
sha: "b1".to_owned(),
|
|
||||||
bytes: b"classifications: []\n".to_vec(),
|
|
||||||
}]
|
|
||||||
);
|
);
|
||||||
// Each tree is read once for both paths.
|
// Each tree is read once for both paths.
|
||||||
let seen = seen.lock().unwrap();
|
let seen = seen.lock().unwrap();
|
||||||
|
|
|
||||||
|
|
@ -3,14 +3,11 @@
|
||||||
//! `swarm_secret_client::review_classifier::forge_token_path`. Requesting
|
//! `swarm_secret_client::review_classifier::forge_token_path`. Requesting
|
||||||
//! this account as a reviewer starts a classifier run, and this daemon posts
|
//! this account as a reviewer starts a classifier run, and this daemon posts
|
||||||
//! the result as the account's review (`crate::review_classifier::review`).
|
//! the result as the account's review (`crate::review_classifier::review`).
|
||||||
//! This daemon also pushes the try-out page's branches as the account
|
//! The classifier itself never holds the token.
|
||||||
//! (`crate::review_classifier::open_pr`). The classifier itself never holds
|
|
||||||
//! the token.
|
|
||||||
//!
|
//!
|
||||||
//! The account gets no repo access here. A repo admin adds it to a repo by
|
//! The account gets no repo access here. A repo admin adds it to a repo by
|
||||||
//! hand: read to request it as a reviewer, write (or a place on the branch
|
//! hand: read to request it as a reviewer, write (or a place on the branch
|
||||||
//! protection's approvals whitelist) for its approval to count, and write to
|
//! protection's approvals whitelist) for its approval to count.
|
||||||
//! push a branch.
|
|
||||||
//!
|
//!
|
||||||
//! The token decision is [`super::agent_token`]'s, under this module's name
|
//! The token decision is [`super::agent_token`]'s, under this module's name
|
||||||
//! and scopes, so a stored token the forge still lists with the same last
|
//! and scopes, so a stored token the forge still lists with the same last
|
||||||
|
|
@ -20,16 +17,13 @@
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
|
|
||||||
use anyhow::{Context, Result, bail};
|
use anyhow::{Context, Result, bail};
|
||||||
use forgejo_api::structs::{
|
use forgejo_api::structs::{CreatePullReviewOptions, RepoSearchQuery};
|
||||||
ChangeFileOperation, ChangeFileOperationOperation, ChangeFilesOptions, CreatePullReviewOptions,
|
|
||||||
Identity, RepoSearchQuery,
|
|
||||||
};
|
|
||||||
use forgejo_api::{Auth, Forgejo};
|
use forgejo_api::{Auth, Forgejo};
|
||||||
use swarm_secret_client::forge;
|
use swarm_secret_client::forge;
|
||||||
use swarm_secret_client::review_classifier::forge_token_path;
|
use swarm_secret_client::review_classifier::forge_token_path;
|
||||||
|
|
||||||
use super::Client;
|
use super::Client;
|
||||||
use super::agent_token::{Decision, Observed, classify_named, is_not_found};
|
use super::agent_token::{Decision, Observed, classify_named};
|
||||||
|
|
||||||
/// The classifier's forge login: the reviewer someone requests.
|
/// The classifier's forge login: the reviewer someone requests.
|
||||||
pub const LOGIN: &str = "review-classifier";
|
pub const LOGIN: &str = "review-classifier";
|
||||||
|
|
@ -37,8 +31,8 @@ pub const LOGIN: &str = "review-classifier";
|
||||||
/// The forge's name for the account's one token.
|
/// The forge's name for the account's one token.
|
||||||
pub const TOKEN_NAME: &str = "swarm-review-classifier";
|
pub const TOKEN_NAME: &str = "swarm-review-classifier";
|
||||||
|
|
||||||
/// What the token may do: post reviews and push branches, both of which the
|
/// What the token may do: post reviews, which the forge files under the
|
||||||
/// forge files under the repository scope. Which repos is the account's access, granted by hand.
|
/// repository scope. Which repos is the account's access, granted by hand.
|
||||||
pub const TOKEN_SCOPES: &str = "write:repository";
|
pub const TOKEN_SCOPES: &str = "write:repository";
|
||||||
|
|
||||||
/// How often [`spawn`] re-checks the account and its token.
|
/// How often [`spawn`] re-checks the account and its token.
|
||||||
|
|
@ -68,29 +62,6 @@ pub struct Review {
|
||||||
pub body: String,
|
pub body: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Who a commit names as its author.
|
|
||||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
|
||||||
pub struct CommitAuthor {
|
|
||||||
pub name: String,
|
|
||||||
pub email: String,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// One file written as one commit on a new branch.
|
|
||||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
|
||||||
pub struct BranchCommit {
|
|
||||||
/// The branch the new one starts from, at its head.
|
|
||||||
pub base: String,
|
|
||||||
/// Must not exist yet: the forge refuses the commit when it does.
|
|
||||||
pub branch: String,
|
|
||||||
pub path: String,
|
|
||||||
/// The blob sha of the file on `base` it replaces; `None` creates it.
|
|
||||||
/// The forge refuses the commit when `base` no longer holds that blob.
|
|
||||||
pub sha: Option<String>,
|
|
||||||
pub content: Vec<u8>,
|
|
||||||
pub message: String,
|
|
||||||
pub author: CommitAuthor,
|
|
||||||
}
|
|
||||||
|
|
||||||
impl Client {
|
impl Client {
|
||||||
/// Make sure [`LOGIN`] exists and holds a live [`TOKEN_NAME`] token in
|
/// Make sure [`LOGIN`] exists and holds a live [`TOKEN_NAME`] token in
|
||||||
/// the store, minting one when it does not. The whole job of the
|
/// the store, minting one when it does not. The whole job of the
|
||||||
|
|
@ -150,8 +121,19 @@ impl Client {
|
||||||
commit: Option<&str>,
|
commit: Option<&str>,
|
||||||
review: Review,
|
review: Review,
|
||||||
) -> Result<Option<String>> {
|
) -> Result<Option<String>> {
|
||||||
self.as_classifier()
|
let path = forge_token_path()?;
|
||||||
.await?
|
let store = crate::store::connect()
|
||||||
|
.await
|
||||||
|
.context("logging in to the swarm secret store")?;
|
||||||
|
let Some(credential) = store
|
||||||
|
.read_optional::<forge::Credential>(&path)
|
||||||
|
.await
|
||||||
|
.with_context(|| format!("reading {path}"))?
|
||||||
|
else {
|
||||||
|
bail!("nothing at {path}: the classifier's forge token is not minted yet");
|
||||||
|
};
|
||||||
|
Forgejo::new(Auth::Token(&credential.value), self.url.clone())
|
||||||
|
.context("building a forge client with the classifier's token")?
|
||||||
.repo_create_pull_review(
|
.repo_create_pull_review(
|
||||||
owner,
|
owner,
|
||||||
repo,
|
repo,
|
||||||
|
|
@ -168,63 +150,6 @@ impl Client {
|
||||||
.map(|posted| posted.html_url.map(String::from))
|
.map(|posted| posted.html_url.map(String::from))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Make `commit` as [`LOGIN`]: one commit on a new branch, from the head
|
|
||||||
/// of its base. Reads the token from the store on every call.
|
|
||||||
///
|
|
||||||
/// # Errors
|
|
||||||
/// When the store holds no token yet, or the forge refuses the commit;
|
|
||||||
/// a [`forgejo_api::ForgejoError`] in the chain carries the forge's
|
|
||||||
/// status.
|
|
||||||
pub async fn push_classifier_branch(
|
|
||||||
&self,
|
|
||||||
owner: &str,
|
|
||||||
repo: &str,
|
|
||||||
commit: BranchCommit,
|
|
||||||
) -> Result<()> {
|
|
||||||
push_branch(&self.as_classifier().await?, owner, repo, commit).await
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The forge account `login` as a commit author: its full name, or its
|
|
||||||
/// login when it has none, and its [`noreply_address`], so the commit
|
|
||||||
/// links to the account without showing its email. `None` when the
|
|
||||||
/// forge has no such account.
|
|
||||||
///
|
|
||||||
/// # Errors
|
|
||||||
/// When the forge refuses the read, or this daemon's forge URL has no
|
|
||||||
/// host.
|
|
||||||
pub async fn commit_author(&self, login: &str) -> Result<Option<CommitAuthor>> {
|
|
||||||
let user = match self.api.call(|api| api.user_get(login).into_future()).await {
|
|
||||||
Ok(user) => user,
|
|
||||||
Err(e) if is_not_found(&e) => return Ok(None),
|
|
||||||
Err(e) => return Err(e).with_context(|| format!("read forge user {login}")),
|
|
||||||
};
|
|
||||||
let login = user.login.as_deref().unwrap_or(login);
|
|
||||||
let email = noreply_address(&self.url, login)?;
|
|
||||||
let name = user
|
|
||||||
.full_name
|
|
||||||
.map(|n| n.trim().to_owned())
|
|
||||||
.filter(|n| !n.is_empty())
|
|
||||||
.unwrap_or_else(|| login.to_owned());
|
|
||||||
Ok(Some(CommitAuthor { name, email }))
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A forge client acting as [`LOGIN`], with the token the store holds.
|
|
||||||
async fn as_classifier(&self) -> Result<Forgejo> {
|
|
||||||
let path = forge_token_path()?;
|
|
||||||
let store = crate::store::connect()
|
|
||||||
.await
|
|
||||||
.context("logging in to the swarm secret store")?;
|
|
||||||
let Some(credential) = store
|
|
||||||
.read_optional::<forge::Credential>(&path)
|
|
||||||
.await
|
|
||||||
.with_context(|| format!("reading {path}"))?
|
|
||||||
else {
|
|
||||||
bail!("nothing at {path}: the classifier's forge token is not minted yet");
|
|
||||||
};
|
|
||||||
Forgejo::new(Auth::Token(&credential.value), self.url.clone())
|
|
||||||
.context("building a forge client with the classifier's token")
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The stored token at `path` and every token the forge lists for
|
/// The stored token at `path` and every token the forge lists for
|
||||||
/// [`LOGIN`], which must exist.
|
/// [`LOGIN`], which must exist.
|
||||||
async fn read_classifier_token(
|
async fn read_classifier_token(
|
||||||
|
|
@ -300,60 +225,6 @@ impl Client {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The address the forge itself puts on a web commit by `login` when the
|
|
||||||
/// account keeps its email private: the lowercased login at
|
|
||||||
/// `NO_REPLY_ADDRESS`. The forge module leaves that at its default,
|
|
||||||
/// `noreply.<DOMAIN>`, and sets `DOMAIN` from
|
|
||||||
/// `services.hyperhive.swarm.forge.domain`, the option this daemon's forge
|
|
||||||
/// URL `forge` is built from, so the domain is that URL's host.
|
|
||||||
fn noreply_address(forge: &url::Url, login: &str) -> Result<String> {
|
|
||||||
let domain = forge
|
|
||||||
.host_str()
|
|
||||||
.with_context(|| format!("the forge URL {forge} names no host"))?;
|
|
||||||
Ok(format!("{}@noreply.{domain}", login.to_lowercase()))
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Make `commit` with `forge`'s identity as committer, through the contents
|
|
||||||
/// API.
|
|
||||||
async fn push_branch(forge: &Forgejo, owner: &str, repo: &str, commit: BranchCommit) -> Result<()> {
|
|
||||||
use base64::Engine as _;
|
|
||||||
|
|
||||||
let branch = commit.branch.clone();
|
|
||||||
forge
|
|
||||||
.repo_change_files(
|
|
||||||
owner,
|
|
||||||
repo,
|
|
||||||
ChangeFilesOptions {
|
|
||||||
author: Some(Identity {
|
|
||||||
name: Some(commit.author.name),
|
|
||||||
email: Some(commit.author.email),
|
|
||||||
}),
|
|
||||||
branch: Some(commit.base),
|
|
||||||
committer: None,
|
|
||||||
dates: None,
|
|
||||||
files: vec![ChangeFileOperation {
|
|
||||||
content: Some(
|
|
||||||
base64::engine::general_purpose::STANDARD.encode(&commit.content),
|
|
||||||
),
|
|
||||||
from_path: None,
|
|
||||||
operation: if commit.sha.is_some() {
|
|
||||||
ChangeFileOperationOperation::Update
|
|
||||||
} else {
|
|
||||||
ChangeFileOperationOperation::Create
|
|
||||||
},
|
|
||||||
path: commit.path,
|
|
||||||
sha: commit.sha,
|
|
||||||
}],
|
|
||||||
force_overwrite_new_branch: Some(false),
|
|
||||||
message: Some(commit.message),
|
|
||||||
new_branch: Some(commit.branch),
|
|
||||||
signoff: None,
|
|
||||||
},
|
|
||||||
)
|
|
||||||
.await
|
|
||||||
.with_context(|| format!("pushing {LOGIN}'s branch {branch} to {owner}/{repo}"))?;
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
/// Check the account now and every [`RECONCILE_INTERVAL`] after, and call
|
/// Check the account now and every [`RECONCILE_INTERVAL`] after, and call
|
||||||
/// `enqueue`, which inserts a `MintClassifierForgeToken` node, when it needs
|
/// `enqueue`, which inserts a `MintClassifierForgeToken` node, when it needs
|
||||||
/// a job. A pass that fails is logged and retried on the next tick.
|
/// a job. A pass that fails is logged and retried on the next tick.
|
||||||
|
|
@ -435,133 +306,6 @@ mod tests {
|
||||||
assert!(lockdown_patch(&seen, LOGIN).is_none());
|
assert!(lockdown_patch(&seen, LOGIN).is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The one request `push_branch` sends for `sha`, as a stand-in forge
|
|
||||||
/// saw it: method, path and JSON body.
|
|
||||||
async fn pushed(sha: Option<&str>) -> (String, String, serde_json::Value) {
|
|
||||||
use axum::http::{Method, StatusCode, Uri, header::CONTENT_TYPE};
|
|
||||||
|
|
||||||
let seen = Arc::new(std::sync::Mutex::new(Vec::new()));
|
|
||||||
let recorded = Arc::clone(&seen);
|
|
||||||
let app = axum::Router::new().fallback(
|
|
||||||
move |method: Method, uri: Uri, body: axum::body::Bytes| {
|
|
||||||
let recorded = Arc::clone(&recorded);
|
|
||||||
async move {
|
|
||||||
recorded.lock().unwrap().push((
|
|
||||||
method.to_string(),
|
|
||||||
uri.path().to_owned(),
|
|
||||||
serde_json::from_slice(&body).unwrap_or_default(),
|
|
||||||
));
|
|
||||||
(
|
|
||||||
StatusCode::CREATED,
|
|
||||||
[(CONTENT_TYPE, "application/json")],
|
|
||||||
r#"{"commit":{"sha":"c1"}}"#,
|
|
||||||
)
|
|
||||||
}
|
|
||||||
},
|
|
||||||
);
|
|
||||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
|
|
||||||
let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap();
|
|
||||||
tokio::spawn(async move { axum::serve(listener, app).await });
|
|
||||||
let forge = Forgejo::new(Auth::Token("stub-token"), url).unwrap();
|
|
||||||
let commit = BranchCommit {
|
|
||||||
base: "main".to_owned(),
|
|
||||||
branch: "review-classifier/alice-0a1b2c3d".to_owned(),
|
|
||||||
path: ".hyperhive/review-classifier.yml".to_owned(),
|
|
||||||
sha: sha.map(str::to_owned),
|
|
||||||
content: b"classifications: []\n".to_vec(),
|
|
||||||
message: "review classifier: add no-ads".to_owned(),
|
|
||||||
author: CommitAuthor {
|
|
||||||
name: "Alice".to_owned(),
|
|
||||||
email: "alice@example.test".to_owned(),
|
|
||||||
},
|
|
||||||
};
|
|
||||||
push_branch(&forge, "o", "r", commit).await.expect("pushed");
|
|
||||||
let mut seen = seen.lock().unwrap();
|
|
||||||
assert_eq!(seen.len(), 1, "{seen:?}");
|
|
||||||
seen.remove(0)
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn a_push_is_one_commit_on_a_new_branch_authored_by_the_operator() {
|
|
||||||
let (method, path, body) = pushed(Some("b1")).await;
|
|
||||||
assert_eq!(method, "POST");
|
|
||||||
assert_eq!(path, "/api/v1/repos/o/r/contents");
|
|
||||||
assert_eq!(body["branch"], "main");
|
|
||||||
assert_eq!(body["new_branch"], "review-classifier/alice-0a1b2c3d");
|
|
||||||
assert_eq!(body["force_overwrite_new_branch"], false);
|
|
||||||
assert_eq!(body["message"], "review classifier: add no-ads");
|
|
||||||
assert_eq!(
|
|
||||||
body["author"],
|
|
||||||
serde_json::json!({ "name": "Alice", "email": "alice@example.test" })
|
|
||||||
);
|
|
||||||
assert_eq!(body["committer"], serde_json::Value::Null);
|
|
||||||
assert_eq!(
|
|
||||||
body["files"],
|
|
||||||
serde_json::json!([{
|
|
||||||
"operation": "update",
|
|
||||||
"path": ".hyperhive/review-classifier.yml",
|
|
||||||
"sha": "b1",
|
|
||||||
// base64 of "classifications: []\n"
|
|
||||||
"content": "Y2xhc3NpZmljYXRpb25zOiBbXQo=",
|
|
||||||
"from_path": null,
|
|
||||||
}])
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn a_push_without_a_blob_sha_creates_the_file() {
|
|
||||||
let (_, _, body) = pushed(None).await;
|
|
||||||
assert_eq!(body["files"][0]["operation"], "create");
|
|
||||||
assert_eq!(body["files"][0]["sha"], serde_json::Value::Null);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn a_commit_author_is_the_accounts_name_at_its_noreply_address_or_none() {
|
|
||||||
use super::super::tests::stub_forge;
|
|
||||||
|
|
||||||
let (client, _) = stub_forge(
|
|
||||||
(404, "{}"),
|
|
||||||
(
|
|
||||||
200,
|
|
||||||
r#"{"login":"Alice","full_name":" Alice A ","email":"private@x.test"}"#,
|
|
||||||
),
|
|
||||||
(404, "{}"),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
let domain = client.url.host_str().expect("a host").to_owned();
|
|
||||||
assert_eq!(
|
|
||||||
client.commit_author("alice").await.expect("read"),
|
|
||||||
Some(CommitAuthor {
|
|
||||||
name: "Alice A".to_owned(),
|
|
||||||
email: format!("alice@noreply.{domain}"),
|
|
||||||
})
|
|
||||||
);
|
|
||||||
let (client, _) = stub_forge(
|
|
||||||
(404, "{}"),
|
|
||||||
(200, r#"{"login":"bob","full_name":"","email":""}"#),
|
|
||||||
(404, "{}"),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
let bob = client.commit_author("bob").await.expect("read");
|
|
||||||
assert_eq!(bob.map(|a| a.name).as_deref(), Some("bob"));
|
|
||||||
let (client, _) = stub_forge(
|
|
||||||
(404, "{}"),
|
|
||||||
(404, r#"{"message":"user does not exist"}"#),
|
|
||||||
(404, "{}"),
|
|
||||||
)
|
|
||||||
.await;
|
|
||||||
assert_eq!(client.commit_author("nobody").await.expect("read"), None);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn the_noreply_address_is_the_lowercased_login_at_noreply_dot_the_forge_host() {
|
|
||||||
let forge = url::Url::parse("https://Forge.Example:8443/").expect("a URL");
|
|
||||||
assert_eq!(
|
|
||||||
noreply_address(&forge, "Alice-B").expect("a host"),
|
|
||||||
"alice-b@noreply.forge.example"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn the_job_locks_down_the_classifiers_own_account() {
|
async fn the_job_locks_down_the_classifiers_own_account() {
|
||||||
use super::super::tests::{lockdown_patch, stub_forge};
|
use super::super::tests::{lockdown_patch, stub_forge};
|
||||||
|
|
|
||||||
|
|
@ -3420,7 +3420,6 @@ fn build_app(state: AppState) -> axum::Router {
|
||||||
.routes(routes!(review_classifier::post_try_out))
|
.routes(routes!(review_classifier::post_try_out))
|
||||||
.routes(routes!(review_classifier::get_try_out))
|
.routes(routes!(review_classifier::get_try_out))
|
||||||
.routes(routes!(review_classifier::get_rules))
|
.routes(routes!(review_classifier::get_rules))
|
||||||
.routes(routes!(review_classifier::open_pr::post_open_pr))
|
|
||||||
.routes(routes!(review_classifier::runs::get_runs))
|
.routes(routes!(review_classifier::runs::get_runs))
|
||||||
.routes(routes!(
|
.routes(routes!(
|
||||||
review_classifier::settings::get_settings,
|
review_classifier::settings::get_settings,
|
||||||
|
|
|
||||||
|
|
@ -2,8 +2,7 @@
|
||||||
//! `GET /api/review-classifier/try-out/{id}`: run classifications against a
|
//! `GET /api/review-classifier/try-out/{id}`: run classifications against a
|
||||||
//! PR from the swarm UI, then read the result back.
|
//! PR from the swarm UI, then read the result back.
|
||||||
//! `GET /api/review-classifier/rules`: a repo's own classifications, from
|
//! `GET /api/review-classifier/rules`: a repo's own classifications, from
|
||||||
//! the rules file on its default branch. [`open_pr`] puts classifications on
|
//! the rules file on its default branch.
|
||||||
//! a new branch of the repo, for a PR.
|
|
||||||
//!
|
//!
|
||||||
//! The POST reads the PR with this daemon's forge token, sends one request to
|
//! The POST reads the PR with this daemon's forge token, sends one request to
|
||||||
//! the review classifier on [`SUBJECT`] and answers 202 with an id at once:
|
//! the review classifier on [`SUBJECT`] and answers 202 with an id at once:
|
||||||
|
|
@ -40,7 +39,6 @@ use self::settings::{Settings, SettingsSource};
|
||||||
use crate::forge::{self, DefaultBranchFiles, PullForReview};
|
use crate::forge::{self, DefaultBranchFiles, PullForReview};
|
||||||
use crate::{AppState, error_problem};
|
use crate::{AppState, error_problem};
|
||||||
|
|
||||||
pub(crate) mod open_pr;
|
|
||||||
pub(crate) mod review;
|
pub(crate) mod review;
|
||||||
pub(crate) mod runs;
|
pub(crate) mod runs;
|
||||||
pub(crate) mod settings;
|
pub(crate) mod settings;
|
||||||
|
|
@ -498,7 +496,7 @@ async fn rules(
|
||||||
),
|
),
|
||||||
));
|
));
|
||||||
}
|
}
|
||||||
[file] => (&file.path, &file.bytes),
|
[file] => file,
|
||||||
_ => {
|
_ => {
|
||||||
return Err(error_problem(
|
return Err(error_problem(
|
||||||
StatusCode::UNPROCESSABLE_ENTITY,
|
StatusCode::UNPROCESSABLE_ENTITY,
|
||||||
|
|
@ -767,23 +765,17 @@ pub(crate) mod tests {
|
||||||
use tower::ServiceExt as _;
|
use tower::ServiceExt as _;
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
use crate::forge::BranchFile;
|
|
||||||
use crate::forge::classifier_account::{BranchCommit, CommitAuthor};
|
|
||||||
|
|
||||||
/// A forge holding one PR, `o/r` 7, with `files` on `o/r`'s default
|
/// A forge holding one PR, `o/r` 7, with `files` on `o/r`'s default
|
||||||
/// branch, `settings` stored, and a classifier answering every run with
|
/// branch, `settings` stored, and a classifier answering every run with
|
||||||
/// `reply` and keeping its request. `o/empty` has no commit, `o/down`
|
/// `reply` and keeping its request. `o/empty` has no commit, `o/down`
|
||||||
/// fails with something other than a 404, and `o/hang` never answers.
|
/// fails with something other than a 404, and `o/hang` never answers.
|
||||||
/// The forge has the accounts `alice` and `bob`, and keeps each branch
|
|
||||||
/// pushed with its repo, or refuses it with `push_refusal`.
|
|
||||||
#[derive(Clone)]
|
#[derive(Clone)]
|
||||||
pub(crate) struct Fake {
|
pub(crate) struct Fake {
|
||||||
reply: Reply,
|
reply: Reply,
|
||||||
files: Vec<(&'static str, String)>,
|
files: Vec<(&'static str, String)>,
|
||||||
pub(crate) settings: Settings,
|
pub(crate) settings: Settings,
|
||||||
pub(crate) sent: Arc<std::sync::Mutex<Vec<TryOut>>>,
|
pub(crate) sent: Arc<std::sync::Mutex<Vec<TryOut>>>,
|
||||||
pub(crate) pushed: Arc<std::sync::Mutex<Vec<(String, BranchCommit)>>>,
|
|
||||||
pub(crate) push_refusal: Option<StatusCode>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Fake {
|
impl Fake {
|
||||||
|
|
@ -793,37 +785,10 @@ pub(crate) mod tests {
|
||||||
files,
|
files,
|
||||||
settings: Settings::default(),
|
settings: Settings::default(),
|
||||||
sent: Arc::default(),
|
sent: Arc::default(),
|
||||||
pushed: Arc::default(),
|
|
||||||
push_refusal: None,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
impl open_pr::BranchSink for Fake {
|
|
||||||
async fn commit_author(&self, login: &str) -> anyhow::Result<Option<CommitAuthor>> {
|
|
||||||
let name = match login {
|
|
||||||
"alice" => "Alice",
|
|
||||||
"bob" => "bob",
|
|
||||||
_ => return Ok(None),
|
|
||||||
};
|
|
||||||
Ok(Some(CommitAuthor {
|
|
||||||
name: name.to_owned(),
|
|
||||||
email: format!("{login}@example.test"),
|
|
||||||
}))
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn push(&self, owner: &str, repo: &str, commit: BranchCommit) -> anyhow::Result<()> {
|
|
||||||
if let Some(status) = self.push_refusal {
|
|
||||||
return Err(ForgejoError::UnexpectedStatusCode(status).into());
|
|
||||||
}
|
|
||||||
self.pushed
|
|
||||||
.lock()
|
|
||||||
.expect("unpoisoned")
|
|
||||||
.push((format!("{owner}/{repo}"), commit));
|
|
||||||
Ok(())
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
impl SettingsSource for Fake {
|
impl SettingsSource for Fake {
|
||||||
async fn settings(&self) -> anyhow::Result<Settings> {
|
async fn settings(&self) -> anyhow::Result<Settings> {
|
||||||
Ok(self.settings.clone())
|
Ok(self.settings.clone())
|
||||||
|
|
@ -860,15 +825,10 @@ pub(crate) mod tests {
|
||||||
("o", "r") => Ok(Some(DefaultBranchFiles {
|
("o", "r") => Ok(Some(DefaultBranchFiles {
|
||||||
branch: "main".to_owned(),
|
branch: "main".to_owned(),
|
||||||
commit: "def456".to_owned(),
|
commit: "def456".to_owned(),
|
||||||
web_url: Some(url::Url::parse("https://forge.test/o/r").expect("a URL")),
|
|
||||||
files: paths
|
files: paths
|
||||||
.iter()
|
.iter()
|
||||||
.filter_map(|p| self.files.iter().find(|(f, _)| f == p))
|
.filter_map(|p| self.files.iter().find(|(f, _)| f == p))
|
||||||
.map(|(f, body)| BranchFile {
|
.map(|(f, body)| ((*f).to_owned(), body.clone().into_bytes()))
|
||||||
path: (*f).to_owned(),
|
|
||||||
sha: format!("sha-{f}"),
|
|
||||||
bytes: body.clone().into_bytes(),
|
|
||||||
})
|
|
||||||
.collect(),
|
.collect(),
|
||||||
})),
|
})),
|
||||||
("o", "empty") => Ok(None),
|
("o", "empty") => Ok(None),
|
||||||
|
|
|
||||||
|
|
@ -1,837 +0,0 @@
|
||||||
//! `POST /api/review-classifier/open-pr`: put classifications written on the
|
|
||||||
//! try-out page on a new branch of the repo, for the operator to open a PR
|
|
||||||
//! from.
|
|
||||||
//!
|
|
||||||
//! The classifications are merged by trimmed name into the rules file on the
|
|
||||||
//! repo's default branch: one with a name the file holds replaces that entry,
|
|
||||||
//! one with a new name goes at the end, and every other entry stays. A repo
|
|
||||||
//! with no rules file gets `.hyperhive/review-classifier.yml`. In a YAML file,
|
|
||||||
//! the entries the merge leaves alone keep their text, comments included.
|
|
||||||
//!
|
|
||||||
//! The commit is made by the classifier's forge account
|
|
||||||
//! ([`LOGIN`]) with its own token, on a branch of the repo itself, so that
|
|
||||||
//! account needs write access to the repo. Its author is the operator: the
|
|
||||||
//! `Remote-User` the gateway sets names the forge account of the same login,
|
|
||||||
//! since a forge account made by an SSO login is named after the SSO user.
|
|
||||||
//! The answer links to the forge's compare page for the branch, where the
|
|
||||||
//! operator opens the PR as themselves.
|
|
||||||
|
|
||||||
use std::fmt::Write as _;
|
|
||||||
use std::future::Future;
|
|
||||||
|
|
||||||
use axum::Json;
|
|
||||||
use axum::extract::State;
|
|
||||||
use axum::extract::rejection::JsonRejection;
|
|
||||||
use axum::http::{HeaderMap, StatusCode};
|
|
||||||
use forgejo_api::{ApiErrorKind, ForgejoError};
|
|
||||||
use problem_details::ProblemDetails;
|
|
||||||
use serde::{Deserialize, Serialize};
|
|
||||||
use swarm_queue_client::review_classifier::{Classification, Polarity};
|
|
||||||
use utoipa::ToSchema;
|
|
||||||
|
|
||||||
use super::{FileSource, RULES_PATHS, check_classifications, operator, parse_rules, split_repo};
|
|
||||||
use crate::forge::classifier_account::{BranchCommit, CommitAuthor, LOGIN};
|
|
||||||
use crate::forge::{self, BranchFile};
|
|
||||||
use crate::{AppState, error_problem};
|
|
||||||
|
|
||||||
/// Where a branch's name starts, so the forge's branch list groups them.
|
|
||||||
const BRANCH_PREFIX: &str = "review-classifier/";
|
|
||||||
|
|
||||||
/// The longest a line of a folded YAML description gets, indent included.
|
|
||||||
const YAML_WIDTH: usize = 78;
|
|
||||||
|
|
||||||
/// Classifications to propose to a repo.
|
|
||||||
#[derive(Debug, Deserialize, ToSchema)]
|
|
||||||
pub(crate) struct OpenPrRequest {
|
|
||||||
/// `owner/name`.
|
|
||||||
repo: String,
|
|
||||||
/// One to ten, each with a distinct name.
|
|
||||||
classifications: Vec<Classification>,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The branch the commit went on.
|
|
||||||
#[derive(Debug, Serialize, ToSchema)]
|
|
||||||
pub(crate) struct OpenPrBranch {
|
|
||||||
branch: String,
|
|
||||||
/// The rules file the commit wrote.
|
|
||||||
path: String,
|
|
||||||
/// The forge's page comparing `branch` with the default branch, where
|
|
||||||
/// the PR is opened.
|
|
||||||
compare_url: String,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Where the commit is made: the forge in production, a fake in the tests.
|
|
||||||
pub(crate) trait BranchSink {
|
|
||||||
/// The forge account `login` as a commit author, or `None` when the
|
|
||||||
/// forge has none.
|
|
||||||
fn commit_author(
|
|
||||||
&self,
|
|
||||||
login: &str,
|
|
||||||
) -> impl Future<Output = anyhow::Result<Option<CommitAuthor>>> + Send;
|
|
||||||
|
|
||||||
fn push(
|
|
||||||
&self,
|
|
||||||
owner: &str,
|
|
||||||
repo: &str,
|
|
||||||
commit: BranchCommit,
|
|
||||||
) -> impl Future<Output = anyhow::Result<()>> + Send;
|
|
||||||
}
|
|
||||||
|
|
||||||
impl BranchSink for forge::Client {
|
|
||||||
async fn commit_author(&self, login: &str) -> anyhow::Result<Option<CommitAuthor>> {
|
|
||||||
forge::Client::commit_author(self, login).await
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn push(&self, owner: &str, repo: &str, commit: BranchCommit) -> anyhow::Result<()> {
|
|
||||||
self.push_classifier_branch(owner, repo, commit).await
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Put classifications on a new branch, authored by the operator.
|
|
||||||
#[utoipa::path(
|
|
||||||
post,
|
|
||||||
path = "/api/review-classifier/open-pr",
|
|
||||||
request_body = OpenPrRequest,
|
|
||||||
responses(
|
|
||||||
(status = 201, description = "the branch is on the forge; open the PR at \
|
|
||||||
`compare_url`", body = OpenPrBranch),
|
|
||||||
(status = 400, description = "the body is not this shape, or fails a check the \
|
|
||||||
try-out POST makes on its repo or classifications (problem+json)", body = String),
|
|
||||||
(status = 401, description = "the request carries no `Remote-User`, so there is no \
|
|
||||||
author (problem+json)", body = String),
|
|
||||||
(status = 403, description = "the forge refused the classifier's account the push: \
|
|
||||||
it is not a collaborator with write access on the repo (problem+json)",
|
|
||||||
body = String),
|
|
||||||
(status = 404, description = "the forge knows no such repo, or its default branch \
|
|
||||||
has no commit (problem+json)", body = String),
|
|
||||||
(status = 409, description = "the forge has no account named like the operator, who \
|
|
||||||
has to log in to it once; or the default branch's rules file changed while the \
|
|
||||||
commit was made (problem+json)", body = String),
|
|
||||||
(status = 422, description = "the default branch holds both rules files, or one the \
|
|
||||||
`rules` GET refuses; the rules already hold every classification as it is; or \
|
|
||||||
the merged rules fail the try-out POST's checks, such as more than ten \
|
|
||||||
(problem+json)", body = String),
|
|
||||||
(status = 503, description = "this host has no forge token yet, or the classifier's \
|
|
||||||
account has none in the store; or the forge failed or did not answer within two \
|
|
||||||
minutes (problem+json)", body = String),
|
|
||||||
),
|
|
||||||
tag = "review-classifier"
|
|
||||||
)]
|
|
||||||
pub(crate) async fn post_open_pr(
|
|
||||||
State(state): State<AppState>,
|
|
||||||
headers: HeaderMap,
|
|
||||||
body: Result<Json<OpenPrRequest>, JsonRejection>,
|
|
||||||
) -> Result<(StatusCode, Json<OpenPrBranch>), ProblemDetails> {
|
|
||||||
let operator = operator(&headers).ok_or_else(|| {
|
|
||||||
error_problem(
|
|
||||||
StatusCode::UNAUTHORIZED,
|
|
||||||
"no Remote-User header: the gateway names the operator, and this request did not \
|
|
||||||
come through it",
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
let Json(req) = body.map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.body_text()))?;
|
|
||||||
let (owner, name) =
|
|
||||||
split_repo(&req.repo).map_err(|d| error_problem(StatusCode::BAD_REQUEST, &d))?;
|
|
||||||
check_classifications(&req.classifications)
|
|
||||||
.map_err(|d| error_problem(StatusCode::BAD_REQUEST, &d))?;
|
|
||||||
#[cfg(test)]
|
|
||||||
if let Some(fake) = state.try_outs.fake.clone() {
|
|
||||||
return open(&*fake, &operator, owner, name, req.classifications).await;
|
|
||||||
}
|
|
||||||
let Some(forge) = state.forge.get() else {
|
|
||||||
return Err(error_problem(
|
|
||||||
StatusCode::SERVICE_UNAVAILABLE,
|
|
||||||
"no forge access on this host yet",
|
|
||||||
));
|
|
||||||
};
|
|
||||||
open(forge.as_ref(), &operator, owner, name, req.classifications).await
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Resolve the author, merge the rules and push the branch.
|
|
||||||
async fn open<F: FileSource + BranchSink + Sync>(
|
|
||||||
forge: &F,
|
|
||||||
operator: &str,
|
|
||||||
owner: &str,
|
|
||||||
name: &str,
|
|
||||||
rows: Vec<Classification>,
|
|
||||||
) -> Result<(StatusCode, Json<OpenPrBranch>), ProblemDetails> {
|
|
||||||
let repo = format!("{owner}/{name}");
|
|
||||||
let author = deadline(forge.commit_author(operator))
|
|
||||||
.await?
|
|
||||||
.map_err(|e| forge_problem(&e, &repo))?
|
|
||||||
.ok_or_else(|| {
|
|
||||||
error_problem(
|
|
||||||
StatusCode::CONFLICT,
|
|
||||||
&format!(
|
|
||||||
"the forge has no account {operator}, to name as the commit's author: log in \
|
|
||||||
to the forge once, then try again"
|
|
||||||
),
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
let read = deadline(forge.default_branch_files(owner, name, &RULES_PATHS))
|
|
||||||
.await?
|
|
||||||
.map_err(|e| forge_problem(&e, &repo))?
|
|
||||||
.ok_or_else(|| {
|
|
||||||
error_problem(
|
|
||||||
StatusCode::NOT_FOUND,
|
|
||||||
&format!("{repo} has no commit on its default branch to branch from"),
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
let existing = match read.files.as_slice() {
|
|
||||||
[] => None,
|
|
||||||
[file] => Some(file),
|
|
||||||
_ => {
|
|
||||||
return Err(error_problem(
|
|
||||||
StatusCode::UNPROCESSABLE_ENTITY,
|
|
||||||
&format!(
|
|
||||||
"{repo}'s default branch {} holds both rules files: keep one",
|
|
||||||
read.branch
|
|
||||||
),
|
|
||||||
));
|
|
||||||
}
|
|
||||||
};
|
|
||||||
let merged =
|
|
||||||
merge(existing, rows).map_err(|d| error_problem(StatusCode::UNPROCESSABLE_ENTITY, &d))?;
|
|
||||||
let web_url = read.web_url.ok_or_else(|| {
|
|
||||||
error_problem(
|
|
||||||
StatusCode::SERVICE_UNAVAILABLE,
|
|
||||||
&format!("the forge named no web page for {repo}"),
|
|
||||||
)
|
|
||||||
})?;
|
|
||||||
let branch =
|
|
||||||
branch_name(operator).map_err(|e| error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e))?;
|
|
||||||
let compare_url = format!(
|
|
||||||
"{}/compare/{}...{branch}",
|
|
||||||
web_url.as_str().trim_end_matches('/'),
|
|
||||||
read.branch
|
|
||||||
);
|
|
||||||
let commit = BranchCommit {
|
|
||||||
base: read.branch,
|
|
||||||
branch: branch.clone(),
|
|
||||||
path: merged.path.clone(),
|
|
||||||
sha: merged.sha,
|
|
||||||
content: merged.content,
|
|
||||||
message: merged.message,
|
|
||||||
author,
|
|
||||||
};
|
|
||||||
deadline(forge.push(owner, name, commit))
|
|
||||||
.await?
|
|
||||||
.map_err(|e| push_problem(&e, &repo))?;
|
|
||||||
tracing::info!(%repo, %branch, %operator, path = %merged.path, "review classifier branch pushed");
|
|
||||||
Ok((
|
|
||||||
StatusCode::CREATED,
|
|
||||||
Json(OpenPrBranch {
|
|
||||||
branch,
|
|
||||||
path: merged.path,
|
|
||||||
compare_url,
|
|
||||||
}),
|
|
||||||
))
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn deadline<T>(work: impl Future<Output = T>) -> Result<T, ProblemDetails> {
|
|
||||||
forge::within_forge_deadline("review classifier branch", work)
|
|
||||||
.await
|
|
||||||
.ok_or_else(|| {
|
|
||||||
error_problem(
|
|
||||||
StatusCode::SERVICE_UNAVAILABLE,
|
|
||||||
"the forge did not answer within the deadline",
|
|
||||||
)
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
/// A fresh branch for `operator`: [`BRANCH_PREFIX`], their name in the
|
|
||||||
/// characters a ref takes without question, and eight random hex digits.
|
|
||||||
fn branch_name(operator: &str) -> Result<String, String> {
|
|
||||||
let mut bytes = [0u8; 4];
|
|
||||||
getrandom::fill(&mut bytes).map_err(|e| format!("drawing a branch name: {e}"))?;
|
|
||||||
let who: String = operator
|
|
||||||
.chars()
|
|
||||||
.map(|c| {
|
|
||||||
if c.is_ascii_alphanumeric() {
|
|
||||||
c.to_ascii_lowercase()
|
|
||||||
} else {
|
|
||||||
'-'
|
|
||||||
}
|
|
||||||
})
|
|
||||||
.collect();
|
|
||||||
let mut name = format!("{BRANCH_PREFIX}{who}-");
|
|
||||||
for b in bytes {
|
|
||||||
let _ = write!(name, "{b:02x}");
|
|
||||||
}
|
|
||||||
Ok(name)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// 404 for a repo the forge doesn't know, 503 for anything else.
|
|
||||||
fn forge_problem(e: &anyhow::Error, repo: &str) -> ProblemDetails {
|
|
||||||
if forge_status(e) == Some(StatusCode::NOT_FOUND) {
|
|
||||||
return error_problem(StatusCode::NOT_FOUND, &format!("no repo {repo}"));
|
|
||||||
}
|
|
||||||
let detail = format!("{e:#}");
|
|
||||||
tracing::warn!(error = %detail, %repo, "preparing a review classifier branch failed");
|
|
||||||
error_problem(StatusCode::SERVICE_UNAVAILABLE, &detail)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// 403 when the account may not push, 409 when the rules file or the
|
|
||||||
/// branch changed under the commit, 503 for anything else.
|
|
||||||
fn push_problem(e: &anyhow::Error, repo: &str) -> ProblemDetails {
|
|
||||||
let detail = format!("{e:#}");
|
|
||||||
tracing::warn!(error = %detail, %repo, "pushing a review classifier branch failed");
|
|
||||||
match forge_status(e) {
|
|
||||||
// The forge hides a repo the account can't read behind a 404.
|
|
||||||
Some(StatusCode::FORBIDDEN | StatusCode::NOT_FOUND) => error_problem(
|
|
||||||
StatusCode::FORBIDDEN,
|
|
||||||
&format!(
|
|
||||||
"{LOGIN} may not push to {repo}: a repo admin adds it as a collaborator with \
|
|
||||||
write access ({detail})"
|
|
||||||
),
|
|
||||||
),
|
|
||||||
Some(StatusCode::CONFLICT | StatusCode::UNPROCESSABLE_ENTITY) => error_problem(
|
|
||||||
StatusCode::CONFLICT,
|
|
||||||
&format!("the default branch changed while the branch was made: try again ({detail})"),
|
|
||||||
),
|
|
||||||
_ => error_problem(StatusCode::SERVICE_UNAVAILABLE, &detail),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The forge's status, when a forge answer is what failed.
|
|
||||||
fn forge_status(e: &anyhow::Error) -> Option<StatusCode> {
|
|
||||||
match e.downcast_ref::<ForgejoError>()? {
|
|
||||||
ForgejoError::ApiError(api) => match api.error_kind() {
|
|
||||||
ApiErrorKind::Forbidden | ApiErrorKind::Unauthorized => Some(StatusCode::FORBIDDEN),
|
|
||||||
ApiErrorKind::NotFound { .. } => Some(StatusCode::NOT_FOUND),
|
|
||||||
ApiErrorKind::ValidationFailed => Some(StatusCode::UNPROCESSABLE_ENTITY),
|
|
||||||
ApiErrorKind::Other(s) => Some(*s),
|
|
||||||
_ => None,
|
|
||||||
},
|
|
||||||
ForgejoError::UnexpectedStatusCode(s) => Some(*s),
|
|
||||||
_ => None,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// The rules file a commit writes.
|
|
||||||
#[derive(Debug)]
|
|
||||||
struct Merged {
|
|
||||||
path: String,
|
|
||||||
/// The blob it replaces; `None` when the repo has no rules file.
|
|
||||||
sha: Option<String>,
|
|
||||||
content: Vec<u8>,
|
|
||||||
message: String,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// `rows` merged into `existing`, or why they can't be.
|
|
||||||
fn merge(existing: Option<&BranchFile>, rows: Vec<Classification>) -> Result<Merged, String> {
|
|
||||||
let [yml, _] = RULES_PATHS;
|
|
||||||
let (path, sha, text, current) = match existing {
|
|
||||||
Some(file) => (
|
|
||||||
file.path.clone(),
|
|
||||||
Some(file.sha.clone()),
|
|
||||||
Some(String::from_utf8_lossy(&file.bytes).into_owned()),
|
|
||||||
parse_rules(&file.path, &file.bytes)?,
|
|
||||||
),
|
|
||||||
None => (yml.to_owned(), None, None, Vec::new()),
|
|
||||||
};
|
|
||||||
let mut merged = current.clone();
|
|
||||||
let (mut added, mut changed) = (Vec::new(), Vec::new());
|
|
||||||
for row in rows {
|
|
||||||
let row = Classification {
|
|
||||||
name: row.name.trim().to_owned(),
|
|
||||||
description: row.description.trim().to_owned(),
|
|
||||||
..row
|
|
||||||
};
|
|
||||||
match merged.iter().position(|c| c.name.trim() == row.name) {
|
|
||||||
Some(i) if merged[i] == row => {}
|
|
||||||
Some(i) => {
|
|
||||||
changed.push(row.name.clone());
|
|
||||||
merged[i] = row;
|
|
||||||
}
|
|
||||||
None => {
|
|
||||||
added.push(row.name.clone());
|
|
||||||
merged.push(row);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if added.is_empty() && changed.is_empty() {
|
|
||||||
return Err(format!(
|
|
||||||
"{path} already holds every classification as it is"
|
|
||||||
));
|
|
||||||
}
|
|
||||||
check_classifications(&merged).map_err(|e| format!("{path} with these merged: {e}"))?;
|
|
||||||
let content = if std::path::Path::new(&path)
|
|
||||||
.extension()
|
|
||||||
.is_some_and(|e| e == "json")
|
|
||||||
{
|
|
||||||
let mut json = serde_json::to_string_pretty(&serde_json::json!({
|
|
||||||
"classifications": merged,
|
|
||||||
}))
|
|
||||||
.map_err(|e| format!("encoding {path}: {e}"))?;
|
|
||||||
json.push('\n');
|
|
||||||
json
|
|
||||||
} else {
|
|
||||||
yaml(text.as_deref(), ¤t, &merged)
|
|
||||||
};
|
|
||||||
let mut message = String::from("review classifier:");
|
|
||||||
for (verb, names) in [("add", &added), ("update", &changed)] {
|
|
||||||
if !names.is_empty() {
|
|
||||||
let sep = if message.ends_with(':') { " " } else { "; " };
|
|
||||||
let _ = write!(message, "{sep}{verb} {}", names.join(", "));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Ok(Merged {
|
|
||||||
path,
|
|
||||||
sha,
|
|
||||||
content: content.into_bytes(),
|
|
||||||
message,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
/// `merged` as a YAML rules file. Each entry of `text` that `merged` holds
|
|
||||||
/// unchanged at the same place keeps its text, as long as the result reads
|
|
||||||
/// back as `merged`; otherwise the whole file is written out.
|
|
||||||
fn yaml(text: Option<&str>, current: &[Classification], merged: &[Classification]) -> String {
|
|
||||||
if let Some((head, entries, indent)) = text.and_then(entry_lines) {
|
|
||||||
let mut out = head;
|
|
||||||
for (i, c) in merged.iter().enumerate() {
|
|
||||||
match entries.get(i) {
|
|
||||||
Some(kept) if current.get(i) == Some(c) => out.push_str(kept),
|
|
||||||
_ => {
|
|
||||||
if !out.is_empty() && !out.ends_with('\n') {
|
|
||||||
out.push('\n');
|
|
||||||
}
|
|
||||||
out.push_str(&yaml_entry(c, &indent));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if parse_rules(".yml", out.as_bytes()).as_deref() == Ok(merged) {
|
|
||||||
return out;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
let mut out = String::from("classifications:\n");
|
|
||||||
for c in merged {
|
|
||||||
out.push_str(&yaml_entry(c, " "));
|
|
||||||
}
|
|
||||||
out
|
|
||||||
}
|
|
||||||
|
|
||||||
/// `text` split into what comes before the first entry of its
|
|
||||||
/// `classifications:` list, the text of each entry up to the next one, and
|
|
||||||
/// the indent of their `-`. `None` when no entry follows that key. The split
|
|
||||||
/// is only a guess at the layout: [`yaml`] keeps it when what it builds reads
|
|
||||||
/// back as it should.
|
|
||||||
fn entry_lines(text: &str) -> Option<(String, Vec<String>, String)> {
|
|
||||||
let lines: Vec<&str> = text.split_inclusive('\n').collect();
|
|
||||||
let key = lines
|
|
||||||
.iter()
|
|
||||||
.position(|l| l.trim_end() == "classifications:")?;
|
|
||||||
let first = key
|
|
||||||
+ 1
|
|
||||||
+ lines[key + 1..]
|
|
||||||
.iter()
|
|
||||||
.position(|l| !l.trim().is_empty() && !l.trim_start().starts_with('#'))?;
|
|
||||||
let indent: String = lines[first].chars().take_while(|c| *c == ' ').collect();
|
|
||||||
let mut entries: Vec<String> = Vec::new();
|
|
||||||
for line in &lines[first..] {
|
|
||||||
if line
|
|
||||||
.strip_prefix(indent.as_str())
|
|
||||||
.is_some_and(|rest| rest.starts_with("- "))
|
|
||||||
{
|
|
||||||
entries.push((*line).to_owned());
|
|
||||||
} else {
|
|
||||||
entries.last_mut()?.push_str(line);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Some((lines[..first].concat(), entries, indent))
|
|
||||||
}
|
|
||||||
|
|
||||||
/// One classification as an entry of a YAML block list, its `-` at
|
|
||||||
/// `indent`.
|
|
||||||
fn yaml_entry(c: &Classification, indent: &str) -> String {
|
|
||||||
let polarity = match c.polarity {
|
|
||||||
Polarity::Positive => "positive",
|
|
||||||
Polarity::Negative => "negative",
|
|
||||||
};
|
|
||||||
let inner = format!("{indent} ");
|
|
||||||
let mut out = format!("{indent}- name: {}\n", yaml_name(&c.name));
|
|
||||||
if foldable(&c.description) {
|
|
||||||
let _ = writeln!(out, "{inner}description: >-");
|
|
||||||
let lead = format!("{inner} ");
|
|
||||||
let mut line = String::new();
|
|
||||||
for word in c.description.split(' ') {
|
|
||||||
if !line.is_empty() && lead.len() + line.len() + 1 + word.len() > YAML_WIDTH {
|
|
||||||
let _ = writeln!(out, "{lead}{line}");
|
|
||||||
line.clear();
|
|
||||||
}
|
|
||||||
if !line.is_empty() {
|
|
||||||
line.push(' ');
|
|
||||||
}
|
|
||||||
line.push_str(word);
|
|
||||||
}
|
|
||||||
let _ = writeln!(out, "{lead}{line}");
|
|
||||||
} else {
|
|
||||||
let _ = writeln!(out, "{inner}description: {}", json_string(&c.description));
|
|
||||||
}
|
|
||||||
let _ = writeln!(out, "{inner}polarity: {polarity}");
|
|
||||||
let _ = writeln!(out, "{inner}threshold: {}", c.threshold);
|
|
||||||
out
|
|
||||||
}
|
|
||||||
|
|
||||||
/// `name` bare when it can't read as anything but that string, quoted
|
|
||||||
/// otherwise.
|
|
||||||
fn yaml_name(name: &str) -> String {
|
|
||||||
let bare = name.starts_with(|c: char| c.is_ascii_lowercase())
|
|
||||||
&& name
|
|
||||||
.chars()
|
|
||||||
.all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-')
|
|
||||||
&& !matches!(
|
|
||||||
name,
|
|
||||||
"y" | "n" | "yes" | "no" | "on" | "off" | "true" | "false" | "null"
|
|
||||||
);
|
|
||||||
if bare {
|
|
||||||
name.to_owned()
|
|
||||||
} else {
|
|
||||||
json_string(name)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Whether a folded block scalar reads back as exactly `s`, which is trimmed
|
|
||||||
/// and not empty: one line of printable text, words split by single spaces.
|
|
||||||
fn foldable(s: &str) -> bool {
|
|
||||||
!s.contains(" ") && !s.chars().any(char::is_control)
|
|
||||||
}
|
|
||||||
|
|
||||||
/// `s` as a double-quoted scalar; JSON's escapes are YAML's too.
|
|
||||||
fn json_string(s: &str) -> String {
|
|
||||||
serde_json::Value::from(s).to_string()
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(test)]
|
|
||||||
mod tests {
|
|
||||||
use std::sync::Arc;
|
|
||||||
|
|
||||||
use axum::body::Body;
|
|
||||||
use axum::http::{Request, StatusCode};
|
|
||||||
use swarm_queue_client::review_classifier::{Overall, Reply};
|
|
||||||
use tower::ServiceExt as _;
|
|
||||||
|
|
||||||
use super::super::tests::Fake;
|
|
||||||
use super::*;
|
|
||||||
|
|
||||||
fn rule(name: &str, description: &str) -> Classification {
|
|
||||||
Classification {
|
|
||||||
name: name.to_owned(),
|
|
||||||
description: description.to_owned(),
|
|
||||||
polarity: Polarity::Negative,
|
|
||||||
threshold: 70,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn file(path: &str, text: &str) -> BranchFile {
|
|
||||||
BranchFile {
|
|
||||||
path: path.to_owned(),
|
|
||||||
sha: "b1".to_owned(),
|
|
||||||
bytes: text.as_bytes().to_vec(),
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
const OWN: &str = include_str!("../../../.hyperhive/review-classifier.yml");
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn a_new_rule_leaves_every_existing_entry_as_it_was_written() {
|
|
||||||
let [yml, _] = RULES_PATHS;
|
|
||||||
let before = parse_rules(yml, OWN.as_bytes()).expect("this repo's rules parse");
|
|
||||||
let new = rule(
|
|
||||||
"no-todo",
|
|
||||||
"The diff adds a TODO or FIXME comment without a tracker reference next to it, \
|
|
||||||
which leaves the follow-up with nobody to own it.",
|
|
||||||
);
|
|
||||||
let merged = merge(Some(&file(yml, OWN)), vec![new.clone()]).expect("merges");
|
|
||||||
let text = String::from_utf8(merged.content).expect("utf-8");
|
|
||||||
assert!(text.starts_with(OWN), "{text}");
|
|
||||||
assert_eq!(merged.sha.as_deref(), Some("b1"));
|
|
||||||
assert_eq!(merged.message, "review classifier: add no-todo");
|
|
||||||
let after = parse_rules(yml, text.as_bytes()).expect("parses");
|
|
||||||
assert_eq!(after, [before, vec![new.clone()]].concat());
|
|
||||||
assert!(text.lines().all(|l| l.len() <= YAML_WIDTH), "{text}");
|
|
||||||
|
|
||||||
let unterminated = OWN.trim_end();
|
|
||||||
let merged = merge(Some(&file(yml, unterminated)), vec![new]).expect("merges");
|
|
||||||
let text = String::from_utf8(merged.content).expect("utf-8");
|
|
||||||
assert!(
|
|
||||||
text.starts_with(&format!("{unterminated}\n - name: no-todo\n")),
|
|
||||||
"{text}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn a_rule_with_a_known_name_replaces_that_entry_in_place() {
|
|
||||||
let [yml, json] = RULES_PATHS;
|
|
||||||
let mut before = parse_rules(yml, OWN.as_bytes()).expect("this repo's rules parse");
|
|
||||||
let mut edited = before[1].clone();
|
|
||||||
edited.threshold = 90;
|
|
||||||
let padded = Classification {
|
|
||||||
name: format!(" {} ", edited.name),
|
|
||||||
..edited.clone()
|
|
||||||
};
|
|
||||||
let merged = merge(Some(&file(yml, OWN)), vec![padded.clone()]).expect("merges");
|
|
||||||
let after = parse_rules(yml, &merged.content).expect("parses");
|
|
||||||
before[1] = edited.clone();
|
|
||||||
assert_eq!(after, before);
|
|
||||||
assert_eq!(
|
|
||||||
merged.message,
|
|
||||||
format!("review classifier: update {}", edited.name)
|
|
||||||
);
|
|
||||||
|
|
||||||
let as_json = serde_json::json!({ "classifications": [&before[0], &before[2]] });
|
|
||||||
let merged = merge(
|
|
||||||
Some(&file(json, &as_json.to_string())),
|
|
||||||
vec![padded, rule("x", "y")],
|
|
||||||
)
|
|
||||||
.expect("merges");
|
|
||||||
let after = parse_rules(json, &merged.content).expect("parses");
|
|
||||||
assert_eq!(
|
|
||||||
after,
|
|
||||||
[
|
|
||||||
before[0].clone(),
|
|
||||||
before[2].clone(),
|
|
||||||
edited.clone(),
|
|
||||||
rule("x", "y")
|
|
||||||
]
|
|
||||||
);
|
|
||||||
assert_eq!(
|
|
||||||
merged.message,
|
|
||||||
format!("review classifier: add {}, x", edited.name)
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn no_rules_file_creates_the_yaml_one() {
|
|
||||||
let rules = vec![
|
|
||||||
rule("no-ads", "No ads."),
|
|
||||||
rule("null", "N."),
|
|
||||||
rule("Yes: #1", "A \"quote\"."),
|
|
||||||
// A full line, then a double space where the wrap falls.
|
|
||||||
rule("spaced", &format!("{} y", "x".repeat(72))),
|
|
||||||
rule("broken", "A\nnewline."),
|
|
||||||
];
|
|
||||||
let merged = merge(None, rules.clone()).expect("merges");
|
|
||||||
let [yml, _] = RULES_PATHS;
|
|
||||||
assert_eq!(merged.path, yml);
|
|
||||||
assert_eq!(merged.sha, None);
|
|
||||||
let after = parse_rules(yml, &merged.content).expect("parses");
|
|
||||||
assert_eq!(after, rules);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn a_list_the_splice_cant_follow_is_written_out_whole() {
|
|
||||||
let [yml, _] = RULES_PATHS;
|
|
||||||
let flow =
|
|
||||||
"classifications: [{name: a, description: b, polarity: negative, threshold: 70}]";
|
|
||||||
let merged = merge(Some(&file(yml, flow)), vec![rule("c", "d")]).expect("merges");
|
|
||||||
let after = parse_rules(yml, &merged.content).expect("parses");
|
|
||||||
assert_eq!(after, [rule("a", "b"), rule("c", "d")]);
|
|
||||||
|
|
||||||
// Kept as it is, `b` would point at an anchor the new `a` lacks.
|
|
||||||
let aliased = "\
|
|
||||||
classifications:
|
|
||||||
- name: a
|
|
||||||
description: &shared Shared.
|
|
||||||
polarity: negative
|
|
||||||
threshold: 70
|
|
||||||
- name: b
|
|
||||||
description: *shared
|
|
||||||
polarity: negative
|
|
||||||
threshold: 70
|
|
||||||
";
|
|
||||||
let mut a = rule("a", "Its own.");
|
|
||||||
a.threshold = 90;
|
|
||||||
let merged = merge(Some(&file(yml, aliased)), vec![a.clone()]).expect("merges");
|
|
||||||
let after = parse_rules(yml, &merged.content).expect("parses");
|
|
||||||
assert_eq!(after, [a, rule("b", "Shared.")]);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn nothing_new_or_too_many_or_a_bad_file_is_refused() {
|
|
||||||
let [yml, _] = RULES_PATHS;
|
|
||||||
let before = parse_rules(yml, OWN.as_bytes()).expect("this repo's rules parse");
|
|
||||||
let err = merge(Some(&file(yml, OWN)), before.clone()).expect_err("nothing to change");
|
|
||||||
assert!(err.contains("already holds"), "{err}");
|
|
||||||
let many: Vec<_> = (0..=10 - before.len())
|
|
||||||
.map(|i| rule(&format!("extra-{i}"), "d"))
|
|
||||||
.collect();
|
|
||||||
let err = merge(Some(&file(yml, OWN)), many).expect_err("eleven");
|
|
||||||
assert!(err.contains("11 classifications"), "{err}");
|
|
||||||
let err = merge(Some(&file(yml, "nope: 1")), vec![rule("a", "b")]).expect_err("bad");
|
|
||||||
assert!(err.contains(yml), "{err}");
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn a_branch_name_is_a_plain_ref_under_the_prefix() {
|
|
||||||
let name = branch_name("Alice Smith/../x").expect("a name");
|
|
||||||
let (who, hex) = name
|
|
||||||
.strip_prefix(BRANCH_PREFIX)
|
|
||||||
.and_then(|r| r.rsplit_once('-'))
|
|
||||||
.expect("prefix and suffix");
|
|
||||||
assert_eq!(who, "alice-smith----x");
|
|
||||||
assert_eq!(hex.len(), 8);
|
|
||||||
assert!(hex.bytes().all(|b| b.is_ascii_hexdigit()), "{name}");
|
|
||||||
}
|
|
||||||
|
|
||||||
fn app(fake: Fake) -> axum::Router {
|
|
||||||
let (mut state, _) = crate::tests::state_with_roster();
|
|
||||||
state.try_outs.fake = Some(Arc::new(fake));
|
|
||||||
crate::build_app(state)
|
|
||||||
}
|
|
||||||
|
|
||||||
fn fake() -> Fake {
|
|
||||||
Fake::new(
|
|
||||||
Reply::Done {
|
|
||||||
rows: Vec::new(),
|
|
||||||
overall: Overall::Approve,
|
|
||||||
},
|
|
||||||
vec![super::super::tests::rules_json()],
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
fn body(repo: &str) -> String {
|
|
||||||
serde_json::json!({
|
|
||||||
"repo": repo,
|
|
||||||
"classifications": [{
|
|
||||||
"name": "no-ads",
|
|
||||||
"description": "No ads.",
|
|
||||||
"polarity": "negative",
|
|
||||||
"threshold": 80,
|
|
||||||
}],
|
|
||||||
})
|
|
||||||
.to_string()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn post(
|
|
||||||
app: &axum::Router,
|
|
||||||
user: Option<&str>,
|
|
||||||
body: String,
|
|
||||||
) -> (StatusCode, serde_json::Value) {
|
|
||||||
let mut req = Request::post("/api/review-classifier/open-pr")
|
|
||||||
.header("content-type", "application/json");
|
|
||||||
if let Some(user) = user {
|
|
||||||
req = req.header("remote-user", user);
|
|
||||||
}
|
|
||||||
let res = app
|
|
||||||
.clone()
|
|
||||||
.oneshot(req.body(Body::from(body)).expect("a request"))
|
|
||||||
.await
|
|
||||||
.expect("infallible");
|
|
||||||
let status = res.status();
|
|
||||||
let bytes = axum::body::to_bytes(res.into_body(), usize::MAX)
|
|
||||||
.await
|
|
||||||
.expect("a body");
|
|
||||||
(status, serde_json::from_slice(&bytes).unwrap_or_default())
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn a_branch_is_pushed_as_the_operator_and_linked_for_the_pr() {
|
|
||||||
let fake = fake();
|
|
||||||
let pushed = Arc::clone(&fake.pushed);
|
|
||||||
let (status, answer) = post(&app(fake), Some("alice"), body("o/r")).await;
|
|
||||||
assert_eq!(status, StatusCode::CREATED, "{answer}");
|
|
||||||
let branch = answer["branch"].as_str().expect("a branch");
|
|
||||||
assert!(branch.starts_with("review-classifier/alice-"), "{branch}");
|
|
||||||
assert_eq!(
|
|
||||||
answer["compare_url"],
|
|
||||||
format!("https://forge.test/o/r/compare/main...{branch}")
|
|
||||||
);
|
|
||||||
let [_, json] = RULES_PATHS;
|
|
||||||
assert_eq!(answer["path"], json);
|
|
||||||
let pushed = pushed.lock().expect("unpoisoned");
|
|
||||||
let [(repo, commit)] = pushed.as_slice() else {
|
|
||||||
panic!("one push expected: {pushed:?}");
|
|
||||||
};
|
|
||||||
assert_eq!(repo, "o/r");
|
|
||||||
assert_eq!(
|
|
||||||
commit.author,
|
|
||||||
CommitAuthor {
|
|
||||||
name: "Alice".to_owned(),
|
|
||||||
email: "alice@example.test".to_owned(),
|
|
||||||
}
|
|
||||||
);
|
|
||||||
assert_eq!(commit.base, "main");
|
|
||||||
assert_eq!(commit.branch, branch);
|
|
||||||
assert_eq!(commit.sha.as_deref(), Some(format!("sha-{json}").as_str()));
|
|
||||||
let names: Vec<_> = parse_rules(json, &commit.content)
|
|
||||||
.expect("parses")
|
|
||||||
.into_iter()
|
|
||||||
.map(|c| c.name)
|
|
||||||
.collect();
|
|
||||||
assert_eq!(names, ["datetime", "no-ads"]);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn the_author_is_the_session_user_not_one_in_the_body() {
|
|
||||||
let fake = fake();
|
|
||||||
let pushed = Arc::clone(&fake.pushed);
|
|
||||||
let mut body: serde_json::Value = serde_json::from_str(&body("o/r")).expect("json");
|
|
||||||
body["author"] = "alice".into();
|
|
||||||
let (status, answer) = post(&app(fake), Some("bob"), body.to_string()).await;
|
|
||||||
assert_eq!(status, StatusCode::CREATED, "{answer}");
|
|
||||||
let pushed = pushed.lock().expect("unpoisoned");
|
|
||||||
assert_eq!(pushed[0].1.author.email, "bob@example.test");
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn no_session_user_or_no_forge_account_pushes_nothing() {
|
|
||||||
for (user, want, says) in [
|
|
||||||
(None, StatusCode::UNAUTHORIZED, "Remote-User"),
|
|
||||||
(Some(" "), StatusCode::UNAUTHORIZED, "Remote-User"),
|
|
||||||
(
|
|
||||||
Some("mallory"),
|
|
||||||
StatusCode::CONFLICT,
|
|
||||||
"log in to the forge once",
|
|
||||||
),
|
|
||||||
] {
|
|
||||||
let fake = fake();
|
|
||||||
let pushed = Arc::clone(&fake.pushed);
|
|
||||||
let (status, problem) = post(&app(fake), user, body("o/r")).await;
|
|
||||||
assert_eq!(status, want, "{user:?}: {problem}");
|
|
||||||
let detail = problem["detail"].as_str().expect("a detail");
|
|
||||||
assert!(detail.contains(says), "{user:?}: {detail}");
|
|
||||||
assert!(pushed.lock().expect("unpoisoned").is_empty(), "{user:?}");
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn a_refused_push_names_the_missing_access() {
|
|
||||||
for (refusal, want, says) in [
|
|
||||||
(StatusCode::FORBIDDEN, StatusCode::FORBIDDEN, "write access"),
|
|
||||||
(StatusCode::NOT_FOUND, StatusCode::FORBIDDEN, "write access"),
|
|
||||||
(StatusCode::CONFLICT, StatusCode::CONFLICT, "try again"),
|
|
||||||
(
|
|
||||||
StatusCode::INTERNAL_SERVER_ERROR,
|
|
||||||
StatusCode::SERVICE_UNAVAILABLE,
|
|
||||||
"500",
|
|
||||||
),
|
|
||||||
] {
|
|
||||||
let mut fake = fake();
|
|
||||||
fake.push_refusal = Some(refusal);
|
|
||||||
let (status, problem) = post(&app(fake), Some("alice"), body("o/r")).await;
|
|
||||||
assert_eq!(status, want, "{refusal}: {problem}");
|
|
||||||
let detail = problem["detail"].as_str().expect("a detail");
|
|
||||||
assert!(detail.contains(says), "{refusal}: {detail}");
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[tokio::test]
|
|
||||||
async fn a_bad_body_or_repo_is_refused_before_the_forge() {
|
|
||||||
for (body, want) in [
|
|
||||||
("{".to_owned(), StatusCode::BAD_REQUEST),
|
|
||||||
(body("o"), StatusCode::BAD_REQUEST),
|
|
||||||
(
|
|
||||||
serde_json::json!({ "repo": "o/r", "classifications": [] }).to_string(),
|
|
||||||
StatusCode::BAD_REQUEST,
|
|
||||||
),
|
|
||||||
(body("o/elsewhere"), StatusCode::NOT_FOUND),
|
|
||||||
(body("o/empty"), StatusCode::NOT_FOUND),
|
|
||||||
(body("o/down"), StatusCode::SERVICE_UNAVAILABLE),
|
|
||||||
] {
|
|
||||||
let (status, problem) = post(&app(fake()), Some("alice"), body.clone()).await;
|
|
||||||
assert_eq!(status, want, "{body}: {problem}");
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Loading…
Reference in a new issue