Skip to content

Commit 83d67e0

Browse files
committed
gate(eligibility): hold PRs against a priority issue until its window opens (#9738)
Priority issues carry the highest payout, so assignment fairness matters most there -- and 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. A PR closing a `gittensor:priority` issue is now gate-eligible only once the label has been publicly present for `gate.priorityEligibilityWindow` minutes (default 30, per repo, `0` disables it). A PR arriving inside the window is NOT rejected: it is HELD with a neutral comment naming the moment it becomes eligible, and proceeds normally once the window elapses. No penalty beyond waiting. Two decisions worth stating: - The clock is anchored to the EARLIEST labeling event, not the most recent. The spec requires that re-applying the label not reset the window for already-open PRs; an earliest anchor gives that to everyone and makes "when does this issue open for work" a single knowable instant nothing can move. - Every unknown FAILS OPEN -- an unreadable timestamp, an absent label, a GraphQL error, a missing token. Holding a contributor's PR on a fact we could not read is a penalty for our own gap. The hold reuses the existing merge-hold rail (`heldForManualReview`), so it can never close a PR. Merge holds become DATA rather than a widening list of booleans. `MERGE_HOLD_INPUTS` is the one table; the input type (`Record<MergeHoldInput, boolean>`), the `heldForManualReview` fold, and both test fixtures all derive from it. Adding a hold was three independent edits that could each be forgotten -- declaring one and not folding it into the decision compiled fine. It is now one entry, and omitting the wiring is a compile error at the single call site. `normalizeOptionalNonNegativeInteger` exists because `0` is meaningful here (it turns the rule off) and the positive-integer normalizer would have discarded it as invalid and silently applied the default -- an operator's explicit "off" becoming "on". 100% statements and branches on both changed source files.
1 parent 39239df commit 83d67e0

16 files changed

Lines changed: 518 additions & 27 deletions

File tree

.loopover.yml.example

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,22 @@ gate:
407407
# DB-backed (dashboard-settable too); this overrides the stored value.
408408
requireFreshRebaseWindow: 10
409409

410+
# Priority-issue eligibility window (#9738) — minutes a `gittensor:priority` label must have been publicly
411+
# present before a PR that closes that issue is gate-eligible.
412+
#
413+
# Priority carries the highest payout, so assignment fairness matters most there, and first-come pickup is
414+
# only fair if everyone can SEE the issue before anyone can act on it: a PR opened moments after the label
415+
# lands means the window between "issue becomes valuable" and "issue is claimed" was effectively zero for
416+
# everyone else watching the repo.
417+
#
418+
# A PR arriving inside the window is NOT rejected. It is HELD, with a neutral comment naming the moment it
419+
# becomes eligible, and proceeds normally once the window elapses — no penalty beyond waiting, and the
420+
# contributor keeps their work. The clock is anchored to the EARLIEST time the label was applied, so
421+
# re-applying it never resets the window for anyone.
422+
#
423+
# Whole number of minutes, 0 to 1440. `0` turns the rule off. Default: 30. Config-as-code only.
424+
priorityEligibilityWindow: 30
425+
410426
# Stale-base auto-rebase threshold. When the repository's current default branch is at least this many
411427
# commits ahead of a PR's own base commit, the pre-review readiness gate forces an update_branch before
412428
# review runs — independent of GitHub's own mergeable_state "behind" signal, which only fires when the

apps/loopover-ui/public/openapi.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10191,6 +10191,12 @@
1019110191
"guardrailEscalationSelfConsistencyRuns": {
1019210192
"type": "number",
1019310193
"nullable": true
10194+
},
10195+
"priorityEligibilityWindowMinutes": {
10196+
"type": "integer",
10197+
"nullable": true,
10198+
"minimum": 0,
10199+
"maximum": 1440
1019410200
}
1019510201
},
1019610202
"required": [

config/examples/loopover.full.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,22 @@ gate:
421421
# DB-backed (dashboard-settable too); this overrides the stored value.
422422
requireFreshRebaseWindow: 10
423423

424+
# Priority-issue eligibility window (#9738) — minutes a `gittensor:priority` label must have been publicly
425+
# present before a PR that closes that issue is gate-eligible.
426+
#
427+
# Priority carries the highest payout, so assignment fairness matters most there, and first-come pickup is
428+
# only fair if everyone can SEE the issue before anyone can act on it: a PR opened moments after the label
429+
# lands means the window between "issue becomes valuable" and "issue is claimed" was effectively zero for
430+
# everyone else watching the repo.
431+
#
432+
# A PR arriving inside the window is NOT rejected. It is HELD, with a neutral comment naming the moment it
433+
# becomes eligible, and proceeds normally once the window elapses — no penalty beyond waiting, and the
434+
# contributor keeps their work. The clock is anchored to the EARLIEST time the label was applied, so
435+
# re-applying it never resets the window for anyone.
436+
#
437+
# Whole number of minutes, 0 to 1440. `0` turns the rule off. Default: 30. Config-as-code only.
438+
priorityEligibilityWindow: 30
439+
424440
# Stale-base auto-rebase threshold. When the repository's current default branch is at least this many
425441
# commits ahead of a PR's own base commit, the pre-review readiness gate forces an update_branch before
426442
# review runs — independent of GitHub's own mergeable_state "behind" signal, which only fires when the

packages/loopover-engine/src/focus-manifest.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,12 @@ export type FocusManifestGateConfig = {
209209
* (byte-identical to today) — a discrete positive-minutes count, not a score, so it is neither clamped
210210
* nor rounded; an invalid value (fractional, non-positive, non-finite) is dropped with a warning. */
211211
requireFreshRebaseWindowMinutes: number | null;
212+
/** `gate.priorityEligibilityWindow` (#9738): minutes a `gittensor:priority` label must have been publicly
213+
* present before a PR closing that issue is gate-eligible. Priority issues carry the highest payout, so
214+
* first-come pickup is only fair if everyone can see the issue before anyone can act on it. A PR inside
215+
* the window is HELD with a neutral comment, never rejected, and proceeds once the window elapses.
216+
* `0` disables the rule; null means the shipped default (30). */
217+
priorityEligibilityWindowMinutes: number | null;
212218
/** `gate.staleBaseAheadByThreshold` (#review-grounding stale-base fact): a commit count. When the repo's
213219
* current default branch is at least this many commits ahead of a PR's own base commit, the pre-review
214220
* readiness gate forces an `update_branch` (same action class as the existing `mergeableState: "behind"`
@@ -1386,6 +1392,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = {
13861392
dryRun: null,
13871393
premergeContentRecheck: null,
13881394
requireFreshRebaseWindowMinutes: null,
1395+
priorityEligibilityWindowMinutes: null,
13891396
staleBaseAheadByThreshold: null,
13901397
claMode: null,
13911398
claConsentPhrase: null,
@@ -1837,6 +1844,7 @@ const GATE_TOP_LEVEL_KEYS = new Set<string>([
18371844
"dryRun",
18381845
"premergeContentRecheck",
18391846
"requireFreshRebaseWindow",
1847+
"priorityEligibilityWindow",
18401848
"staleBaseAheadByThreshold",
18411849
"claMode",
18421850
"cla",
@@ -1941,6 +1949,8 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
19411949
dryRun: normalizeOptionalBoolean(record.dryRun, "gate.dryRun", warnings),
19421950
premergeContentRecheck: normalizeOptionalBoolean(record.premergeContentRecheck, "gate.premergeContentRecheck", warnings),
19431951
requireFreshRebaseWindowMinutes: normalizeOptionalPositiveInteger(record.requireFreshRebaseWindow, "gate.requireFreshRebaseWindow", warnings),
1952+
// Zero is MEANINGFUL here (it turns the rule off), so this cannot use normalizeOptionalPositiveInteger.
1953+
priorityEligibilityWindowMinutes: normalizeOptionalNonNegativeInteger(record.priorityEligibilityWindow, "gate.priorityEligibilityWindow", warnings, MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES),
19441954
staleBaseAheadByThreshold: normalizeOptionalPositiveInteger(record.staleBaseAheadByThreshold, "gate.staleBaseAheadByThreshold", warnings),
19451955
claMode: normalizeOptionalGateMode(record.claMode, "gate.claMode", warnings),
19461956
claConsentPhrase: parsePublicSafeText(claRecord?.consentPhrase, "gate.cla.consentPhrase", warnings),
@@ -2004,6 +2014,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
20042014
gate.dryRun !== null ||
20052015
gate.premergeContentRecheck !== null ||
20062016
gate.requireFreshRebaseWindowMinutes !== null ||
2017+
gate.priorityEligibilityWindowMinutes !== null ||
20072018
gate.staleBaseAheadByThreshold !== null ||
20082019
gate.claMode !== null ||
20092020
gate.claConsentPhrase !== null ||
@@ -2221,6 +2232,17 @@ export function experimentalConfigToJson(experimental: FocusManifestExperimental
22212232
return out;
22222233
}
22232234

2235+
/** A NON-NEGATIVE integer, for a field where zero is a real setting rather than an absence -- e.g.
2236+
* `gate.priorityEligibilityWindow: 0` deliberately turns the window off, which `normalizeOptionalPositiveInteger`
2237+
* below would reject as invalid and silently replace with the shipped default (#9738). Bounded above so a typo
2238+
* cannot set a window that never opens. */
2239+
function normalizeOptionalNonNegativeInteger(value: JsonValue | undefined, field: string, warnings: string[], max: number): number | null {
2240+
if (value === undefined || value === null) return null;
2241+
if (typeof value === "number" && Number.isInteger(value) && value >= 0 && value <= max) return value;
2242+
warnings.push(`Manifest field "${field}" must be a whole number between 0 and ${max}; ignoring it.`);
2243+
return null;
2244+
}
2245+
22242246
/** A positive INTEGER count (not a score/confidence) — e.g. `contentLane.maxAppendedEntries` counts discrete
22252247
* surfaces[] entries, so a fractional value (a likely typo) would render a nonsensical contributor-facing close
22262248
* message ("append between 1 and 2.5 entries"). Rejects fractional and non-positive values alike. */
@@ -2233,6 +2255,9 @@ function normalizeOptionalPositiveInteger(value: JsonValue | undefined, field: s
22332255

22342256
const MAX_CONTRIBUTOR_OPEN_ITEM_CAP = 100;
22352257

2258+
/** A day. Long enough for any deliberate cool-off, short enough that a typo cannot park work indefinitely. */
2259+
const MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES = 24 * 60;
2260+
22362261
function normalizeOptionalContributorOpenItemCap(value: JsonValue | undefined, field: string, warnings: string[]): number | null {
22372262
const parsed = normalizeOptionalPositiveInteger(value, field, warnings);
22382263
if (parsed === null) return null;

scripts/check-docs-drift.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ export const SETTINGS_ALIAS_MANIFEST: AliasManifestRow[] = [
190190
{ field: "aiReviewOnMerge", aliases: ["onMerge"] },
191191
{ field: "aiReviewReviewers", aliases: ["reviewers:"] },
192192
{ field: "requireFreshRebaseWindowMinutes", aliases: ["requireFreshRebaseWindow"] },
193+
{ field: "priorityEligibilityWindowMinutes", aliases: ["priorityEligibilityWindow"] },
193194
{ field: "sizeGateMaxFiles", aliases: ["maxFiles"] },
194195
{ field: "sizeGateMaxLines", aliases: ["maxLines"] },
195196
];

src/github/backfill.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3559,6 +3559,46 @@ export async function fetchLivePullRequestMergedAt(
35593559
return result === undefined ? undefined : (result.data.merged_at ?? null);
35603560
}
35613561

3562+
/**
3563+
* When a label FIRST landed on an issue, ISO-8601, or null when it never did / cannot be read (#9738).
3564+
*
3565+
* `first: N` on purpose: the eligibility window is anchored to the EARLIEST labeling so re-applying the label
3566+
* cannot reset the clock for anyone. GitHub returns timeline items chronologically, so the first LABELED_EVENT
3567+
* naming this label is the moment the issue became publicly valuable.
3568+
*
3569+
* Reads at most one page. A maintainer who has labeled and unlabeled an issue more times than that has an
3570+
* issue whose history is not what this rule is for, and a null here FAILS OPEN (no hold) rather than guessing.
3571+
*/
3572+
export async function fetchIssueLabelFirstAppliedAt(
3573+
env: Env,
3574+
repoFullName: string,
3575+
issueNumber: number,
3576+
labelName: string,
3577+
token: string | undefined,
3578+
admissionKey?: GitHubRateLimitAdmissionKey,
3579+
): Promise<string | null> {
3580+
if (!token || !labelName) return null;
3581+
const { owner, name } = repoParts(repoFullName);
3582+
if (!owner || !name) return null;
3583+
const query = `query LoopOverIssueLabeledAt { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { issue(number: ${issueNumber}) { timelineItems(first: 100, itemTypes: [LABELED_EVENT]) { nodes { __typename ... on LabeledEvent { createdAt label { name } } } } } } }`;
3584+
const result = await githubGraphQl<{
3585+
data?: {
3586+
repository?: {
3587+
issue?: { timelineItems?: { nodes?: Array<{ createdAt?: string | null; label?: { name?: string | null } | null } | null> | null } | null } | null;
3588+
} | null;
3589+
};
3590+
errors?: unknown[];
3591+
}>(env, query, token, admissionKey).catch(() => undefined);
3592+
if (result === undefined) return null;
3593+
if (Array.isArray(result.errors) && result.errors.length > 0) return null;
3594+
const wanted = labelName.toLowerCase();
3595+
for (const node of result.data?.repository?.issue?.timelineItems?.nodes ?? []) {
3596+
const label = node?.label?.name;
3597+
if (typeof label === "string" && label.toLowerCase() === wanted && typeof node?.createdAt === "string") return node.createdAt;
3598+
}
3599+
return null;
3600+
}
3601+
35623602
export type LinkedIssueClosureByPullRequestResult = "closed_by_pull_request" | "not_closed_by_pull_request" | "fetch_error";
35633603

35643604
/** Verifies whether GitHub attributes this issue's closure to the specific PR, via GraphQL's `ClosedEvent.closer`

src/openapi/schemas.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { AGENT_ACTION_CLASSES, AUTONOMY_LEVELS, FEASIBILITY_VERDICTS, PUBLIC_SUR
44
// #9773: the request bodies these routes really accept, from the one place they are defined.
55
import { checkBeforeStartSchema, slopRiskSchema, validateFocusManifestSchema, validateLinkedIssueSchema } from "@loopover/contract/api-requests";
66
import { MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../settings/agent-actions";
7+
import { MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES } from "../review/priority-eligibility-window";
78
import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP } from "../types";
89
import {
910
MAX_FIND_OPPORTUNITIES_TARGETS,
@@ -759,6 +760,8 @@ export const RepositorySettingsSchema = z
759760
gateDryRun: z.boolean().optional(),
760761
premergeContentRecheck: z.boolean().optional(),
761762
requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(),
763+
// #9738: non-negative, not positive -- 0 is the documented way to turn the window off.
764+
priorityEligibilityWindowMinutes: z.number().int().min(0).max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES).nullable().optional(),
762765
staleBaseAheadByThreshold: z.number().int().positive().nullable().optional(),
763766
mergeReadinessGateMode: z.enum(["off", "advisory", "block"]),
764767
manifestPolicyGateMode: z.enum(["off", "advisory", "block"]),

src/queue/processors.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ import {
106106
backfillRepositorySegment,
107107
fetchAndStorePullRequestFilesForReview,
108108
fetchBaseAheadBy,
109+
fetchIssueLabelFirstAppliedAt,
109110
fetchLinkedIssueFacts,
110111
fetchLiveBaseBranchAdvancedAt,
111112
invalidateCiStateCache,
@@ -192,6 +193,7 @@ import {
192193
type PullRequestFreshness,
193194
} from "../github/pr-freshness";
194195
import { DEFAULT_TYPE_LABELS, resolvePrTypeLabel } from "../settings/pr-type-label";
196+
import { DEFAULT_PRIORITY_ELIGIBILITY_WINDOW_MINUTES, resolvePriorityEligibilityHold } from "../review/priority-eligibility-window";
195197
import { fetchLinkedIssueLabelsForPropagation } from "../review/linked-issue-label-propagation-fetch";
196198
import { shouldPublishReviewCheck } from "../review/check-names";
197199
import { fetchPublicContributorProfile } from "../github/public";
@@ -2795,6 +2797,7 @@ function buildAgentMaintenancePlanInput(args: {
27952797
linkedIssueRulesConfig: Awaited<ReturnType<typeof loadLinkedIssueHardRules>>;
27962798
migrationCollisionHold: AgentActionPlanInput["migrationCollisionHold"];
27972799
unlinkedIssueMatchHold: AgentActionPlanInput["unlinkedIssueMatchHold"];
2800+
priorityEligibilityHold: AgentActionPlanInput["priorityEligibilityHold"];
27982801
aiReviewLowConfidenceHold: AgentActionPlanInput["aiReviewLowConfidenceHold"];
27992802
unlinkedIssueMatchClose: AgentActionPlanInput["unlinkedIssueMatchClose"];
28002803
liveMergeState: string | undefined;
@@ -2834,6 +2837,7 @@ function buildAgentMaintenancePlanInput(args: {
28342837
linkedIssueRulesConfig,
28352838
migrationCollisionHold,
28362839
unlinkedIssueMatchHold,
2840+
priorityEligibilityHold,
28372841
aiReviewLowConfidenceHold,
28382842
unlinkedIssueMatchClose,
28392843
liveMergeState,
@@ -2912,6 +2916,7 @@ function buildAgentMaintenancePlanInput(args: {
29122916
},
29132917
...(migrationCollisionHold !== undefined ? { migrationCollisionHold } : {}),
29142918
...(unlinkedIssueMatchHold !== undefined ? { unlinkedIssueMatchHold } : {}),
2919+
...(priorityEligibilityHold !== undefined ? { priorityEligibilityHold } : {}),
29152920
...(aiReviewLowConfidenceHold !== undefined ? { aiReviewLowConfidenceHold } : {}),
29162921
...(unlinkedIssueMatchClose !== undefined ? { unlinkedIssueMatchClose } : {}),
29172922
manualReviewLockContentionResolved,
@@ -3452,6 +3457,24 @@ async function runAgentMaintenancePlanAndExecute(
34523457
})
34533458
: undefined;
34543459
const unlinkedIssueMatchHold = unlinkedIssueMatchDisposition?.kind === "hold" ? unlinkedIssueMatchDisposition : undefined;
3460+
3461+
// Priority-issue eligibility window (#9738). A PR closing a `gittensor:priority` issue is gate-eligible
3462+
// only once the label has been publicly present for the configured window, so first-come pickup is fair to
3463+
// everyone watching the repo rather than to whoever was already looking. Resolved here (not in the planner)
3464+
// because it needs a GitHub read; the DECISION is the pure evaluator, unit-tested on its own.
3465+
//
3466+
// FAIL-OPEN throughout: a fetch that throws, a label that was never applied, or a timestamp we cannot parse
3467+
// all yield no hold. Holding a contributor's PR on a fact we could not read is a penalty for our own gap.
3468+
const priorityEligibilityHold = await resolvePriorityEligibilityHold({
3469+
env,
3470+
repoFullName,
3471+
linkedIssues: pr.linkedIssues,
3472+
prCreatedAt: pr.createdAt ?? null,
3473+
windowMinutes: settings.priorityEligibilityWindowMinutes ?? DEFAULT_PRIORITY_ELIGIBILITY_WINDOW_MINUTES,
3474+
priorityLabel: settings.typeLabels?.priority ?? DEFAULT_TYPE_LABELS.priority,
3475+
token: ciToken,
3476+
fetchLabeledAt: (repo, issueNumber, label) => fetchIssueLabelFirstAppliedAt(env, repo, issueNumber, label, ciToken),
3477+
}).catch(() => undefined);
34553478
const unlinkedIssueMatchClose = unlinkedIssueMatchDisposition?.kind === "close" ? unlinkedIssueMatchDisposition : undefined;
34563479

34573480
// Contributor blacklist (#1425): resolve whether the PR author is on the repo's blacklist (the shared/global
@@ -3653,6 +3676,7 @@ async function runAgentMaintenancePlanAndExecute(
36533676
linkedIssueRulesConfig,
36543677
migrationCollisionHold,
36553678
unlinkedIssueMatchHold,
3679+
priorityEligibilityHold,
36563680
aiReviewLowConfidenceHold: aiReviewLowConfidenceHold ?? aiReviewSalvageableHold,
36573681
unlinkedIssueMatchClose,
36583682
liveMergeState,

0 commit comments

Comments
 (0)