Skip to content

Commit 31d559f

Browse files
feat(web): add a Code Review page that edits each repository's rules through pull requests (#711)
* feat(web): add a Code Review page that edits each repository's rules through pull requests Each triaged repository's rules show as switches under Checks, Judging and Code review. Changes collect in a tray that previews the policy after them and opens one pull request through the policy patch route. While that pull request is open, the switches show its branch's file and mark each rule it changes until it merges. A policy that extends a shared file, or a file too large to read, is shown read only. * refactor(web): move the judging and code review settings onto the Code Review page The decision model leaves Settings, Triage; Settings, Code review and the repository page's code review card go. Links that asked for a model open the Code Review page. * docs: describe the Code Review page
1 parent d14ba61 commit 31d559f

47 files changed

Lines changed: 1182 additions & 217 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎README.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ identity, grants, credentials, runs and audit.
99

1010
Nothing runs by default. Setup asks for a host connection (a GitHub App or
1111
Bitbucket Data Center) and nothing else. A decision model is not a hub
12-
variable, and triage does not start until one is saved in Settings.
12+
variable, and triage does not start until one is saved on the Code review page.
1313

1414
## Get started
1515

@@ -45,12 +45,14 @@ Or by hand:
4545
2. **Connect GitHub.** Sign up; the portal shows **Set up Triage**: create the
4646
GitHub App, choose repositories, then open Repositories
4747
([Set up in the portal](docs/SELF_HOST.md#set-up-in-the-portal)). Setup does
48-
not ask for a decision model. Save one in Settings → Triage
48+
not ask for a decision model. Save one on the **Code review** page
4949
([decision model](docs/DECISION_MODEL.md)); triage starts once it is saved.
5050
3. **Add a policy file.** Commit `.corbits-triage.yml` to the repository's
5151
default branch, press **Start a policy file** in the portal to open a pull
5252
request with starter rules, or give your coding agent the Configure Prompt
53-
inside the repository. The format is in
53+
inside the repository. Change rules, judging and code review later from the
54+
portal's **Code review** page, which opens a pull request for each batch.
55+
The format is in
5456
[Policy file](docs/PRODUCT.md#policy-file).
5557

5658
**Configure Prompt**

‎apps/web/src/App.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import Audit from "./pages/Audit.tsx";
2626
import Inbox from "./pages/Inbox.tsx";
2727
import PRDetail from "./pages/PRDetail.tsx";
2828
import Repositories from "./pages/Repositories.tsx";
29+
import CodeReview from "./pages/CodeReview.tsx";
2930
import Settings from "./pages/Settings.tsx";
3031
import { legacyTriageTarget } from "./lib/inbox-view.ts";
3132

@@ -179,6 +180,7 @@ function Gate() {
179180
<Route path="/audit" element={<Audit />} />
180181
<Route path="/runs/*" element={<TriageRedirect />} />
181182
<Route path="/repositories/:owner?/:repo?" element={<Repositories />} />
183+
<Route path="/code-review" element={<CodeReview />} />
182184
<Route path="/settings" element={<Settings />} />
183185
<Route path="/settings/:tab" element={<Settings />} />
184186
<Route path="/" element={<Inbox />} />

‎apps/web/src/cards.css‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,30 @@
5454
.settings .sentence { flex-wrap: wrap; }
5555
.settings .sentence .txt { flex-basis: auto; }
5656
}
57+
58+
/* Code review: one repository's rules as switches, the tray that opens their pull request, and the settings every repository shares */
59+
.code-review .repo-switch { width: fit-content; margin: 16px 0 4px; }
60+
.code-review [data-slot="command-palette-inline-field"] { display: flex; align-items: center; gap: 8px; }
61+
.code-review [data-slot="command-palette-inline-results"] { left: 0; right: auto; }
62+
.code-review .repo-switch-btn { display: inline-flex; align-items: center; gap: 8px; padding: 6px 10px; font: inherit; font-size: 15px; color: inherit; background: var(--white); border: 1px solid var(--rule-strong); border-radius: var(--r-md); cursor: pointer; }
63+
.code-review .repo-switch-btn:focus-visible { outline: 0; box-shadow: var(--focus); }
64+
.code-review [data-slot="command-palette-input"] { height: 38px; padding: 0 10px; font-size: 15px; border: 1px solid var(--rule-strong); border-radius: var(--r-md); }
65+
.code-review [data-slot="command-palette-input"]:focus { border-color: var(--ink); box-shadow: var(--focus); }
66+
.code-review .rule { border-bottom: 1px solid var(--rule); }
67+
.code-review .rule .sentence { border-bottom: 0; }
68+
.code-review .rule.off .txt { color: var(--ink-mute); }
69+
.code-review .rule .help { margin: 0 0 10px; }
70+
.code-review .ask { display: grid; gap: 6px; margin: 0; padding: 0 0 12px; border: 0; font-size: 13px; }
71+
.code-review .ask label { display: flex; align-items: center; gap: 6px; }
72+
.code-review :is(.ask, .judge-form, .tray) .acts { display: flex; flex-wrap: wrap; gap: 8px; }
73+
.code-review .judge-form { display: grid; gap: 8px; margin-top: 16px; }
74+
.code-review .judge-form .preview { margin: 0; font-weight: 600; }
75+
.code-review .tray { position: sticky; bottom: 12px; z-index: 1; max-height: 45vh; overflow: auto; margin-top: 24px; padding: 14px 16px 16px; font-size: 14px; background: var(--white); border: 1px solid var(--rule-strong); border-radius: var(--r-md); }
76+
.code-review .tray ul { margin: 8px 0; padding-left: 18px; }
77+
.code-review .tray .acts { margin-top: 12px; }
78+
.code-review .file-pair { margin-top: 8px; }
79+
.code-review .file-pair summary { cursor: pointer; font-size: 13px; }
80+
.code-review .file-text { max-height: 240px; margin: 4px 0 8px; padding: 8px; overflow: auto; white-space: pre-wrap; font: 12px/1.4 var(--mono); background: var(--fill); }
81+
.code-review .for-all { margin: 48px 0 0; font-size: 20px; }
82+
.code-review .on-fail { display: block; font-size: 12px; color: var(--ink-mute); }
83+
.code-review .txt .pending { margin-left: 8px; vertical-align: middle; }

‎apps/web/src/components/Layout.tsx‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { usePortal } from "../lib/portal.tsx";
1313
import { triageEnabledRepos } from "../lib/repo-rows.ts";
1414
import { createHubTransport } from "../lib/hub-transport.ts";
1515
import { useSession } from "../lib/session.tsx";
16-
import { CheckIcon, GearIcon, InboxIcon, RepoIcon, SignOutIcon, UpDownIcon } from "./inbox-icons.tsx";
16+
import { CheckIcon, GearIcon, InboxIcon, RepoIcon, ReviewIcon, SignOutIcon, UpDownIcon } from "./inbox-icons.tsx";
1717

1818
function WorkspaceSwitcher() {
1919
const { snapshot, warn } = usePortal();
@@ -121,6 +121,7 @@ function triageList(path: string): boolean {
121121

122122
function roomOf(path: string, pullPage: boolean) {
123123
if (path.startsWith("/settings")) return "settings";
124+
if (path.startsWith("/code-review")) return "review";
124125
if (path.startsWith("/repositories")) return "repos";
125126
if (pullPage) return "pr";
126127
if (triageList(path)) return "inbox";
@@ -138,8 +139,8 @@ export default function Layout({ children }: { children: ReactNode }) {
138139
const pullPage = path.startsWith("/triage/pr/") || path.startsWith("/prs/");
139140
const room = roomOf(path, pullPage);
140141
const triageActive = room === "inbox" || room === "pr";
141-
// Only settings and the pull request page sit in the stage card; every other page draws its own inset panels.
142-
const ownPanels = room !== "settings" && room !== "pr";
142+
// Only settings, code review and the pull request page sit in the stage card; every other page draws its own inset panels.
143+
const ownPanels = room !== "settings" && room !== "review" && room !== "pr";
143144
const stageClass = room === "pr" ? "stage has-sliver" : "stage";
144145

145146
useEffect(function markBody() {
@@ -164,6 +165,10 @@ export default function Layout({ children }: { children: ReactNode }) {
164165
Triage
165166
{openPullsUnanswered ? null : <span className="n">{needsYou}</span>}
166167
</Link>
168+
<Link className="nav-a" to="/code-review" aria-current={room === "review" ? "true" : undefined}>
169+
<ReviewIcon />
170+
Code review
171+
</Link>
167172
<span className="grow" />
168173
<WorkspaceSwitcher />
169174
<UserBlock />
@@ -178,6 +183,10 @@ export default function Layout({ children }: { children: ReactNode }) {
178183
<InboxIcon />
179184
Triage
180185
</NavLink>
186+
<NavLink to="/code-review">
187+
<ReviewIcon />
188+
Review
189+
</NavLink>
181190
<NavLink to="/repositories">
182191
<RepoIcon />
183192
Repos
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { Button } from "@corbits/react-ui";
2+
import { errorText } from "@corbits/common";
3+
import type { CheckPack, PolicyFileStatus, PolicyPatchChange, RepoSource } from "@corbits/triage-contracts";
4+
import { counted } from "../../lib/plural.ts";
5+
import { usePatchPreview, usePolicyPatch, type PatchPreview } from "../../lib/policy-file.ts";
6+
import { changeSentences, touchedIds } from "../../lib/policy-patch.ts";
7+
import PolicyRules from "../repo-panel/PolicyRules.tsx";
8+
9+
type ChangeTrayProps = { repo: string; source: RepoSource; file: PolicyFileStatus; pack: CheckPack; changes: readonly PolicyPatchChange[]; disabled: boolean; onOpened: () => void };
10+
11+
function Preview({ preview, path, source }: { preview: PatchPreview; path: string; source: RepoSource }) {
12+
const { read } = preview;
13+
return (
14+
<>
15+
<p className="lead">After it merges, the policy reads:</p>
16+
{read.kind === "file" ? <PolicyRules pack={read.pack} source={source} /> : <p className="reason">{read.reason}</p>}
17+
<details className="file-pair">
18+
<summary><code>{path}</code> before and after</summary>
19+
<div><span>Before</span><pre className="file-text">{preview.before}</pre></div>
20+
<div><span>After</span><pre className="file-text">{preview.after}</pre></div>
21+
</details>
22+
</>
23+
);
24+
}
25+
26+
/** The batch in sentences, then what the policy reads after it and the file itself; one button opens the pull request. */
27+
export default function ChangeTray({ repo, source, file, pack, changes, disabled, onOpened }: ChangeTrayProps) {
28+
const preview = usePatchPreview(repo, source, changes);
29+
const open = usePolicyPatch(repo, source);
30+
const count = touchedIds(changes).length;
31+
const error = preview.error ?? open.error;
32+
// With a policy pull request open, the batch is applied to its branch, which may already hold it.
33+
const unchanged = preview.data !== undefined && preview.data.before === preview.data.after;
34+
35+
return (
36+
<aside className="tray" aria-labelledby="tray-title">
37+
<header>
38+
<b id="tray-title">{counted(count, "change", "changes")}. Open a pull request to apply {count === 1 ? "it" : "them"}.</b>
39+
</header>
40+
<ul>{changeSentences(pack, changes).map((sentence) => <li key={sentence}>{sentence}</li>)}</ul>
41+
{preview.data ? <Preview preview={preview.data} path={file.path} source={source} /> : null}
42+
{unchanged ? <p className="help">The open pull request already makes these changes.</p> : null}
43+
{error ? <p role="alert" className="error">{errorText(error)}</p> : null}
44+
<div className="acts">
45+
<Button type="button" size="sm" disabled={disabled || !preview.isSuccess || unchanged || open.isPending} onClick={() => open.mutate(changes, { onSuccess: onOpened })}>Open a pull request</Button>
46+
</div>
47+
</aside>
48+
);
49+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import type { ReactNode } from "react";
2+
import { presetFor } from "@corbits/triage-contracts";
3+
import { modelSecrets } from "../../lib/credential-actions.ts";
4+
import { DeniedNotice } from "../../lib/denied.tsx";
5+
import { isActiveCredential, type PortalSnapshot } from "../../lib/hub-api.ts";
6+
import { usePortal } from "../../lib/portal.tsx";
7+
import { DecisionModelForm } from "../DecisionModelForm.tsx";
8+
import { KeyRow } from "../settings/KeyRow.tsx";
9+
10+
/** A preset by its name; your own provider by its endpoint and model. */
11+
function judgeModel(saved: { endpoint: string; model: string }): ReactNode {
12+
const preset = presetFor(saved);
13+
if (preset.id !== "custom") return <b>{preset.label}</b>;
14+
return <><b>{saved.endpoint}</b> · <span className="mono small-text">{saved.model}</span></>;
15+
}
16+
17+
/** The decision model every repository's judging rules use, set once for the workspace. */
18+
export function DecisionModelSection({ snapshot }: { snapshot: PortalSnapshot }) {
19+
const { removeDecisionModel } = usePortal();
20+
const saved = snapshot.decisionModel;
21+
const words = saved ? <>Judge pull requests with {judgeModel(saved)}</> : "Judge pull requests with a decision model";
22+
23+
return (
24+
<section className="sec" id="decision-model" aria-labelledby="sec-decision-model">
25+
<header>
26+
<h2 id="sec-decision-model">Judging</h2>
27+
<p>Answers every repository's judging rules and chooses the verdict.</p>
28+
</header>
29+
<div>
30+
{snapshot.denied.credentials && <DeniedNotice section="credentials" />}
31+
<div className="sentences">
32+
<KeyRow
33+
words={words}
34+
name="decision model"
35+
keys={modelSecrets(snapshot.credentials).filter(isActiveCredential)}
36+
denied={snapshot.denied.credentials}
37+
remove={removeDecisionModel}
38+
error={snapshot.credentialsError}
39+
editor={(onSaved) => <DecisionModelForm onSaved={onSaved} />}
40+
/>
41+
</div>
42+
</div>
43+
</section>
44+
);
45+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import { afterEach, expect, mock, test } from "bun:test";
2+
import { act } from "react";
3+
import { createRoot, type Root } from "react-dom/client";
4+
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
5+
import { starterPack } from "@corbits/triage-contracts";
6+
import PolicySwitches from "./PolicySwitches.tsx";
7+
8+
const REPO = "acme/widgets";
9+
const pack = starterPack(REPO);
10+
let root: Root | undefined;
11+
const originalFetch = globalThis.fetch;
12+
13+
// Other DOM test files replace this module for the whole test process, so this file sets its own.
14+
mock.module("../../lib/portal.tsx", () => ({
15+
usePortal() {
16+
return { snapshot: { workspace: { tenantId: "tenant" } } };
17+
},
18+
}));
19+
20+
afterEach(function unmount() {
21+
globalThis.fetch = originalFetch;
22+
act(() => root?.unmount());
23+
document.body.innerHTML = "";
24+
});
25+
26+
type Sent = { url: string; body: unknown };
27+
28+
function fakeHub(sent: Sent[]): void {
29+
globalThis.fetch = async function answer(input: RequestInfo | URL, init?: RequestInit) {
30+
const url = String(input);
31+
sent.push({ url, body: typeof init?.body === "string" ? JSON.parse(init.body) : undefined });
32+
if (url.endsWith("/policy/patch")) return Response.json({ before: "rules: []\n", after: "rules: [] # off\n" });
33+
return Response.json({ status: "ok", pack, source: {}, automatic: [], wanted: [], mergeRisks: [], codeReview: false });
34+
} as typeof fetch;
35+
}
36+
37+
async function settle() {
38+
for (let i = 0; i < 5; i += 1) await act(() => new Promise((resolve) => setTimeout(resolve, 0)));
39+
}
40+
41+
test("turning a rule off lists the change and previews exactly that change", async () => {
42+
const sent: Sent[] = [];
43+
fakeHub(sent);
44+
const host = document.createElement("div");
45+
document.body.append(host);
46+
root = createRoot(host);
47+
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
48+
act(() => root?.render(
49+
<QueryClientProvider client={client}>
50+
<PolicySwitches
51+
repo={REPO}
52+
source="github"
53+
file={{ path: ".corbits-triage.yml", sha: "sha", url: "https://github.com/acme/widgets" }}
54+
pack={pack}
55+
reviewOn={false}
56+
hasModel
57+
reviewMissing={undefined}
58+
pull={null}
59+
pending={[]}
60+
disabled={false}
61+
/>
62+
</QueryClientProvider>,
63+
));
64+
65+
const conflicts = document.querySelector<HTMLButtonElement>('[role="switch"][aria-label="Flag merge conflicts with the base branch"]');
66+
if (!conflicts) throw new Error("no switch for the merge conflicts rule");
67+
act(() => conflicts.click());
68+
await settle();
69+
70+
const tray = document.querySelector("aside.tray");
71+
expect(tray?.querySelector("b")?.textContent).toBe("1 change. Open a pull request to apply it.");
72+
expect([...(tray?.querySelectorAll("li") ?? [])].map((item) => item.textContent)).toEqual(["Turn off “Flag merge conflicts with the base branch”."]);
73+
const patch = sent.find((request) => request.url.endsWith("/policy/patch"));
74+
expect(patch?.body).toEqual({ repo: REPO, source: "github", changes: [{ kind: "rule", id: "no-merge-conflicts", enabled: false }], preview: true });
75+
expect(tray?.textContent).toContain("After it merges, the policy reads:");
76+
});

0 commit comments

Comments
 (0)