diff --git a/apps/loopover-miner-ui/src/routeTree.gen.ts b/apps/loopover-miner-ui/src/routeTree.gen.ts index ed988e9660..c26cdefe71 100644 --- a/apps/loopover-miner-ui/src/routeTree.gen.ts +++ b/apps/loopover-miner-ui/src/routeTree.gen.ts @@ -9,27 +9,27 @@ // Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified. import { Route as rootRouteImport } from './routes/__root' -import { Route as RunHistoryRouteImport } from './routes/run-history' -import { Route as RankedCandidatesRouteImport } from './routes/ranked-candidates' -import { Route as PortfolioRouteImport } from './routes/portfolio' -import { Route as LedgersRouteImport } from './routes/ledgers' -import { Route as EarningsRouteImport } from './routes/earnings' -import { Route as AttemptsRouteImport } from './routes/attempts' import { Route as IndexRouteImport } from './routes/index' +import { Route as AttemptsRouteImport } from './routes/attempts' +import { Route as EarningsRouteImport } from './routes/earnings' +import { Route as LedgersRouteImport } from './routes/ledgers' +import { Route as PortfolioRouteImport } from './routes/portfolio' +import { Route as RankedCandidatesRouteImport } from './routes/ranked-candidates' +import { Route as RunHistoryRouteImport } from './routes/run-history' -const RunHistoryRoute = RunHistoryRouteImport.update({ - id: '/run-history', - path: '/run-history', +const IndexRoute = IndexRouteImport.update({ + id: '/', + path: '/', getParentRoute: () => rootRouteImport, } as any) -const RankedCandidatesRoute = RankedCandidatesRouteImport.update({ - id: '/ranked-candidates', - path: '/ranked-candidates', +const AttemptsRoute = AttemptsRouteImport.update({ + id: '/attempts', + path: '/attempts', getParentRoute: () => rootRouteImport, } as any) -const PortfolioRoute = PortfolioRouteImport.update({ - id: '/portfolio', - path: '/portfolio', +const EarningsRoute = EarningsRouteImport.update({ + id: '/earnings', + path: '/earnings', getParentRoute: () => rootRouteImport, } as any) const LedgersRoute = LedgersRouteImport.update({ @@ -37,19 +37,19 @@ const LedgersRoute = LedgersRouteImport.update({ path: '/ledgers', getParentRoute: () => rootRouteImport, } as any) -const EarningsRoute = EarningsRouteImport.update({ - id: '/earnings', - path: '/earnings', +const PortfolioRoute = PortfolioRouteImport.update({ + id: '/portfolio', + path: '/portfolio', getParentRoute: () => rootRouteImport, } as any) -const AttemptsRoute = AttemptsRouteImport.update({ - id: '/attempts', - path: '/attempts', +const RankedCandidatesRoute = RankedCandidatesRouteImport.update({ + id: '/ranked-candidates', + path: '/ranked-candidates', getParentRoute: () => rootRouteImport, } as any) -const IndexRoute = IndexRouteImport.update({ - id: '/', - path: '/', +const RunHistoryRoute = RunHistoryRouteImport.update({ + id: '/run-history', + path: '/run-history', getParentRoute: () => rootRouteImport, } as any) @@ -123,25 +123,25 @@ export interface RootRouteChildren { declare module '@tanstack/react-router' { interface FileRoutesByPath { - '/run-history': { - id: '/run-history' - path: '/run-history' - fullPath: '/run-history' - preLoaderRoute: typeof RunHistoryRouteImport + '/': { + id: '/' + path: '/' + fullPath: '/' + preLoaderRoute: typeof IndexRouteImport parentRoute: typeof rootRouteImport } - '/ranked-candidates': { - id: '/ranked-candidates' - path: '/ranked-candidates' - fullPath: '/ranked-candidates' - preLoaderRoute: typeof RankedCandidatesRouteImport + '/attempts': { + id: '/attempts' + path: '/attempts' + fullPath: '/attempts' + preLoaderRoute: typeof AttemptsRouteImport parentRoute: typeof rootRouteImport } - '/portfolio': { - id: '/portfolio' - path: '/portfolio' - fullPath: '/portfolio' - preLoaderRoute: typeof PortfolioRouteImport + '/earnings': { + id: '/earnings' + path: '/earnings' + fullPath: '/earnings' + preLoaderRoute: typeof EarningsRouteImport parentRoute: typeof rootRouteImport } '/ledgers': { @@ -151,25 +151,25 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof LedgersRouteImport parentRoute: typeof rootRouteImport } - '/earnings': { - id: '/earnings' - path: '/earnings' - fullPath: '/earnings' - preLoaderRoute: typeof EarningsRouteImport + '/portfolio': { + id: '/portfolio' + path: '/portfolio' + fullPath: '/portfolio' + preLoaderRoute: typeof PortfolioRouteImport parentRoute: typeof rootRouteImport } - '/attempts': { - id: '/attempts' - path: '/attempts' - fullPath: '/attempts' - preLoaderRoute: typeof AttemptsRouteImport + '/ranked-candidates': { + id: '/ranked-candidates' + path: '/ranked-candidates' + fullPath: '/ranked-candidates' + preLoaderRoute: typeof RankedCandidatesRouteImport parentRoute: typeof rootRouteImport } - '/': { - id: '/' - path: '/' - fullPath: '/' - preLoaderRoute: typeof IndexRouteImport + '/run-history': { + id: '/run-history' + path: '/run-history' + fullPath: '/run-history' + preLoaderRoute: typeof RunHistoryRouteImport parentRoute: typeof rootRouteImport } } diff --git a/apps/loopover-ui/content/docs/label-policy.mdx b/apps/loopover-ui/content/docs/label-policy.mdx new file mode 100644 index 0000000000..3ed45c8526 --- /dev/null +++ b/apps/loopover-ui/content/docs/label-policy.mdx @@ -0,0 +1,69 @@ +--- +title: Label policy — which labels carry scoring weight, and the rules for each +description: The labels that affect a contribution's score, who may apply them, and when work on them opens. Every rule here is enforced mechanically in the pipeline, not by judgement, and every enforcement is written to the ledger. +eyebrow: Contributors +--- + +Three labels categorise an issue, and they are not equal: `gittensor:priority` carries the +highest scoring multiplier. That makes it the one label whose application has to be +constrained by a **rule** rather than by judgement — otherwise the highest-value label is +whatever anyone says it is. + +Both rules below are enforced by the pipeline, on every label event. Neither is a +discretionary decision, neither is applied to some contributors and not others, and each +enforcement is written to the decision ledger with its rule id, so the history is +independently checkable. + +## The scoring labels + +| Label | Meaning | Who may apply it | +| --- | --- | --- | +| `gittensor:bug` | A fix, test, doc, chore, refactor, perf, ci, build or style change | Anyone | +| `gittensor:feature` | Genuinely new functionality | Anyone | +| `gittensor:priority` | Work the maintainer originated and triaged as most valuable | **Only valid on maintainer-authored issues** | + +## Rule 1 — priority is only valid on maintainer-authored issues + +`gittensor:priority` marks work **the maintainer originated**. If it is applied to an issue +somebody else authored — by anyone, including a maintainer — the pipeline removes it and +posts a single comment linking here. + +**Maintainer of record** is read from the repository's own permissions (`admin` or +`maintain`), never a hardcoded list of names, so the rule means the same thing on every repo +LoopOver manages. `write` access is deliberately *not* enough: handing out push access would +otherwise widen who can mint the highest multiplier. + +The rule **fails open**. If the author's permission cannot be read, nothing is stripped — a +label is never removed on the strength of a failed lookup, and the issue is re-judged next +time it is labelled. + +Removing the label is not a judgement about the issue. It stays open, and contributions to +it remain welcome under its other labels. + +Rule id: `priority-label-author-eligibility`. + +## Rule 2 — priority issues open for work after a short window + +Priority issues carry the highest payout, so assignment fairness matters most there. First-come +pickup is only fair if everyone can *see* the issue before anyone can act on it: a PR opened +moments after the label lands means the window between "issue becomes valuable" and "issue is +claimed" was effectively zero for everyone else watching the repo. + +So a PR closing a priority issue becomes gate-eligible once the label has been publicly present +for a short window — **30 minutes by default**, configurable per repo via +`gate.priorityEligibilityWindow`, and `0` turns it off. + +A PR opened inside that window is **not rejected**. It is held, with a comment naming the exact +moment it becomes eligible, and it proceeds normally once the window passes. There is no penalty +beyond waiting and nothing to resubmit. + +The clock is anchored to the **earliest** time the label was applied, so re-applying the label +never resets the window for anyone, and "when does this issue open for work" is a single instant +that cannot move. + +Rule id: `priority-eligibility-window`. + +## What is not in scope + +The multiplier *values* themselves are registry-side and are not set here. These two rules govern +which issues may carry the label and when work on them opens — not what the label is worth. diff --git a/apps/loopover-ui/src/components/site/docs-nav.tsx b/apps/loopover-ui/src/components/site/docs-nav.tsx index 5030175182..7fc8074f04 100644 --- a/apps/loopover-ui/src/components/site/docs-nav.tsx +++ b/apps/loopover-ui/src/components/site/docs-nav.tsx @@ -104,6 +104,7 @@ export const docsNav: DocsGroup[] = [ { to: "/docs/loopover-commands", label: "@loopover commands" }, { to: "/docs/branch-analysis", label: "Branch analysis" }, { to: "/docs/scoreability", label: "Scoreability" }, + { to: "/docs/label-policy", label: "Label policy" }, { to: "/docs/upstream-drift", label: "Upstream drift" }, { to: "/docs/backtest-calibration", label: "Backtest & calibration" }, { to: "/docs/verify-this-review", label: "Verify this review" }, diff --git a/src/github/comments.ts b/src/github/comments.ts index 77436322bd..edeff22042 100644 --- a/src/github/comments.ts +++ b/src/github/comments.ts @@ -1,5 +1,6 @@ import { withInstallationTokenRetry } from "./app"; import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; +import { PRIORITY_LABEL_COMMENT_MARKER } from "../review/priority-label-eligibility"; import type { AgentActionMode } from "../settings/agent-execution"; export const PR_PANEL_COMMENT_MARKER = ""; @@ -117,6 +118,19 @@ export async function createOrUpdateAgentCommandComment( return createOrUpdateIssueCommentWithMarker(env, installationId, repoFullName, issueNumber, body, AGENT_COMMAND_COMMENT_MARKER, { mode }); } +/** #9737: the priority-label policy notice. Marked like its siblings so a RE-label updates the existing + * comment instead of posting a second one -- the rule re-runs on every label event by design. */ +export async function createOrUpdatePriorityLabelPolicyComment( + env: Env, + installationId: number, + repoFullName: string, + issueNumber: number, + body: string, + mode: AgentActionMode = "live", +): Promise<{ id: number; html_url?: string; changed: boolean } | null> { + return createOrUpdateIssueCommentWithMarker(env, installationId, repoFullName, issueNumber, body, PRIORITY_LABEL_COMMENT_MARKER, { mode }); +} + // #6724 (review-burst): `changed` distinguishes a genuine no-op (the rendered body was byte-identical to what's // already posted, PATCH skipped -- see the idempotency comment below) from a real create/update, so a caller can // avoid double-counting a republish that produced no visible change. `false` ONLY on the proven-identical path; diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 25db7015e3..38d22ae41c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -152,6 +152,7 @@ import { isSelfAuthoredCiCompletionWebhook } from "../github/self-authored"; import { AGENT_COMMAND_COMMENT_MARKER, createOrUpdateAgentCommandComment, + createOrUpdatePriorityLabelPolicyComment, createOrUpdatePrIntelligenceComment, createOrUpdateVisualFollowupComment, PR_PANEL_COMMENT_MARKER, @@ -192,6 +193,12 @@ import { type PullRequestFreshness, } from "../github/pr-freshness"; import { DEFAULT_TYPE_LABELS, resolvePrTypeLabel } from "../settings/pr-type-label"; +import { + PRIORITY_LABEL_AUTHOR_RULE_ID, + PRIORITY_LABEL_ENFORCEMENT_EVENT, + PRIORITY_LABEL_POLICY_URL, + resolvePriorityLabelEnforcement, +} from "../review/priority-label-eligibility"; import { fetchLinkedIssueLabelsForPropagation } from "../review/linked-issue-label-propagation-fetch"; import { shouldPublishReviewCheck } from "../review/check-names"; import { fetchPublicContributorProfile } from "../github/public"; @@ -6801,6 +6808,82 @@ async function maybeHandleReactionWebhookEvent( return false; } + +/** + * `issues.labeled` -> author-based priority-label eligibility (#9737). + * + * `gittensor:priority` carries the highest scoring multiplier, so the one label whose application must be + * constrained by rule rather than judgment. Priority marks work the MAINTAINER originated and triaged, so + * on a contributor-authored issue it is stripped and the reason stated once, with a link to the policy. + * + * The DECISION is `resolvePriorityLabelEnforcement` (pure, unit-tested); this function is its I/O -- read + * the author's permission, strip, upsert the marked comment, record the enforcement event. Returns `true` + * when it claimed the event, matching every sibling handler here. + * + * FAIL-SAFE end to end: an unreadable permission yields no strip (the evaluator's own rule), and every + * GitHub call is caught so a label event can never fail the webhook. Re-labelling simply re-runs it, which + * is why the comment carries a marker and is updated rather than re-posted. + */ +async function maybeHandlePriorityLabelEligibility( + env: Env, + deliveryId: string, + eventName: string, + payload: GitHubWebhookPayload, +): Promise { + if (eventName !== "issues" || payload.action !== "labeled") return false; + const repoFullName = payload.repository?.full_name; + const issue = payload.issue; + const installationId = payload.installation?.id; + if (!repoFullName || !issue || !installationId) return false; + // A pull request arrives on the `issues` event too; the evaluator refuses it, but skipping here saves a + // permission read on every PR label. + if (issue.pull_request !== undefined && issue.pull_request !== null) return false; + + const settings = await resolveRepositorySettings(env, repoFullName).catch(() => undefined); + /* v8 ignore next 2 -- noUncheckedIndexedAccess fallback: PrTypeLabelSet is a Record, so + DEFAULT_TYPE_LABELS.priority reads as possibly-undefined to the type system though it is always set. */ + const priorityLabel: string = settings?.typeLabels?.priority ?? DEFAULT_TYPE_LABELS.priority ?? "gittensor:priority"; + const labels = (issue.labels ?? []).map((label) => label?.name ?? "").filter((name) => name.length > 0); + // Only the labelled event for THIS label matters; anything else is another label's business. + if (!labels.some((name) => name.toLowerCase() === priorityLabel.toLowerCase())) return false; + + const authorLogin = issue.user?.login ?? null; + const authorPermission = authorLogin + ? await getRepositoryCollaboratorPermission(env, installationId, repoFullName, authorLogin).catch(() => null) + : null; + + const { verdict, commentBody } = resolvePriorityLabelEnforcement({ + priorityLabel, + labels, + authorLogin, + authorPermission, + isPullRequest: false, + policyUrl: PRIORITY_LABEL_POLICY_URL, + }); + if (!verdict.strip || commentBody === null) return false; + + await removePullRequestLabel(env, installationId, repoFullName, issue.number, priorityLabel).catch(() => undefined); + await createOrUpdatePriorityLabelPolicyComment(env, installationId, repoFullName, issue.number, commentBody).catch(() => undefined); + await recordAuditEvent(env, { + eventType: PRIORITY_LABEL_ENFORCEMENT_EVENT, + actor: payload.sender?.login ?? null, + targetKey: `${repoFullName}#${issue.number}`, + outcome: "success", + detail: verdict.reason, + metadata: { ruleId: PRIORITY_LABEL_AUTHOR_RULE_ID, issueAuthor: authorLogin, authorPermission, label: priorityLabel }, + }).catch(() => undefined); + await recordWebhookEvent(env, { + deliveryId, + eventName, + action: payload.action, + installationId, + repositoryFullName: repoFullName, + payloadHash: "processed", + status: "processed", + }).catch(() => undefined); + return true; +} + /** * Handles the `issue_comment` webhook event's command/mention dispatch chain — panel retrigger, panel * generate-tests, gate-override, resolve/explain/generate-tests/review/pause/resume/configuration/plan @@ -7843,6 +7926,11 @@ export async function processGitHubWebhook( if (await maybeHandleReactionWebhookEvent(env, deliveryId, eventName, payload)) return; + // #9737: an `issues.labeled` event carrying the priority label is judged against the issue's AUTHOR + // before anything else looks at it -- the label is the scoring input, so the sooner an ineligible one + // is removed the less it can be acted on. + if (await maybeHandlePriorityLabelEligibility(env, deliveryId, eventName, payload)) return; + if ( await maybeHandleIssueCommentCommandWebhookEvent(env, deliveryId, eventName, payload) ) diff --git a/src/review/priority-label-eligibility.ts b/src/review/priority-label-eligibility.ts new file mode 100644 index 0000000000..4b47553859 --- /dev/null +++ b/src/review/priority-label-eligibility.ts @@ -0,0 +1,125 @@ +import { LOOPOVER_SITE_URL } from "../github/footer"; + +// Author-based eligibility for the priority label (#9737). +// +// `gittensor:priority` carries the highest scoring multiplier, which makes it the one label whose +// application must be constrained by rule rather than by judgment. The rule is mechanical: priority marks +// work the MAINTAINER has originated and triaged as most valuable, so the label is only valid on an issue +// the maintainer authored. Applied by anyone (including a maintainer) to a contributor-authored issue, it +// is stripped and the reason is stated once. +// +// Maintainer-of-record is derived from the repo's own permissions, never a hardcoded login, so the rule +// generalizes to any repo ORB manages. +// +// This module is the DECISION only. It takes facts and returns a verdict, so every branch is unit-testable +// without a webhook, a GitHub token, or a clock; the caller performs the strip, the comment and the ledger +// write. Same split as the linked-issue hard rules and the priority eligibility WINDOW (#9738), which is +// this rule's sibling: that one governs WHEN work on a priority issue may start, this one governs WHICH +// issues may carry the label at all. + +/** The rule's stable identifier, written to the ledger with every enforcement decision. */ +export const PRIORITY_LABEL_AUTHOR_RULE_ID = "priority-label-author-eligibility"; + +/** + * Repo permissions that make someone a maintainer OF RECORD for this rule. + * + * `admin` and `maintain` only. `write` is deliberately excluded: a contributor granted push access to a + * fork-free workflow is not thereby entitled to mint the highest-multiplier label, and the rule would be + * trivially widened by handing out write. GitHub's own permission vocabulary is the source -- see + * `getRepositoryCollaboratorPermission`. + */ +export const MAINTAINER_OF_RECORD_PERMISSIONS: readonly string[] = ["admin", "maintain"]; + +export function isMaintainerOfRecord(permission: string | null | undefined): boolean { + return typeof permission === "string" && MAINTAINER_OF_RECORD_PERMISSIONS.includes(permission.toLowerCase()); +} + +export type PriorityLabelVerdict = + | { strip: false; reason: null } + | { strip: true; reason: string; comment: string }; + +const KEEP: PriorityLabelVerdict = { strip: false, reason: null }; + +export type PriorityLabelEligibilityInput = { + /** The label whose application is being judged, as the repo names it. */ + priorityLabel: string; + /** Labels currently on the issue. */ + labels: readonly string[]; + /** The issue's AUTHOR -- not the actor who applied the label. */ + authorLogin: string | null | undefined; + /** The author's permission on this repo, from GitHub. `null` when it could not be read. */ + authorPermission: string | null | undefined; + /** True when the issue is a pull request. PRs carry the same label name for a different purpose. */ + isPullRequest: boolean; + /** Where the policy is written down, linked from the comment so the rule is never just an assertion. */ + policyUrl: string; +}; + +/** + * PURE evaluator. Returns `strip: true` only when every fact needed to justify removing the label is known + * and the author is provably not a maintainer of record. + * + * FAILS OPEN on every uncertainty -- an unreadable permission, an unknown author, a label that is not + * present. Stripping the highest-value label off a maintainer's own issue because we could not read a + * permission would be a worse error than leaving one wrongly applied, and the periodic sweep will re-judge + * it once the read succeeds. + * + * A PULL REQUEST is never touched. `gittensor:priority` is also the PR TYPE label for a content submission + * (see settings/pr-type-label.ts), which ORB applies itself -- this rule is about issues, and conflating + * the two would have it fighting the labeller. + */ +export function evaluatePriorityLabelEligibility(input: PriorityLabelEligibilityInput): PriorityLabelVerdict { + if (input.isPullRequest) return KEEP; + + const wanted = input.priorityLabel.trim().toLowerCase(); + if (!wanted) return KEEP; + if (!input.labels.some((label) => label.trim().toLowerCase() === wanted)) return KEEP; + + const author = (input.authorLogin ?? "").trim(); + if (!author) return KEEP; + // An unreadable permission is not evidence of anything. + if (input.authorPermission === null || input.authorPermission === undefined) return KEEP; + if (isMaintainerOfRecord(input.authorPermission)) return KEEP; + + return { + strip: true, + reason: `${PRIORITY_LABEL_AUTHOR_RULE_ID}: issue authored by @${author} (repo permission "${input.authorPermission}"), which is not a maintainer of record`, + comment: [ + `\`${input.priorityLabel}\` marks work a maintainer originated and triaged as highest-value, so it only applies to maintainer-authored issues — it has been removed from this one automatically.`, + "", + "This is not a judgement about the issue: it stays open and contributions to it are still welcome under its other labels. The policy, and which labels carry scoring weight, are documented here:", + "", + input.policyUrl, + ].join("\n"), + }; +} + +/** Where the policy lives, built from the canonical site URL rather than a second hardcoded origin -- + * linked from the comment so the rule is never a bare assertion, with one place to change it. */ +export const PRIORITY_LABEL_POLICY_URL = `${LOOPOVER_SITE_URL}/docs/label-policy`; + +/** Where the enforcement decision is written, so a strip can be audited without reading GitHub. */ +export const PRIORITY_LABEL_ENFORCEMENT_EVENT = "labels.priority_author_ineligible"; + +/** The marker that makes the comment IDEMPOTENT: found on an existing comment, that comment is updated + * rather than a second one posted. Re-labelling therefore re-strips without ever spamming the thread. */ +export const PRIORITY_LABEL_COMMENT_MARKER = ""; + +export type PriorityLabelEnforcement = { + verdict: PriorityLabelVerdict; + /** The comment body to post or update, marker included. Null when nothing is to be said. */ + commentBody: string | null; +}; + +/** + * The full enforcement shape for one issue: the verdict plus the exact comment body to write. + * + * Kept beside the evaluator (and equally pure) so the caller's only remaining job is I/O -- strip the + * label, upsert the marked comment, record the event. That split is what lets the whole rule be tested + * without a webhook or a token. + */ +export function resolvePriorityLabelEnforcement(input: PriorityLabelEligibilityInput): PriorityLabelEnforcement { + const verdict = evaluatePriorityLabelEligibility(input); + if (!verdict.strip) return { verdict, commentBody: null }; + return { verdict, commentBody: `${PRIORITY_LABEL_COMMENT_MARKER}\n\n${verdict.comment}` }; +} diff --git a/test/unit/priority-label-eligibility.test.ts b/test/unit/priority-label-eligibility.test.ts new file mode 100644 index 0000000000..36f56ac1ae --- /dev/null +++ b/test/unit/priority-label-eligibility.test.ts @@ -0,0 +1,98 @@ +// Author-based eligibility for the priority label (#9737): every branch, both sides of every fallback. +import { describe, expect, it } from "vitest"; +import { + MAINTAINER_OF_RECORD_PERMISSIONS, + PRIORITY_LABEL_AUTHOR_RULE_ID, + evaluatePriorityLabelEligibility, + isMaintainerOfRecord, + PRIORITY_LABEL_COMMENT_MARKER, + resolvePriorityLabelEnforcement, +} from "../../src/review/priority-label-eligibility"; + +const BASE = { + priorityLabel: "gittensor:priority", + labels: ["gittensor:priority"], + authorLogin: "contributor", + authorPermission: "read", + isPullRequest: false, + policyUrl: "https://loopover.ai/docs/label-policy", +}; + +describe("priority label author eligibility (#9737)", () => { + it("strips the label from a contributor-authored issue, and says why", () => { + const verdict = evaluatePriorityLabelEligibility(BASE); + // The union narrows on `strip`, so a caller cannot read `comment` off a keep verdict by accident. + if (!verdict.strip) throw new Error("expected the label to be stripped"); + expect(verdict.reason).toContain(PRIORITY_LABEL_AUTHOR_RULE_ID); + expect(verdict.reason).toContain("@contributor"); + // The comment must read as policy, not as a verdict on the person or the issue. + expect(verdict.comment).toContain(BASE.policyUrl); + expect(verdict.comment).toContain("still welcome"); + expect(verdict.comment).not.toMatch(/violation|rejected|invalid|spam/i); + }); + + it("leaves a maintainer-authored issue alone, for every maintainer permission", () => { + for (const permission of MAINTAINER_OF_RECORD_PERMISSIONS) { + expect(evaluatePriorityLabelEligibility({ ...BASE, authorPermission: permission }).strip, permission).toBe(false); + } + }); + + it("treats WRITE as not-a-maintainer, so handing out push access does not widen the rule", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, authorPermission: "write" }).strip).toBe(true); + expect(isMaintainerOfRecord("write")).toBe(false); + expect(isMaintainerOfRecord("triage")).toBe(false); + }); + + it("matches permissions case-insensitively", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, authorPermission: "ADMIN" }).strip).toBe(false); + expect(isMaintainerOfRecord("Maintain")).toBe(true); + }); + + it("never touches a PULL REQUEST — the same label is the PR type label ORB applies itself", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, isPullRequest: true }).strip).toBe(false); + }); + + it("does nothing when the label is not actually on the issue", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, labels: ["gittensor:bug"] }).strip).toBe(false); + expect(evaluatePriorityLabelEligibility({ ...BASE, labels: [] }).strip).toBe(false); + }); + + it("matches the label case-insensitively and ignores surrounding whitespace", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, labels: [" Gittensor:Priority "] }).strip).toBe(true); + }); + + it("FAILS OPEN when the permission could not be read", () => { + // Stripping the highest-value label off a maintainer's own issue because WE could not read a permission + // is worse than leaving one wrongly applied; the sweep re-judges it once the read succeeds. + expect(evaluatePriorityLabelEligibility({ ...BASE, authorPermission: null }).strip).toBe(false); + expect(evaluatePriorityLabelEligibility({ ...BASE, authorPermission: undefined }).strip).toBe(false); + }); + + it("FAILS OPEN when the author is unknown", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, authorLogin: null }).strip).toBe(false); + expect(evaluatePriorityLabelEligibility({ ...BASE, authorLogin: " " }).strip).toBe(false); + }); + + it("FAILS OPEN when the repo configures no priority label", () => { + expect(evaluatePriorityLabelEligibility({ ...BASE, priorityLabel: "" }).strip).toBe(false); + }); + + it("exposes a stable rule id, because the ledger records it", () => { + expect(PRIORITY_LABEL_AUTHOR_RULE_ID).toBe("priority-label-author-eligibility"); + }); +}); + +describe("resolvePriorityLabelEnforcement (#9737)", () => { + it("marks the comment so a re-label updates it instead of posting a second one", () => { + const { verdict, commentBody } = resolvePriorityLabelEnforcement(BASE); + expect(verdict.strip).toBe(true); + expect(commentBody).toContain(PRIORITY_LABEL_COMMENT_MARKER); + expect(commentBody).toContain(BASE.policyUrl); + }); + + it("says nothing at all when the label is legitimate", () => { + const { verdict, commentBody } = resolvePriorityLabelEnforcement({ ...BASE, authorPermission: "admin" }); + expect(verdict.strip).toBe(false); + expect(commentBody).toBeNull(); + }); +}); diff --git a/test/unit/priority-label-webhook.test.ts b/test/unit/priority-label-webhook.test.ts new file mode 100644 index 0000000000..5042fb7327 --- /dev/null +++ b/test/unit/priority-label-webhook.test.ts @@ -0,0 +1,253 @@ +// The `issues.labeled` enforcement path for #9737, end to end through the real webhook processor. +// +// The DECISION has its own unit tests (priority-label-eligibility.test.ts); this covers the I/O the +// decision drives: the permission read, the label removal, the marked comment, and the ledger event -- plus +// the paths that must do NOTHING, which is where a rule like this does damage if it is wrong. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { generateKeyPairSync } from "node:crypto"; +import { processJob } from "../../src/queue/processors"; +import { createTestEnv } from "../helpers/d1"; +import { upsertRepositoryFromGitHub, upsertRepositorySettings } from "../../src/db/repositories"; +import * as repositorySettingsModule from "../../src/settings/repository-settings"; +import { PRIORITY_LABEL_COMMENT_MARKER, PRIORITY_LABEL_ENFORCEMENT_EVENT } from "../../src/review/priority-label-eligibility"; + +const REPO = "JSONbored/gittensory"; + +/** Same helper the other webhook suites use: the App path needs a real key to mint an installation token. */ +function generateRsaPrivateKeyPem(): string { + const { privateKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); + return privateKey.export({ type: "pkcs1", format: "pem" }).toString(); +} + +type Call = { method: string; url: string; body?: string }; + +/** Stub GitHub: records every call, answers the three endpoints this path touches. */ +function stubGitHub(permission: string | null, calls: Call[], existingComments: Array<{ id: number; body: string; user?: { login: string; type?: string } }> = []) { + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const method = (init?.method ?? "GET").toUpperCase(); + calls.push({ method, url, ...(typeof init?.body === "string" ? { body: init.body } : {}) }); + + if (url.includes("/access_tokens")) return Response.json({ token: "ghs_test", expires_at: "2099-01-01T00:00:00Z" }); + if (url.includes("/collaborators/") && url.endsWith("/permission")) { + return permission === null ? new Response("nope", { status: 404 }) : Response.json({ permission }); + } + if (url.includes("/issues/") && url.includes("/comments") && method === "GET") return Response.json(existingComments); + if (url.includes("/issues/") && url.includes("/comments") && method === "POST") return Response.json({ id: 1, html_url: "u" }); + if (url.includes("/comments/") && method === "PATCH") return Response.json({ id: 1, html_url: "u" }); + if (url.includes("/labels/") && method === "DELETE") return Response.json([]); + if (url.includes("/installation")) return Response.json({ id: 123, account: { login: "JSONbored" } }); + return new Response("not found", { status: 404 }); + }); +} + +async function seed() { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem(), GITHUB_APP_SLUG: "loopover-orb" }); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: REPO, private: false, owner: { login: "JSONbored" } } as never); + await upsertRepositorySettings(env, { repoFullName: REPO, typeLabelsEnabled: true }); + return env; +} + +function labeledPayload(over: Record = {}) { + return { + action: "labeled", + installation: { id: 123 }, + repository: { full_name: REPO, name: "gittensory", private: false, owner: { login: "JSONbored" } }, + sender: { login: "someone" }, + label: { name: "gittensor:priority" }, + issue: { + number: 7, + title: "An issue", + state: "open", + user: { login: "contributor" }, + labels: [{ name: "gittensor:priority" }], + ...over, + }, + }; +} + +async function run(env: ReturnType, payload: unknown, deliveryId = "d1") { + await processJob(env, { type: "github-webhook", deliveryId, eventName: "issues", payload } as never); +} + +describe("issues.labeled priority enforcement (#9737)", () => { + afterEach(() => { + // The stubbed fetch and any resolver spy must not leak into the next case -- a leaked spy here would + // silently make a later assertion pass for the wrong reason. + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + it("strips the label, comments once with the marker, and records the enforcement", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload()); + + const removed = calls.find((call) => call.method === "DELETE" && call.url.includes("/labels/")); + expect(removed, "the label is removed").toBeTruthy(); + expect(decodeURIComponent(removed!.url)).toContain("gittensor:priority"); + + const commented = calls.find((call) => call.method === "POST" && call.url.includes("/comments")); + expect(commented?.body, "the comment carries the marker so a re-label updates it").toContain(PRIORITY_LABEL_COMMENT_MARKER); + + const audit = await env.DB.prepare("select detail, outcome from audit_events where event_type = ?") + .bind(PRIORITY_LABEL_ENFORCEMENT_EVENT) + .first<{ detail: string; outcome: string }>(); + expect(audit?.outcome).toBe("success"); + expect(audit?.detail, "the ledger says which rule fired and why").toContain("priority-label-author-eligibility"); + }); + + it("UPDATES the existing comment on a re-label instead of posting a second one", async () => { + const env = await seed(); + const calls: Call[] = []; + // The bot's own prior notice is already on the thread. + // Authored by the App itself -- the upsert only ever updates its OWN marked comment, never a human's. + stubGitHub("read", calls, [{ id: 55, body: `${PRIORITY_LABEL_COMMENT_MARKER}\n\nolder text`, user: { login: "loopover-orb[bot]", type: "Bot" } }]); + + await run(env, labeledPayload(), "d-relabel"); + + expect(calls.some((call) => call.method === "POST" && call.url.includes("/comments")), "no second comment").toBe(false); + }); + + it("does NOTHING for a maintainer-authored issue", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("admin", calls); + + await run(env, labeledPayload(), "d-maintainer"); + + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + expect(calls.some((call) => call.method === "POST" && call.url.includes("/comments"))).toBe(false); + }); + + it("FAILS OPEN when the permission cannot be read", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub(null, calls); + + await run(env, labeledPayload(), "d-unreadable"); + + expect(calls.some((call) => call.method === "DELETE"), "an unreadable permission never strips a label").toBe(false); + }); + + it("ignores a PULL REQUEST carrying the same label", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload({ pull_request: { url: "https://api.github.com/pulls/7" } }), "d-pr"); + + // The same label name is the PR TYPE label ORB applies itself; touching it would fight the labeller. + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + expect(calls.some((call) => call.url.includes("/permission")), "and costs no permission read").toBe(false); + }); + + it("ignores a labeled event for a DIFFERENT label", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload({ labels: [{ name: "gittensor:bug" }] }), "d-other-label"); + + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + }); + + it("ignores an issues action that is not `labeled`", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, { ...labeledPayload(), action: "opened" }, "d-opened"); + + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + }); + + it("ignores an `issues` payload missing the fields it needs, rather than throwing on the webhook path", async () => { + // A webhook handler that throws on a shape it did not expect fails the whole delivery, including every + // handler after it. Each of these is a field this path reads. + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + const base = labeledPayload(); + for (const [name, payload] of [ + ["no repository", { ...base, repository: undefined }], + ["no issue", { ...base, issue: undefined }], + ["no installation", { ...base, installation: undefined }], + ] as const) { + await expect(run(env, payload, `d-missing-${name.replace(/\s/g, "-")}`), name).resolves.not.toThrow(); + } + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + }); + + it("handles an issue with no author and no labels without reading a permission", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload({ user: undefined, labels: undefined }), "d-bare-issue"); + + expect(calls.some((call) => call.url.includes("/permission"))).toBe(false); + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + }); + + it("records the enforcement even when the webhook carries no sender", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + const { sender: _sender, ...senderless } = labeledPayload(); + await run(env, senderless, "d-no-sender"); + + const audit = await env.DB.prepare("select actor from audit_events where event_type = ?") + .bind(PRIORITY_LABEL_ENFORCEMENT_EVENT) + .first<{ actor: string | null }>(); + expect(audit, "the strip is still recorded").toBeTruthy(); + expect(audit?.actor, "with no actor rather than a fabricated one").toBeNull(); + }); + + it("honours a repo's CUSTOM priority label name", async () => { + // The label is per-repo configurable; enforcing the default name on a repo that renamed it would both + // miss the real label and touch one the repo does not use for this. + const env = await seed(); + // typeLabels is config-as-code only (no DB column). The RESOLVER is stubbed rather than a manifest + // fixture written: what this asserts is that the handler reads the resolved label instead of the + // built-in default -- how a repo comes to have a custom one is resolveEffectiveSettings' own business, + // and it has its own tests. + const resolved = await repositorySettingsModule.resolveRepositorySettings(env, REPO); + vi.spyOn(repositorySettingsModule, "resolveRepositorySettings").mockResolvedValue({ + ...resolved, + typeLabels: { ...resolved.typeLabels, priority: "team:top" }, + }); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload({ labels: [{ name: "team:top" }] }), "d-custom-label"); + + const removed = calls.find((call) => call.method === "DELETE" && call.url.includes("/labels/")); + expect(decodeURIComponent(removed?.url ?? "")).toContain("team:top"); + }); + + it("tolerates a label entry with no name", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload({ labels: [{}, { name: "gittensor:priority" }] }), "d-nameless-label"); + + expect(calls.some((call) => call.method === "DELETE"), "the real label is still found and removed").toBe(true); + }); + + it("tolerates an author object with no login", async () => { + const env = await seed(); + const calls: Call[] = []; + stubGitHub("read", calls); + + await run(env, labeledPayload({ user: {} }), "d-nameless-author"); + + expect(calls.some((call) => call.url.includes("/permission")), "no author means no permission read").toBe(false); + expect(calls.some((call) => call.method === "DELETE")).toBe(false); + }); +});