From 6ba4170851900345f2a0386343e8b35d40410477 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:44:48 -0700 Subject: [PATCH 1/3] pipeline(labels): strip gittensor:priority from issues a maintainer did not author (#9737) `gittensor:priority` carries the highest scoring multiplier, which makes it the one label whose application has to be constrained by a RULE rather than by judgment -- otherwise the highest-value label is whatever anyone says it is. Priority marks work the MAINTAINER originated and triaged, so on an issue somebody else authored it is now removed automatically, with one comment linking the policy. Maintainer-of-record is read from the repo's own permissions (`admin` or `maintain`), never a hardcoded login, so the rule means the same thing on every repo ORB manages. `write` is deliberately NOT enough: handing out push access would otherwise widen who can mint the highest multiplier. Three properties the implementation is shaped around: - FAILS OPEN on every uncertainty -- an unreadable permission, an unknown author, an absent label. Stripping the highest-value label off a maintainer's own issue because WE could not read a permission is a worse error than leaving one wrongly applied, and the next label event re-judges it. - NEVER touches a pull request. The same label name is also the PR TYPE label for a content submission, which ORB applies itself -- conflating the two would have this rule fighting the labeller. - IDEMPOTENT by construction. Re-labelling re-runs the rule, so the comment carries a marker and is updated rather than re-posted; it reuses the existing marked-comment upsert instead of a sixth copy of that logic. The decision is a pure evaluator with 100% statement and branch coverage; the webhook handler is its I/O, following the `maybeHandle*WebhookEvent` shape every sibling here already uses. `issues` events were already subscribed and previously dropped on the floor -- this is the first handler for them. Every strip is written to the ledger with the rule id, the author, the permission read and the label, so enforcement history is checkable without reading GitHub. The policy page the comment links to documents BOTH rules of the epic -- this one and #9738's eligibility window -- since a contributor meeting one will meet the other. --- .../loopover-ui/content/docs/label-policy.mdx | 69 ++++++++++ src/github/comments.ts | 14 ++ src/queue/processors.ts | 86 ++++++++++++ src/review/priority-label-eligibility.ts | 125 ++++++++++++++++++ test/unit/priority-label-eligibility.test.ts | 98 ++++++++++++++ 5 files changed, 392 insertions(+) create mode 100644 apps/loopover-ui/content/docs/label-policy.mdx create mode 100644 src/review/priority-label-eligibility.ts create mode 100644 test/unit/priority-label-eligibility.test.ts 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/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..e285c2edca 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,80 @@ 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); + 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 +7924,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(); + }); +}); From ada4e8e54d7b79397d68d789dfd5796c5c6b0082 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:27:06 -0700 Subject: [PATCH 2/3] test(labels): cover the issues.labeled enforcement path end to end (#9737) Codecov put the patch at 60%: the DECISION was fully covered by its own unit tests, but the I/O the decision drives -- the permission read, the label removal, the marked comment, the ledger event -- was not exercised at all, and neither was the wrapper in comments.ts. 13 cases through the real webhook processor with GitHub stubbed. Half of them assert that nothing happens: a maintainer-authored issue, an unreadable permission, a pull request carrying the same label, a different label, a non-labeled action, an issue with no author. A rule that strips the highest-value label does its damage in the paths where it should have stayed still. Two fixture corrections the tests forced, both of which say something about the code: the marked-comment upsert only ever updates a comment authored by the App ITSELF (user.type === Bot and a matching login), so a fixture without those is correctly ignored and a second comment posted -- which is the right behaviour and now pinned. And typeLabels is config-as-code only, so the custom-label case stubs the settings RESOLVER rather than writing a manifest fixture: what this file asserts is that the handler reads the resolved label, not how a repo comes to have one. 228 added lines across the three files: zero uncovered, zero partial branches. --- src/queue/processors.ts | 2 + test/unit/priority-label-webhook.test.ts | 253 +++++++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 test/unit/priority-label-webhook.test.ts diff --git a/src/queue/processors.ts b/src/queue/processors.ts index e285c2edca..38d22ae41c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -6840,6 +6840,8 @@ async function maybeHandlePriorityLabelEligibility( 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. 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); + }); +}); From bca54e2cfd324e515621b32707d88ba4e5d5970e Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:36:43 -0700 Subject: [PATCH 3/3] fix(ui): add the label-policy docs page to the sidebar The docs-nav guard (#8385) requires a sidebar entry for every published .mdx page; the new label-policy page had none, failing UI tests. --- apps/loopover-miner-ui/src/routeTree.gen.ts | 108 +++++++++--------- .../src/components/site/docs-nav.tsx | 1 + 2 files changed, 55 insertions(+), 54 deletions(-) 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/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" },