Skip to content

Commit 68fadf5

Browse files
authored
pipeline(labels): strip gittensor:priority from issues a maintainer did not author (#9737) (#9862)
* 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. * 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. * 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.
1 parent 39239df commit 68fadf5

8 files changed

Lines changed: 702 additions & 54 deletions

File tree

apps/loopover-miner-ui/src/routeTree.gen.ts

Lines changed: 54 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -9,47 +9,47 @@
99
// Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified.
1010

1111
import { Route as rootRouteImport } from './routes/__root'
12-
import { Route as RunHistoryRouteImport } from './routes/run-history'
13-
import { Route as RankedCandidatesRouteImport } from './routes/ranked-candidates'
14-
import { Route as PortfolioRouteImport } from './routes/portfolio'
15-
import { Route as LedgersRouteImport } from './routes/ledgers'
16-
import { Route as EarningsRouteImport } from './routes/earnings'
17-
import { Route as AttemptsRouteImport } from './routes/attempts'
1812
import { Route as IndexRouteImport } from './routes/index'
13+
import { Route as AttemptsRouteImport } from './routes/attempts'
14+
import { Route as EarningsRouteImport } from './routes/earnings'
15+
import { Route as LedgersRouteImport } from './routes/ledgers'
16+
import { Route as PortfolioRouteImport } from './routes/portfolio'
17+
import { Route as RankedCandidatesRouteImport } from './routes/ranked-candidates'
18+
import { Route as RunHistoryRouteImport } from './routes/run-history'
1919

20-
const RunHistoryRoute = RunHistoryRouteImport.update({
21-
id: '/run-history',
22-
path: '/run-history',
20+
const IndexRoute = IndexRouteImport.update({
21+
id: '/',
22+
path: '/',
2323
getParentRoute: () => rootRouteImport,
2424
} as any)
25-
const RankedCandidatesRoute = RankedCandidatesRouteImport.update({
26-
id: '/ranked-candidates',
27-
path: '/ranked-candidates',
25+
const AttemptsRoute = AttemptsRouteImport.update({
26+
id: '/attempts',
27+
path: '/attempts',
2828
getParentRoute: () => rootRouteImport,
2929
} as any)
30-
const PortfolioRoute = PortfolioRouteImport.update({
31-
id: '/portfolio',
32-
path: '/portfolio',
30+
const EarningsRoute = EarningsRouteImport.update({
31+
id: '/earnings',
32+
path: '/earnings',
3333
getParentRoute: () => rootRouteImport,
3434
} as any)
3535
const LedgersRoute = LedgersRouteImport.update({
3636
id: '/ledgers',
3737
path: '/ledgers',
3838
getParentRoute: () => rootRouteImport,
3939
} as any)
40-
const EarningsRoute = EarningsRouteImport.update({
41-
id: '/earnings',
42-
path: '/earnings',
40+
const PortfolioRoute = PortfolioRouteImport.update({
41+
id: '/portfolio',
42+
path: '/portfolio',
4343
getParentRoute: () => rootRouteImport,
4444
} as any)
45-
const AttemptsRoute = AttemptsRouteImport.update({
46-
id: '/attempts',
47-
path: '/attempts',
45+
const RankedCandidatesRoute = RankedCandidatesRouteImport.update({
46+
id: '/ranked-candidates',
47+
path: '/ranked-candidates',
4848
getParentRoute: () => rootRouteImport,
4949
} as any)
50-
const IndexRoute = IndexRouteImport.update({
51-
id: '/',
52-
path: '/',
50+
const RunHistoryRoute = RunHistoryRouteImport.update({
51+
id: '/run-history',
52+
path: '/run-history',
5353
getParentRoute: () => rootRouteImport,
5454
} as any)
5555

@@ -123,25 +123,25 @@ export interface RootRouteChildren {
123123

124124
declare module '@tanstack/react-router' {
125125
interface FileRoutesByPath {
126-
'/run-history': {
127-
id: '/run-history'
128-
path: '/run-history'
129-
fullPath: '/run-history'
130-
preLoaderRoute: typeof RunHistoryRouteImport
126+
'/': {
127+
id: '/'
128+
path: '/'
129+
fullPath: '/'
130+
preLoaderRoute: typeof IndexRouteImport
131131
parentRoute: typeof rootRouteImport
132132
}
133-
'/ranked-candidates': {
134-
id: '/ranked-candidates'
135-
path: '/ranked-candidates'
136-
fullPath: '/ranked-candidates'
137-
preLoaderRoute: typeof RankedCandidatesRouteImport
133+
'/attempts': {
134+
id: '/attempts'
135+
path: '/attempts'
136+
fullPath: '/attempts'
137+
preLoaderRoute: typeof AttemptsRouteImport
138138
parentRoute: typeof rootRouteImport
139139
}
140-
'/portfolio': {
141-
id: '/portfolio'
142-
path: '/portfolio'
143-
fullPath: '/portfolio'
144-
preLoaderRoute: typeof PortfolioRouteImport
140+
'/earnings': {
141+
id: '/earnings'
142+
path: '/earnings'
143+
fullPath: '/earnings'
144+
preLoaderRoute: typeof EarningsRouteImport
145145
parentRoute: typeof rootRouteImport
146146
}
147147
'/ledgers': {
@@ -151,25 +151,25 @@ declare module '@tanstack/react-router' {
151151
preLoaderRoute: typeof LedgersRouteImport
152152
parentRoute: typeof rootRouteImport
153153
}
154-
'/earnings': {
155-
id: '/earnings'
156-
path: '/earnings'
157-
fullPath: '/earnings'
158-
preLoaderRoute: typeof EarningsRouteImport
154+
'/portfolio': {
155+
id: '/portfolio'
156+
path: '/portfolio'
157+
fullPath: '/portfolio'
158+
preLoaderRoute: typeof PortfolioRouteImport
159159
parentRoute: typeof rootRouteImport
160160
}
161-
'/attempts': {
162-
id: '/attempts'
163-
path: '/attempts'
164-
fullPath: '/attempts'
165-
preLoaderRoute: typeof AttemptsRouteImport
161+
'/ranked-candidates': {
162+
id: '/ranked-candidates'
163+
path: '/ranked-candidates'
164+
fullPath: '/ranked-candidates'
165+
preLoaderRoute: typeof RankedCandidatesRouteImport
166166
parentRoute: typeof rootRouteImport
167167
}
168-
'/': {
169-
id: '/'
170-
path: '/'
171-
fullPath: '/'
172-
preLoaderRoute: typeof IndexRouteImport
168+
'/run-history': {
169+
id: '/run-history'
170+
path: '/run-history'
171+
fullPath: '/run-history'
172+
preLoaderRoute: typeof RunHistoryRouteImport
173173
parentRoute: typeof rootRouteImport
174174
}
175175
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
title: Label policy — which labels carry scoring weight, and the rules for each
3+
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.
4+
eyebrow: Contributors
5+
---
6+
7+
Three labels categorise an issue, and they are not equal: `gittensor:priority` carries the
8+
highest scoring multiplier. That makes it the one label whose application has to be
9+
constrained by a **rule** rather than by judgement — otherwise the highest-value label is
10+
whatever anyone says it is.
11+
12+
Both rules below are enforced by the pipeline, on every label event. Neither is a
13+
discretionary decision, neither is applied to some contributors and not others, and each
14+
enforcement is written to the decision ledger with its rule id, so the history is
15+
independently checkable.
16+
17+
## The scoring labels
18+
19+
| Label | Meaning | Who may apply it |
20+
| --- | --- | --- |
21+
| `gittensor:bug` | A fix, test, doc, chore, refactor, perf, ci, build or style change | Anyone |
22+
| `gittensor:feature` | Genuinely new functionality | Anyone |
23+
| `gittensor:priority` | Work the maintainer originated and triaged as most valuable | **Only valid on maintainer-authored issues** |
24+
25+
## Rule 1 — priority is only valid on maintainer-authored issues
26+
27+
`gittensor:priority` marks work **the maintainer originated**. If it is applied to an issue
28+
somebody else authored — by anyone, including a maintainer — the pipeline removes it and
29+
posts a single comment linking here.
30+
31+
**Maintainer of record** is read from the repository's own permissions (`admin` or
32+
`maintain`), never a hardcoded list of names, so the rule means the same thing on every repo
33+
LoopOver manages. `write` access is deliberately *not* enough: handing out push access would
34+
otherwise widen who can mint the highest multiplier.
35+
36+
The rule **fails open**. If the author's permission cannot be read, nothing is stripped — a
37+
label is never removed on the strength of a failed lookup, and the issue is re-judged next
38+
time it is labelled.
39+
40+
Removing the label is not a judgement about the issue. It stays open, and contributions to
41+
it remain welcome under its other labels.
42+
43+
Rule id: `priority-label-author-eligibility`.
44+
45+
## Rule 2 — priority issues open for work after a short window
46+
47+
Priority issues carry the highest payout, so assignment fairness matters most there. First-come
48+
pickup is only fair if everyone can *see* the issue before anyone can act on it: a PR opened
49+
moments after the label lands means the window between "issue becomes valuable" and "issue is
50+
claimed" was effectively zero for everyone else watching the repo.
51+
52+
So a PR closing a priority issue becomes gate-eligible once the label has been publicly present
53+
for a short window — **30 minutes by default**, configurable per repo via
54+
`gate.priorityEligibilityWindow`, and `0` turns it off.
55+
56+
A PR opened inside that window is **not rejected**. It is held, with a comment naming the exact
57+
moment it becomes eligible, and it proceeds normally once the window passes. There is no penalty
58+
beyond waiting and nothing to resubmit.
59+
60+
The clock is anchored to the **earliest** time the label was applied, so re-applying the label
61+
never resets the window for anyone, and "when does this issue open for work" is a single instant
62+
that cannot move.
63+
64+
Rule id: `priority-eligibility-window`.
65+
66+
## What is not in scope
67+
68+
The multiplier *values* themselves are registry-side and are not set here. These two rules govern
69+
which issues may carry the label and when work on them opens — not what the label is worth.

apps/loopover-ui/src/components/site/docs-nav.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ export const docsNav: DocsGroup[] = [
104104
{ to: "/docs/loopover-commands", label: "@loopover commands" },
105105
{ to: "/docs/branch-analysis", label: "Branch analysis" },
106106
{ to: "/docs/scoreability", label: "Scoreability" },
107+
{ to: "/docs/label-policy", label: "Label policy" },
107108
{ to: "/docs/upstream-drift", label: "Upstream drift" },
108109
{ to: "/docs/backtest-calibration", label: "Backtest & calibration" },
109110
{ to: "/docs/verify-this-review", label: "Verify this review" },

src/github/comments.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { withInstallationTokenRetry } from "./app";
22
import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client";
3+
import { PRIORITY_LABEL_COMMENT_MARKER } from "../review/priority-label-eligibility";
34
import type { AgentActionMode } from "../settings/agent-execution";
45

56
export const PR_PANEL_COMMENT_MARKER = "<!-- gittensory-pr-panel:v1 -->";
@@ -117,6 +118,19 @@ export async function createOrUpdateAgentCommandComment(
117118
return createOrUpdateIssueCommentWithMarker(env, installationId, repoFullName, issueNumber, body, AGENT_COMMAND_COMMENT_MARKER, { mode });
118119
}
119120

121+
/** #9737: the priority-label policy notice. Marked like its siblings so a RE-label updates the existing
122+
* comment instead of posting a second one -- the rule re-runs on every label event by design. */
123+
export async function createOrUpdatePriorityLabelPolicyComment(
124+
env: Env,
125+
installationId: number,
126+
repoFullName: string,
127+
issueNumber: number,
128+
body: string,
129+
mode: AgentActionMode = "live",
130+
): Promise<{ id: number; html_url?: string; changed: boolean } | null> {
131+
return createOrUpdateIssueCommentWithMarker(env, installationId, repoFullName, issueNumber, body, PRIORITY_LABEL_COMMENT_MARKER, { mode });
132+
}
133+
120134
// #6724 (review-burst): `changed` distinguishes a genuine no-op (the rendered body was byte-identical to what's
121135
// already posted, PATCH skipped -- see the idempotency comment below) from a real create/update, so a caller can
122136
// avoid double-counting a republish that produced no visible change. `false` ONLY on the proven-identical path;

src/queue/processors.ts

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ import { isSelfAuthoredCiCompletionWebhook } from "../github/self-authored";
152152
import {
153153
AGENT_COMMAND_COMMENT_MARKER,
154154
createOrUpdateAgentCommandComment,
155+
createOrUpdatePriorityLabelPolicyComment,
155156
createOrUpdatePrIntelligenceComment,
156157
createOrUpdateVisualFollowupComment,
157158
PR_PANEL_COMMENT_MARKER,
@@ -192,6 +193,12 @@ import {
192193
type PullRequestFreshness,
193194
} from "../github/pr-freshness";
194195
import { DEFAULT_TYPE_LABELS, resolvePrTypeLabel } from "../settings/pr-type-label";
196+
import {
197+
PRIORITY_LABEL_AUTHOR_RULE_ID,
198+
PRIORITY_LABEL_ENFORCEMENT_EVENT,
199+
PRIORITY_LABEL_POLICY_URL,
200+
resolvePriorityLabelEnforcement,
201+
} from "../review/priority-label-eligibility";
195202
import { fetchLinkedIssueLabelsForPropagation } from "../review/linked-issue-label-propagation-fetch";
196203
import { shouldPublishReviewCheck } from "../review/check-names";
197204
import { fetchPublicContributorProfile } from "../github/public";
@@ -6824,6 +6831,82 @@ async function maybeHandleReactionWebhookEvent(
68246831
return false;
68256832
}
68266833

6834+
6835+
/**
6836+
* `issues.labeled` -> author-based priority-label eligibility (#9737).
6837+
*
6838+
* `gittensor:priority` carries the highest scoring multiplier, so the one label whose application must be
6839+
* constrained by rule rather than judgment. Priority marks work the MAINTAINER originated and triaged, so
6840+
* on a contributor-authored issue it is stripped and the reason stated once, with a link to the policy.
6841+
*
6842+
* The DECISION is `resolvePriorityLabelEnforcement` (pure, unit-tested); this function is its I/O -- read
6843+
* the author's permission, strip, upsert the marked comment, record the enforcement event. Returns `true`
6844+
* when it claimed the event, matching every sibling handler here.
6845+
*
6846+
* FAIL-SAFE end to end: an unreadable permission yields no strip (the evaluator's own rule), and every
6847+
* GitHub call is caught so a label event can never fail the webhook. Re-labelling simply re-runs it, which
6848+
* is why the comment carries a marker and is updated rather than re-posted.
6849+
*/
6850+
async function maybeHandlePriorityLabelEligibility(
6851+
env: Env,
6852+
deliveryId: string,
6853+
eventName: string,
6854+
payload: GitHubWebhookPayload,
6855+
): Promise<boolean> {
6856+
if (eventName !== "issues" || payload.action !== "labeled") return false;
6857+
const repoFullName = payload.repository?.full_name;
6858+
const issue = payload.issue;
6859+
const installationId = payload.installation?.id;
6860+
if (!repoFullName || !issue || !installationId) return false;
6861+
// A pull request arrives on the `issues` event too; the evaluator refuses it, but skipping here saves a
6862+
// permission read on every PR label.
6863+
if (issue.pull_request !== undefined && issue.pull_request !== null) return false;
6864+
6865+
const settings = await resolveRepositorySettings(env, repoFullName).catch(() => undefined);
6866+
/* v8 ignore next 2 -- noUncheckedIndexedAccess fallback: PrTypeLabelSet is a Record<string, string>, so
6867+
DEFAULT_TYPE_LABELS.priority reads as possibly-undefined to the type system though it is always set. */
6868+
const priorityLabel: string = settings?.typeLabels?.priority ?? DEFAULT_TYPE_LABELS.priority ?? "gittensor:priority";
6869+
const labels = (issue.labels ?? []).map((label) => label?.name ?? "").filter((name) => name.length > 0);
6870+
// Only the labelled event for THIS label matters; anything else is another label's business.
6871+
if (!labels.some((name) => name.toLowerCase() === priorityLabel.toLowerCase())) return false;
6872+
6873+
const authorLogin = issue.user?.login ?? null;
6874+
const authorPermission = authorLogin
6875+
? await getRepositoryCollaboratorPermission(env, installationId, repoFullName, authorLogin).catch(() => null)
6876+
: null;
6877+
6878+
const { verdict, commentBody } = resolvePriorityLabelEnforcement({
6879+
priorityLabel,
6880+
labels,
6881+
authorLogin,
6882+
authorPermission,
6883+
isPullRequest: false,
6884+
policyUrl: PRIORITY_LABEL_POLICY_URL,
6885+
});
6886+
if (!verdict.strip || commentBody === null) return false;
6887+
6888+
await removePullRequestLabel(env, installationId, repoFullName, issue.number, priorityLabel).catch(() => undefined);
6889+
await createOrUpdatePriorityLabelPolicyComment(env, installationId, repoFullName, issue.number, commentBody).catch(() => undefined);
6890+
await recordAuditEvent(env, {
6891+
eventType: PRIORITY_LABEL_ENFORCEMENT_EVENT,
6892+
actor: payload.sender?.login ?? null,
6893+
targetKey: `${repoFullName}#${issue.number}`,
6894+
outcome: "success",
6895+
detail: verdict.reason,
6896+
metadata: { ruleId: PRIORITY_LABEL_AUTHOR_RULE_ID, issueAuthor: authorLogin, authorPermission, label: priorityLabel },
6897+
}).catch(() => undefined);
6898+
await recordWebhookEvent(env, {
6899+
deliveryId,
6900+
eventName,
6901+
action: payload.action,
6902+
installationId,
6903+
repositoryFullName: repoFullName,
6904+
payloadHash: "processed",
6905+
status: "processed",
6906+
}).catch(() => undefined);
6907+
return true;
6908+
}
6909+
68276910
/**
68286911
* Handles the `issue_comment` webhook event's command/mention dispatch chain — panel retrigger, panel
68296912
* generate-tests, gate-override, resolve/explain/generate-tests/review/pause/resume/configuration/plan
@@ -7866,6 +7949,11 @@ export async function processGitHubWebhook(
78667949

78677950
if (await maybeHandleReactionWebhookEvent(env, deliveryId, eventName, payload)) return;
78687951

7952+
// #9737: an `issues.labeled` event carrying the priority label is judged against the issue's AUTHOR
7953+
// before anything else looks at it -- the label is the scoring input, so the sooner an ineligible one
7954+
// is removed the less it can be acted on.
7955+
if (await maybeHandlePriorityLabelEligibility(env, deliveryId, eventName, payload)) return;
7956+
78697957
if (
78707958
await maybeHandleIssueCommentCommandWebhookEvent(env, deliveryId, eventName, payload)
78717959
)

0 commit comments

Comments
 (0)