diff --git a/apps/hub/src/github/automation.ts b/apps/hub/src/github/automation.ts index d21064b6..43122ea1 100644 --- a/apps/hub/src/github/automation.ts +++ b/apps/hub/src/github/automation.ts @@ -5,7 +5,7 @@ import { evaluateGrants } from "@intx/authz"; import { schema, type DB } from "@intx/db"; import { generateId } from "@intx/hub-common"; import type { GrantRule } from "@intx/types/authz"; -import { ACTION_KINDS, DO_GRANT_ACTION, doResource, hostKinds, postingKinds, TRIAGE_ROLE, type ActionKind, type RepoSource } from "@corbits/triage-contracts"; +import { ACTION_KINDS, DO_GRANT_ACTION, doResource, hostKinds, postingKinds, TRIAGE_ROLE, VERDICT_LABELS, type ActionKind, type RepoSource } from "@corbits/triage-contracts"; import { lockHolder } from "../system-writes/grants.js"; import { allowRole, grantAdminHost, type Tx } from "./admin-github-grant.js"; @@ -62,7 +62,7 @@ export async function grantWanted(tx: Tx, tenantId: string, repo: string, source await lockHolder(tx, tenantId, roleId); // Checked under the role's lock, which every grant writer takes, so a read overtaken meanwhile writes nothing. if (!stillWanted()) return; - const kinds = hostKinds(source).filter((kind) => wanted.includes(kind) || postingKinds(source).includes(kind)); + const kinds = hostKinds(source).filter((kind) => wanted.includes(kind) || postingKinds(source, VERDICT_LABELS).includes(kind)); await deleteKinds(tx, tenantId, roleId, repo, source, ACTION_KINDS.filter((kind) => !kinds.includes(kind))); for (const kind of kinds) await allowRole(tx, tenantId, roleId, doResource(kind, source, repo), DO_GRANT_ACTION, stamp); } diff --git a/apps/hub/src/github/policy-grants.ts b/apps/hub/src/github/policy-grants.ts index 42d66524..79b5ee1b 100644 --- a/apps/hub/src/github/policy-grants.ts +++ b/apps/hub/src/github/policy-grants.ts @@ -61,7 +61,7 @@ export function createPolicyGrants(deps: PolicyGrantsDeps) { const read = await deps.preview(tenantId, repo, source, text); if (read.status !== "ok") return []; // Enabling a repository grants posting, and triage runs on no other. - const waits = policyKinds(read.pack, read.policyWorkflow?.decide ?? [], source).filter((kind) => !postingKinds(source).includes(kind)); + const waits = policyKinds(read.pack, read.policyWorkflow?.decide ?? [], source).filter((kind) => !postingKinds(source, read.pack.labels).includes(kind)); return waits.filter((kind) => !read.automatic.includes(kind)); } diff --git a/apps/web/src/components/repo-panel/PolicyRules.tsx b/apps/web/src/components/repo-panel/PolicyRules.tsx index cebcffe4..e327a0c8 100644 --- a/apps/web/src/components/repo-panel/PolicyRules.tsx +++ b/apps/web/src/components/repo-panel/PolicyRules.tsx @@ -47,7 +47,7 @@ const WAITS = " (waits for you on Triage)"; * labels, so a label waits there only beside a waiting comment. */ function postText(pack: CheckPack, verdict: VerdictWord, kind: ActionKind, allowed: (kind: ActionKind) => boolean): string { - if (kind === "labels") { + if (kind === "labels" && pack.labels) { const label = `Label “${pack.labels[verdict]}”`; if (allowed("labels")) return label; return allowed("comment") ? `${label} (not added: Triage may not add labels here)` : `${label}${WAITS}`; @@ -90,7 +90,7 @@ export default function PolicyRules({ pack, source, automatic }: { pack: CheckPa
{VERDICT_NAMES[verdict]}
- {postingKinds(source).map((kind) => {postText(pack, verdict, kind, allowed)})} + {postingKinds(source, pack.labels).map((kind) => {postText(pack, verdict, kind, allowed)})} {pack.actions[verdict].filter((step) => step.do !== "nothing").map((step, at) => {doText(step)}{forText(step)}{stepWaits(verdict, step) ? WAITS : ""})}
diff --git a/packages/triage-contracts/src/check-pack.ts b/packages/triage-contracts/src/check-pack.ts index 87c1c12b..98327032 100644 --- a/packages/triage-contracts/src/check-pack.ts +++ b/packages/triage-contracts/src/check-pack.ts @@ -403,7 +403,9 @@ export type PolicyEvent = (typeof POLICY_EVENTS)[number]; export const VERDICTS = ["pass", "fail", "unsure"] as const; export type VerdictWord = (typeof VERDICTS)[number]; -export const VERDICT_LABELS: Record = { pass: "Triage: Ready", fail: "Triage: Not Ready", unsure: "Triage: Needs Human Review" }; +export type VerdictLabels = Record; + +export const VERDICT_LABELS: VerdictLabels = { pass: "Triage: Ready", fail: "Triage: Not Ready", unsure: "Triage: Needs Human Review" }; /** What a head's verdict still waits for. */ export type Awaits = "ci" | "judge" | "code-review"; @@ -442,8 +444,8 @@ export type CheckPack = { checks: Partial>; custom: CustomCheck[]; actions: PolicyActions; - /** So tools watching GitHub can react to it. */ - labels: Record; + /** So tools watching GitHub can react to it; absent, Triage writes no labels. */ + labels?: VerdictLabels; /** The groups the policy file defines, in its order, which steps name as `{ group }`. */ groups?: Record; /** What reading the stored policy left out because it no longer runs; each verdict says so. Never read back. */ @@ -583,7 +585,7 @@ export const checkPackSchema = type({ "checks?": CatalogChecksSchema, "custom?": CustomCheckSchema.array().atMostLength(CUSTOM_CHECK_CAP), "actions?": PolicyActionsSchema, - "labels?": { "+": "reject", "pass?": Label, "fail?": Label, "unsure?": Label }, + "labels?": type("true").or({ "+": "reject", "pass?": Label, "fail?": Label, "unsure?": Label }), "groups?": FileGroupsSchema, }); @@ -1158,8 +1160,8 @@ function parseGroups(raw: unknown): Record | undefined { return groups; } -function parseLabels(raw: unknown): Record { - if (raw === undefined) return { ...VERDICT_LABELS }; +function parseLabels(raw: unknown): VerdictLabels | undefined { + if (raw === undefined || raw === true) return { ...VERDICT_LABELS }; const row = record(raw, "labels"); const extra = Object.keys(row).filter((key) => !(VERDICTS as readonly string[]).includes(key)); if (extra.length) throw new Error(`labels must set only pass, fail or unsure, not ${extra.join(", ")}.`); @@ -1295,7 +1297,7 @@ export function readCheckPack(raw: unknown, expectedRepo?: string, refuse = fals const actions = parseActions(body.actions, dropped, refuse); const labels = parseLabels(body.labels); const groups = parseGroups(body.groups); - return withDropped(refuseTriageLabels({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, repo, checks, custom, actions, labels, ...(groups && { groups }) }), dropped); + return withDropped(refuseTriageLabels({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, repo, checks, custom, actions, ...(labels && { labels }), ...(groups && { groups }) }), dropped); } export type SharedPolicy = Omit; @@ -1304,7 +1306,8 @@ export type PolicyOverride = Pick & { extends: string; checks: Partial>>; actions: Partial; - labels: Partial>; + /** `true` turns the verdict labels on with their default names. */ + labels: Partial | true; }; export function extendsOf(raw: unknown): string | undefined { @@ -1323,7 +1326,7 @@ export function readSharedPolicy(raw: unknown): SharedPolicy { const custom = parseCustom(body.custom); const actions = parseActions(body.actions, dropped, false); const labels = parseLabels(body.labels); - return withDropped({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, checks, custom, actions, labels }, dropped); + return withDropped({ kind: CHECK_PACK_KIND, schemaVersion: CHECK_PACK_SCHEMA_VERSION, checks, custom, actions, ...(labels && { labels }) }, dropped); } /** Built-in rule rows as a repository sets them: `enabled` only where the row names it, so a limit alone keeps the shared switch. */ @@ -1348,8 +1351,9 @@ function parseOverrideActions(row: Partial return actions; } -function parseOverrideLabels(row: Partial> | undefined): PolicyOverride["labels"] { - const labels: PolicyOverride["labels"] = {}; +function parseOverrideLabels(row: true | Partial> | undefined): PolicyOverride["labels"] { + if (row === true) return true; + const labels: Partial = {}; for (const verdict of VERDICTS) { if (row?.[verdict] !== undefined) labels[verdict] = text(row[verdict], `labels ${verdict}`, LABEL_MAX); } @@ -1436,7 +1440,7 @@ function levelLabelRule(label: string): LevelLabelRule | undefined { * step naming a level label would flip it on and off across runs. Sensitive paths and risk add their own level labels. */ export function refuseTriageLabels(pack: CheckPack): CheckPack { - const verdictLabels = Object.values(pack.labels).map((label) => label.toLowerCase()); + const verdictLabels = Object.values(pack.labels ?? {}).map((label) => label.toLowerCase()); for (const [id, label] of labelRules(pack)) { const lower = label.toLowerCase(); const level = levelLabelRule(lower); diff --git a/packages/triage-contracts/src/do.ts b/packages/triage-contracts/src/do.ts index c0ccd99b..d7474218 100644 --- a/packages/triage-contracts/src/do.ts +++ b/packages/triage-contracts/src/do.ts @@ -1,5 +1,5 @@ import { type } from "arktype"; -import { ACTION_KINDS, VERDICTS, type ActionKind, type CheckPack, type VerdictWord } from "./check-pack.js"; +import { ACTION_KINDS, VERDICTS, type ActionKind, type CheckPack, type VerdictLabels, type VerdictWord } from "./check-pack.js"; import { kindsOf, type DecideRule } from "./policy-workflow.js"; import type { RepoSource } from "./repo-source.js"; @@ -41,9 +41,9 @@ export function alwaysHeld(list: VerdictWord, kind: ActionKind): boolean { return (list === "unsure" && kind === "close") || (list !== "pass" && kind === "merge"); } -/** What Triage posts with every verdict: a comment, and a label where the host has labels. */ -export function postingKinds(source: RepoSource): ActionKind[] { - return hostKinds(source).filter((kind) => kind === "comment" || kind === "labels"); +/** What Triage posts with every verdict: a comment, and a label where the host has labels and the policy turns them on. */ +export function postingKinds(source: RepoSource, labels: VerdictLabels | undefined): ActionKind[] { + return hostKinds(source).filter((kind) => kind === "comment" || (kind === "labels" && labels !== undefined)); } /** @@ -52,7 +52,7 @@ export function postingKinds(source: RepoSource): ActionKind[] { */ export function policyKinds(pack: CheckPack, decide: readonly DecideRule[], source: RepoSource): ActionKind[] { const steps = VERDICTS.flatMap((verdict) => pack.actions[verdict].filter((step) => step.do === "nothing" || !alwaysHeld(verdict, step.do)).map((step) => step.do)); - const used = new Set([...postingKinds(source), ...decide.flatMap((rule) => kindsOf(rule.do)), ...steps]); + const used = new Set([...postingKinds(source, pack.labels), ...decide.flatMap((rule) => kindsOf(rule.do)), ...steps]); return hostKinds(source).filter((kind) => used.has(kind)); } diff --git a/packages/triage-contracts/src/policy-export.ts b/packages/triage-contracts/src/policy-export.ts index 682ca711..3c6f1d1d 100644 --- a/packages/triage-contracts/src/policy-export.ts +++ b/packages/triage-contracts/src/policy-export.ts @@ -111,8 +111,9 @@ function fileActions(pack: CheckPack): Record | undefined { return Object.keys(actions).length ? actions : undefined; } -function fileLabels(pack: CheckPack): Record | undefined { - const changed = VERDICTS.flatMap((verdict) => (pack.labels[verdict] === VERDICT_LABELS[verdict] ? [] : [[verdict, pack.labels[verdict]]])); +function fileLabels({ labels }: CheckPack): Record | undefined { + if (!labels) return undefined; + const changed = VERDICTS.flatMap((verdict) => (labels[verdict] === VERDICT_LABELS[verdict] ? [] : [[verdict, labels[verdict]]])); return changed.length ? Object.fromEntries(changed) : undefined; } diff --git a/packages/triage-contracts/src/policy-file-problems.ts b/packages/triage-contracts/src/policy-file-problems.ts index fdeb28c6..efec47c0 100644 --- a/packages/triage-contracts/src/policy-file-problems.ts +++ b/packages/triage-contracts/src/policy-file-problems.ts @@ -44,7 +44,7 @@ const EXPECTED: ReadonlyArray = { @@ -30,8 +30,8 @@ const RETIRED_LABELS = [ export const TRIAGE_LABELS: readonly string[] = [...Object.values(VERDICT_LABELS), FAILED_LABEL, ...RETIRED_LABELS]; /** The mirror takes back these when they no longer apply. */ -export function ownedLabels(labels: Record): string[] { - return [...new Set([...TRIAGE_LABELS, ...Object.values(labels)])]; +export function ownedLabels(labels: VerdictLabels | undefined): string[] { + return [...new Set([...TRIAGE_LABELS, ...Object.values(labels ?? {})])]; } export interface Rendered { diff --git a/packages/triage-contracts/src/resolve-policy.ts b/packages/triage-contracts/src/resolve-policy.ts index 2cb249e6..a36b4713 100644 --- a/packages/triage-contracts/src/resolve-policy.ts +++ b/packages/triage-contracts/src/resolve-policy.ts @@ -5,11 +5,13 @@ import { readPolicyOverride, readSharedPolicy, refuseTriageLabels, + VERDICT_LABELS, type CatalogCheck, type CatalogId, type CheckPack, type PolicyOverride, type SharedPolicy, + type VerdictLabels, } from "./check-pack.js"; /** `sha` is the file's blob on the default branch. */ @@ -61,14 +63,17 @@ export function resolvePolicy(shared: SharedPolicy, repo: PolicyOverride): Check fail: repo.actions.fail ?? shared.actions.fail, unsure: repo.actions.unsure ?? shared.actions.unsure, }, - labels: { - pass: repo.labels.pass ?? shared.labels.pass, - fail: repo.labels.fail ?? shared.labels.fail, - unsure: repo.labels.unsure ?? shared.labels.unsure, - }, + ...resolvedLabels(repo.labels, shared.labels), }); } +/** The repository's own `labels` key wins, each name it leaves out taken from the shared policy or the default; without one the shared policy decides. */ +function resolvedLabels(repo: PolicyOverride["labels"], shared: VerdictLabels | undefined): { labels?: VerdictLabels } { + if (repo === true) return { labels: { ...VERDICT_LABELS } }; + if (Object.keys(repo).length === 0) return shared ? { labels: shared } : {}; + return { labels: { ...VERDICT_LABELS, ...shared, ...repo } }; +} + /** A policy file's pack over the policy file of the repository its `extends` names, which must extend nothing itself. */ export function extendedFilePack(repo: string, content: unknown, shared: { repo: string; policy: Record }): CheckPack { if (shared.policy.extends !== undefined) throw new Error(`${shared.repo}'s policy extends another, so no policy can extend it.`); diff --git a/packages/triage-workflows/src/actions/evaluate.ts b/packages/triage-workflows/src/actions/evaluate.ts index 017caff8..ea435196 100644 --- a/packages/triage-workflows/src/actions/evaluate.ts +++ b/packages/triage-workflows/src/actions/evaluate.ts @@ -1,5 +1,5 @@ import type { EffectContext } from "@intx/workflow"; -import { checkName, FAILED_LABEL, hostKinds, marksWaiting, ownedLabels, repoSourceOf, UNREAD_STATUS, VERDICT_LABELS, type ActionKind, type CheckPack, type RepoSource, type Step } from "@corbits/triage-contracts"; +import { checkName, FAILED_LABEL, hostKinds, marksWaiting, ownedLabels, repoSourceOf, UNREAD_STATUS, VERDICT_LABELS, type ActionKind, type CheckPack, type RepoSource, type Step, type VerdictLabels } from "@corbits/triage-contracts"; import { type } from "arktype"; import { approvalHeldBy, changesWanted, heldApprovals, holdDos, packApproves, stepActions, type ActionInput, type SuggestedAction } from "../logic/actions.js"; import type { CheckResult, DeterministicResult, PrFacts } from "../logic/checks.js"; @@ -155,19 +155,25 @@ export function withoutDos(feedback: string, actions: SuggestedAction[]): string * verdict label on GitHub, an error status on Bitbucket Data Center. None without a known head, so a newer head is * never touched, nor where triage does not write the verdict. */ -function degradedMark({ repo, number, headSha }: PrFacts, source: RepoSource, automatic: ActionKind[], owned: readonly string[], appLogin: string | undefined): WaitingMark | undefined { +function degradedMark({ repo, number, headSha }: PrFacts, source: RepoSource, automatic: ActionKind[], labels: VerdictLabels | undefined, owned: readonly string[], appLogin: string | undefined): WaitingMark | undefined { if (headSha === "" || !marksWaiting(source, automatic)) return undefined; const pull = { repo, number, onHead: headSha, ...(appLogin && { appLogin }) }; const args = source === "bitbucket-datacenter" ? { ...pull, labels: [], owned: [], status: UNREAD_STATUS, statusHead: headSha } - : { ...pull, labels: [FAILED_LABEL], owned }; + : { ...pull, labels: labels ? [FAILED_LABEL] : [], owned }; return { effectId: `degraded:${repo.replace("/", "--")}:${number}:${headSha}`, repoSource: source, args }; } +/** Without a policy Triage labels with the default names. */ +function verdictLabelsOf(it: Item): VerdictLabels | undefined { + return it.pack ? it.pack.labels : VERDICT_LABELS; +} + function degradedOf(it: Item, reason: string): Verdict { - const verdict = { ...stamp(it, null), ...degradedVerdict(reason), automatic: it.automatic, actions: [] }; + const labels = verdictLabelsOf(it); + const verdict = { ...stamp(it, null), ...degradedVerdict(reason, labels), automatic: it.automatic, actions: [] }; const request = toMirrorRequest(verdict, it.pack && ownedLabels(it.pack.labels)); - const mark = degradedMark(it.facts, repoSourceOf(it.repoSource), verdict.automatic, request.owned, it.appLogin); + const mark = degradedMark(it.facts, repoSourceOf(it.repoSource), verdict.automatic, labels, request.owned, it.appLogin); return { ...verdict, request, ...(mark && { mark }) }; } @@ -205,14 +211,12 @@ function verdictFrom(it: Item, rendered: RenderOutput): Verdict { const shown = it.blocksOn ? rendered.feedback : withDos(rendered.feedback, actions); // What reading the policy left out is said on every verdict, so the policy can be fixed. const reasons = [...rendered.reasons, ...(it.pack?.dropped ?? [])]; - const labels = it.pack && !rendered.waiting ? [it.pack.labels[rendered.verdict]] : rendered.labels; const owned = it.pack ? ownedLabels(it.pack.labels) : undefined; const source = repoSourceOf(it.repoSource); - const labelNames = it.pack?.labels ?? VERDICT_LABELS; const closed = it.facts.state === "closed"; - const mark = closed ? closedMark(source, it.facts, automatic, labelNames, it.appLogin) : rendered.waiting && waitingMark(source, it.facts, ["ci"], automatic, it.appLogin); + const mark = closed ? closedMark(source, it.facts, automatic, verdictLabelsOf(it), it.appLogin) : rendered.waiting && waitingMark(source, it.facts, ["ci"], automatic, it.appLogin); const verdict = { - ...stamp(it, it.facts.headSha), ...rendered, labels, reasons, feedback: shown, automatic, actions, + ...stamp(it, it.facts.headSha), ...rendered, reasons, feedback: shown, automatic, actions, ...(it.pack && packApproves(it.pack) && { approvable: held === undefined }), ...(wanted !== undefined && { changesWanted: wanted }), ...(mark && { mark }), ...(closed && { closed: true as const }), }; @@ -261,7 +265,7 @@ function verdictOf(it: Item, replies: unknown[], partReplies: unknown[]): Verdic const answers = { decisions, malformed: false }; const answered = choiceOver(verdictAnswer, VERDICT_CHOICES); const rendered = renderVerdict({ - author: it.facts.author, repo: it.facts.repo, repoSource: repoSourceOf(it.repoSource), headSha: it.facts.headSha, det: it.det, answers, plan, focused, stack: it.facts.stack, judgeError, + author: it.facts.author, repo: it.facts.repo, repoSource: repoSourceOf(it.repoSource), headSha: it.facts.headSha, det: it.det, labels: verdictLabelsOf(it), answers, plan, focused, stack: it.facts.stack, judgeError, judgeLimitExceeded: it.judgeLimitExceeded, judgeSkipped: it.judgeSkipped, reviewers: it.facts.reviewers, choice: answered, ...(choice && "judgeError" in choice && { choiceRefused: choice.judgeError }), ...(hostAwaited(it) !== undefined && { hostAwaited: hostAwaited(it) }), }); diff --git a/packages/triage-workflows/src/actions/waiting.ts b/packages/triage-workflows/src/actions/waiting.ts index 2e8c1bad..be9b4a43 100644 --- a/packages/triage-workflows/src/actions/waiting.ts +++ b/packages/triage-workflows/src/actions/waiting.ts @@ -1,5 +1,5 @@ import type { EffectContext } from "@intx/workflow"; -import { ownedLabels, parseCheckPack, repoSourceOf, trustedRequestOf, WAITING_STATUS, type Awaits, type KeptVerdict, type RepoSource, type VerdictWord } from "@corbits/triage-contracts"; +import { ownedLabels, parseCheckPack, repoSourceOf, trustedRequestOf, WAITING_STATUS, type Awaits, type KeptVerdict, type RepoSource, type VerdictLabels } from "@corbits/triage-contracts"; import { progressComment, type WaitingMark } from "../logic/render.js"; type Head = { repo: string; number: number; headSha: string }; @@ -26,10 +26,10 @@ export function waitingMark(source: RepoSource, { repo, number, headSha }: Head, * says Triage did not finish, and Triage's labels come off where it labels. Without its comment a verdict on the head * cannot be told from a stale one, so nothing is written. */ -export function closedMark(source: RepoSource, { repo, number, headSha, merged }: Head & { merged: boolean }, automatic: readonly unknown[], labels: Record, appLogin: string | undefined): WaitingMark | undefined { +export function closedMark(source: RepoSource, { repo, number, headSha, merged }: Head & { merged: boolean }, automatic: readonly unknown[], labels: VerdictLabels | undefined, appLogin: string | undefined): WaitingMark | undefined { const comment = automatic.includes("comment"); if (!comment) return undefined; - const owned = source === "github" && automatic.includes("labels") ? ownedLabels(labels) : []; + const owned = source === "github" && automatic.includes("labels") && labels ? ownedLabels(labels) : []; const args = { repo, number, onHead: headSha, labels: [], owned, closedOut: { merged, comment }, ...(appLogin && { appLogin }) }; return { effectId: `closed:${repo.replace("/", "--")}:${number}:${headSha}`, repoSource: source, args }; } @@ -43,7 +43,7 @@ export async function unready(input: unknown, _ctx: EffectContext, _signal: Abor if (!request || !trusted || request["kind"] !== "pr" || request["newHead"] !== true || repoSourceOf(request["repoSource"]) !== "github") return { unready: true }; const { repo, prNumber, headSha, automatic } = request; const pack = parseCheckPack(request["pack"] ?? request["checkPack"]); - if (typeof repo !== "string" || typeof prNumber !== "number" || typeof headSha !== "string" || !Array.isArray(automatic) || !automatic.includes("labels") || !pack) return { unready: true }; + if (typeof repo !== "string" || typeof prNumber !== "number" || typeof headSha !== "string" || !Array.isArray(automatic) || !automatic.includes("labels") || !pack?.labels) return { unready: true }; const appLogin = typeof request["appLogin"] === "string" ? request["appLogin"] : undefined; const args = { repo, number: prNumber, onHead: headSha, labels: [], owned: Object.values(pack.labels), ...(appLogin && { appLogin }) }; return { unready: true, mark: { effectId: `unready:${repo.replace("/", "--")}:${prNumber}:${headSha}`, repoSource: "github", args } }; diff --git a/packages/triage-workflows/src/logic/duplicates.test.ts b/packages/triage-workflows/src/logic/duplicates.test.ts index e0d94c02..dba285fd 100644 --- a/packages/triage-workflows/src/logic/duplicates.test.ts +++ b/packages/triage-workflows/src/logic/duplicates.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; import type { EffectContext } from "@intx/workflow"; -import { readCheckPack, readPolicyFile } from "@corbits/triage-contracts"; +import { readCheckPack, readPolicyFile, VERDICT_LABELS } from "@corbits/triage-contracts"; import { readFacts } from "../actions/facts.js"; import { rules } from "../actions/index.js"; import { deriveState, type PrFacts } from "./checks.js"; @@ -111,7 +111,7 @@ describe("duplicates-open-pull-request", () => { return JSON.stringify({ id, type: "score", score: 1, legend: levels, probabilities: Object.fromEntries(probabilities.map((p, index) => [String(index), p])), confidence: 0.9 }); } const answers = parseAnswers([score("duplicates-open-pull-request-001", [0.2, 0.5, 0.3]), score("duplicates-open-pull-request-002", [0.8, 0.15, 0.05]), JSON.stringify({ id: "instructed", type: "noul", noul: 0.1 })].join("")); - const verdict = renderVerdict({ repoSource: "github", author: "octocat", repo: "acme/widgets", headSha: "abc", det, answers, plan }); + const verdict = renderVerdict({ labels: VERDICT_LABELS, repoSource: "github", author: "octocat", repo: "acme/widgets", headSha: "abc", det, answers, plan }); expect(verdict.checks.find((check) => check.check === "duplicates-open-pull-request")).toMatchObject({ result: "fail", reason: "looks like the same change as #13", onFail: "ask", output: { kind: "level", level: "same change" } }); expect(verdict.verdict).toBe("unsure"); expect(verdict.feedback).toContain("Asking a maintainer about:\n- looks like the same change as #13"); diff --git a/packages/triage-workflows/src/logic/render.test.ts b/packages/triage-workflows/src/logic/render.test.ts index 2a479381..5bb59aff 100644 --- a/packages/triage-workflows/src/logic/render.test.ts +++ b/packages/triage-workflows/src/logic/render.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { classificationSources, emptyPack, type CheckPack } from "@corbits/triage-contracts"; +import { classificationSources, emptyPack, VERDICT_LABELS, type CheckPack } from "@corbits/triage-contracts"; import { MAX_MIRROR_COMMENT_BYTES, parseAnswers, renderVerdict } from "./render.js"; import type { DeterministicResult } from "./checks.js"; @@ -9,7 +9,7 @@ describe("cited evidence", () => { const failing = { check: "custom-1", kind: "machine", result: "fail", reason: "breaks the rule", evidence: ["src/a.ts:3"], output: { kind: "score", score: 0, passAt: 70, items: [citation] } } as const; const det: DeterministicResult = { state: "needs-author-update", reason: "a rule failed", findings: [], checks: [failing], needsJudgment: false, sources: classificationSources(emptyPack("acme/widgets")) }; function feedback(repoSource: "github" | "bitbucket-datacenter"): string { - return renderVerdict({ repoSource, author: "octocat", repo: "acme/widgets", headSha: "abc123", det }).feedback; + return renderVerdict({ labels: VERDICT_LABELS, repoSource, author: "octocat", repo: "acme/widgets", headSha: "abc123", det }).feedback; } expect(feedback("github")).toContain("[`src/a.ts:3`](https://github.com/acme/widgets/blob/abc123/src/a.ts?plain=1#L3)"); expect(feedback("bitbucket-datacenter")).toContain("breaks the rule: `src/a.ts:3`"); @@ -25,7 +25,7 @@ describe("a long comment", () => { test("an over-long comment is cut at line boundaries and still posts", () => { const custom = Array.from({ length: 200 }, (_, index) => ({ id: `custom-${index + 1}`, name: `${"N".repeat(500)}-${index}`, group: "code-vs-ci" as const, kind: "model" as const, shape: "is-true" as const, claim: "Keeps boundaries" })); const det: DeterministicResult = { state: "ready-monitoring", reason: "all configured checks pass", findings: [], checks: [], needsJudgment: true, sources: { quality: [], custom, rules: [] } }; - const verdict = renderVerdict({ repoSource: "github", author: "octocat", repo: "acme/widgets", headSha: "abc123", det, answers: noul(Object.fromEntries(custom.map((row) => [row.id, 0.1]))), choice: { pass: 0.1, fail: 0.85, cant_tell: 0.05 } }); + const verdict = renderVerdict({ labels: VERDICT_LABELS, repoSource: "github", author: "octocat", repo: "acme/widgets", headSha: "abc123", det, answers: noul(Object.fromEntries(custom.map((row) => [row.id, 0.1]))), choice: { pass: 0.1, fail: 0.85, cant_tell: 0.05 } }); const lines = verdict.feedback.split("\n").slice(3); expect(new TextEncoder().encode(verdict.feedback).byteLength).toBeLessThanOrEqual(MAX_MIRROR_COMMENT_BYTES); expect(lines[0]).toContain(custom[0]!.name); @@ -55,7 +55,7 @@ describe("score and choose rules", () => { test("a score rule records its level, never fails without failBelow, and fails when the level is likely below it", () => { const kind = JSON.stringify({ id: "kind", type: "choice", choice: "chore", probabilities: { fix: 0.1, feature: 0.1, chore: 0.8 }, confidence: 0.9 }); const answers = parseAnswers([score("style", [0.9, 0.1, 0, 0]), score("care", [0.5, 0.3, 0.2, 0]), kind].join("\n")); - const checks = renderVerdict({ repoSource: "github", author: "octocat", repo: "acme/widgets", headSha: "abc123", det: scored, answers }).checks; + const checks = renderVerdict({ labels: VERDICT_LABELS, repoSource: "github", author: "octocat", repo: "acme/widgets", headSha: "abc123", det: scored, answers }).checks; expect(checks.map((c) => [c.check, c.result, c.evidence])).toEqual([["style", "pass", ["poor"]], ["care", "fail", ["poor"]], ["kind", "fail", ["chore"]]]); expect(checks[1]!.output).toEqual({ kind: "level", level: "poor", levels, failBelow: "good" }); }); @@ -75,6 +75,7 @@ describe("author text in the comment", () => { { check: "focused", kind: "model" as const, result: "fail" as const, reason: "unrelated changes", evidence: hostile }, ]; const verdict = renderVerdict({ + labels: VERDICT_LABELS, repoSource: "github", author: "octocat", repo: "acme/widgets", diff --git a/packages/triage-workflows/src/logic/render.ts b/packages/triage-workflows/src/logic/render.ts index 5100284f..f7fe05ca 100644 --- a/packages/triage-workflows/src/logic/render.ts +++ b/packages/triage-workflows/src/logic/render.ts @@ -1,4 +1,4 @@ -import { BUDGET_EXHAUSTED_NOTE, catalogCheckEnabled, INCOMPLETE_NOTE, citationOf, citationUrl, dependsOnCi, hardFailures, isSizeRule, itemText, NO_CHANGES_REQUESTED, NOT_A_DRAFT, render, TRIAGE_LABELS, VERDICT_LABELS, type ActionKind, type Awaits, type CheckPack, type CheckItem, type CheckOutput, type CodeReviewOutput, type KeptVerdict, type ModelCustomCheck, type PolicySource, type Rendered, type RepoSource, type TriageHeader, type VerdictStatus, type VerdictWord, type WorkflowState } from "@corbits/triage-contracts"; +import { BUDGET_EXHAUSTED_NOTE, catalogCheckEnabled, INCOMPLETE_NOTE, citationOf, citationUrl, dependsOnCi, hardFailures, isSizeRule, itemText, NO_CHANGES_REQUESTED, NOT_A_DRAFT, render, TRIAGE_LABELS, type ActionKind, type Awaits, type CheckPack, type CheckItem, type CheckOutput, type CodeReviewOutput, type KeptVerdict, type ModelCustomCheck, type PolicySource, type Rendered, type RepoSource, type TriageHeader, type VerdictLabels, type VerdictStatus, type VerdictWord, type WorkflowState } from "@corbits/triage-contracts"; import { isRecord } from "@corbits/common"; import type { Stack } from "@corbits/github-reads"; import { Decision } from "@corbits/system-one"; @@ -112,6 +112,8 @@ export interface RenderInput { repoSource: RepoSource; headSha: string; det: DeterministicResult; + /** The verdict label names; absent, the verdict carries no label. */ + labels: VerdictLabels | undefined; answers?: ParsedAnswers | Record | null; plan?: QualityPlan; focused?: FocusedPlan; @@ -251,7 +253,7 @@ function band(noul: number, yesPasses: boolean): Decided { return { result: "unconfirmed", reason: UNDECIDED_REASON }; } -export function evaluateModel({ det, answers, plan = {}, focused, stack, judgeError, judgeLimitExceeded, judgeSkipped, choice }: Omit): ModelEvaluation { +export function evaluateModel({ det, answers, plan = {}, focused, stack, judgeError, judgeLimitExceeded, judgeSkipped, choice }: Omit): ModelEvaluation { if (!det.sources) return { checks: [], unanswered: false, instructed: false }; const sources = det.sources; // A failed chunk leaves only its own questions unanswered; the other chunks' answers still count. @@ -734,6 +736,7 @@ interface RenderContext { /** Set only on a stacked branch: the pull requests merged into it, and the open one it is based on. */ stack?: Stack; labelled: CheckResult[]; + labels: VerdictLabels | undefined; } function withChecks(rendered: VerdictBase, checks: CheckResult[], score: number | null, reasons: string[], ctx: RenderContext): RenderOutput { @@ -741,7 +744,7 @@ function withChecks(rendered: VerdictBase, checks: CheckResult[], score: number ...rendered, ...nextStep(checks, ctx.sources, ctx.reviewers, rendered.verdict), mirror: rendered.state !== "stale-unknown", - labels: [VERDICT_LABELS[rendered.verdict]], + labels: ctx.labels ? [ctx.labels[rendered.verdict]] : [], score, reasons, feedback: authorComment(ctx, checks, rendered.verdict), @@ -782,7 +785,7 @@ export function renderVerdict(input: RenderInput): RenderOutput { const awaited = input.hostAwaited === undefined ? undefined : HOST_AWAITED_TEXT[input.hostAwaited]; const evaluated = evaluateModel(input); const { deciding, labelled } = splitLabelled(det, evaluated.checks); - const ctx = { author, repo, repoSource, headSha, sources: det.sources, reviewers, purpose: input.focused?.purpose, stack: input.stack, labelled }; + const ctx = { author, repo, repoSource, headSha, sources: det.sources, reviewers, purpose: input.focused?.purpose, stack: input.stack, labelled, labels: input.labels }; const checks = [...det.checks, ...deciding]; const flaggedReason = checks.filter(isFlagged).map((check) => reasonOf(check, det.sources)).join("; "); const reasons = reasonsOf(det, deciding); @@ -825,11 +828,11 @@ export function renderVerdict(input: RenderInput): RenderOutput { return withChecks({ ...render(VERDICT_STATES[decided]), verdict: decided, degraded: null, reason: DECIDED_REASONS[decided] }, checks, choice.pass, reasons, ctx); } -export function degradedVerdict(reason: string): RenderOutput { +export function degradedVerdict(reason: string, labels: VerdictLabels | undefined): RenderOutput { return { ...render("stale-unknown"), mirror: false, - labels: [VERDICT_LABELS.unsure], + labels: labels ? [labels.unsure] : [], verdict: "unsure", score: null, reasons: ["Could not evaluate"], diff --git a/packages/triage-workflows/src/logic/request-changes.test.ts b/packages/triage-workflows/src/logic/request-changes.test.ts index 5aa61054..16008c6f 100644 --- a/packages/triage-workflows/src/logic/request-changes.test.ts +++ b/packages/triage-workflows/src/logic/request-changes.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { getReviews, REQUEST_CHANGES_DO_MARKER, type GithubFetch } from "@corbits/github-reads"; -import { DEFAULT_REPO_POLICY, defaultActions, emptyPack, type CheckPack } from "@corbits/triage-contracts"; +import { DEFAULT_REPO_POLICY, defaultActions, emptyPack, VERDICT_LABELS, type CheckPack } from "@corbits/triage-contracts"; import { approvalHeldBy, evaluateActions, heldApprovals } from "./actions.js"; import { deriveState, type PrFacts } from "./checks.js"; import { buildFacts } from "./facts.js"; @@ -37,7 +37,7 @@ const pack: CheckPack = { function input(over: Partial, using = pack) { const pr = { ...facts, ...over }; const det = deriveState(pr, undefined, using); - const rendered = renderVerdict({ repoSource: "github", author: pr.author, repo: pr.repo, headSha: pr.headSha, det, answers: null, choice: { pass: 0.9, fail: 0.05, cant_tell: 0.05 } }); + const rendered = renderVerdict({ labels: VERDICT_LABELS, repoSource: "github", author: pr.author, repo: pr.repo, headSha: pr.headSha, det, answers: null, choice: { pass: 0.9, fail: 0.05, cant_tell: 0.05 } }); return { rendered, input: { facts: pr, checks: rendered.checks, verdict: rendered.verdict, reason: rendered.reason, pack: using } }; } diff --git a/tooling/facts.ts b/tooling/facts.ts index 2329f497..78e89f50 100644 --- a/tooling/facts.ts +++ b/tooling/facts.ts @@ -2,6 +2,7 @@ import { deriveState, type PrFacts } from "../packages/triage-workflows/src/logi import { prepareQualityEvaluation } from "../packages/triage-workflows/src/logic/quality.js"; import { renderVerdict, type RenderOutput } from "../packages/triage-workflows/src/logic/render.js"; import { NO_FACTS } from "../packages/triage-workflows/src/logic/item.js"; +import { VERDICT_LABELS } from "../packages/triage-contracts/src/check-pack.js"; import type { PRRecord } from "../packages/triage-contracts/src/types.js"; function toFacts(pr: PRRecord): PrFacts { @@ -25,6 +26,6 @@ export function triage(pr: PRRecord): RenderOutput { const facts = toFacts(pr); const det = deriveState(facts); const { focused } = prepareQualityEvaluation(facts, det.sources); - const v = renderVerdict({ author: pr.author, repo: facts.repo, repoSource: "github", headSha: facts.headSha, det, focused }); + const v = renderVerdict({ author: pr.author, repo: facts.repo, repoSource: "github", headSha: facts.headSha, det, labels: VERDICT_LABELS, focused }); return { ...v, humanGated: v.humanGated || pr.spam >= 0.9 || pr.confidence < 0.5 }; } diff --git a/tooling/preview.ts b/tooling/preview.ts index 69d926f6..6fc7b90b 100644 --- a/tooling/preview.ts +++ b/tooling/preview.ts @@ -86,7 +86,7 @@ if (import.meta.main) { const facts = factsFor(number); const det = deriveState(facts, undefined, pack); const evaluation = previewEvaluation(facts, det); - const v = renderVerdict({ author: facts.author, repo: facts.repo, repoSource: "github", headSha: facts.headSha, det, plan: evaluation.plan, focused: evaluation.focused, reviewers: facts.reviewers, ...(await previewJudge(det, evaluation)) }); + const v = renderVerdict({ author: facts.author, repo: facts.repo, repoSource: "github", headSha: facts.headSha, det, labels: pack.labels, plan: evaluation.plan, focused: evaluation.focused, reviewers: facts.reviewers, ...(await previewJudge(det, evaluation)) }); rows.push({ pr: `#${number}`, title: facts.title.slice(0, 40),