Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .loopover.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,22 @@ gate:
# DB-backed (dashboard-settable too); this overrides the stored value.
requireFreshRebaseWindow: 10

# Priority-issue eligibility window (#9738) — minutes a `gittensor:priority` label must have been publicly
# present before a PR that closes that issue is gate-eligible.
#
# Priority carries 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 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, and the
# contributor keeps their work. The clock is anchored to the EARLIEST time the label was applied, so
# re-applying it never resets the window for anyone.
#
# Whole number of minutes, 0 to 1440. `0` turns the rule off. Default: 30. Config-as-code only.
priorityEligibilityWindow: 30

# Stale-base auto-rebase threshold. When the repository's current default branch is at least this many
# commits ahead of a PR's own base commit, the pre-review readiness gate forces an update_branch before
# review runs — independent of GitHub's own mergeable_state "behind" signal, which only fires when the
Expand Down
6 changes: 6 additions & 0 deletions apps/loopover-ui/public/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -10191,6 +10191,12 @@
"guardrailEscalationSelfConsistencyRuns": {
"type": "number",
"nullable": true
},
"priorityEligibilityWindowMinutes": {
"type": "integer",
"nullable": true,
"minimum": 0,
"maximum": 1440
}
},
"required": [
Expand Down
16 changes: 16 additions & 0 deletions config/examples/loopover.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,22 @@ gate:
# DB-backed (dashboard-settable too); this overrides the stored value.
requireFreshRebaseWindow: 10

# Priority-issue eligibility window (#9738) — minutes a `gittensor:priority` label must have been publicly
# present before a PR that closes that issue is gate-eligible.
#
# Priority carries 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 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, and the
# contributor keeps their work. The clock is anchored to the EARLIEST time the label was applied, so
# re-applying it never resets the window for anyone.
#
# Whole number of minutes, 0 to 1440. `0` turns the rule off. Default: 30. Config-as-code only.
priorityEligibilityWindow: 30

# Stale-base auto-rebase threshold. When the repository's current default branch is at least this many
# commits ahead of a PR's own base commit, the pre-review readiness gate forces an update_branch before
# review runs — independent of GitHub's own mergeable_state "behind" signal, which only fires when the
Expand Down
26 changes: 26 additions & 0 deletions packages/loopover-engine/src/focus-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,12 @@ export type FocusManifestGateConfig = {
* (byte-identical to today) — a discrete positive-minutes count, not a score, so it is neither clamped
* nor rounded; an invalid value (fractional, non-positive, non-finite) is dropped with a warning. */
requireFreshRebaseWindowMinutes: number | null;
/** `gate.priorityEligibilityWindow` (#9738): minutes a `gittensor:priority` label must have been publicly
* present before a PR closing that issue is gate-eligible. Priority issues carry the highest payout, so
* first-come pickup is only fair if everyone can see the issue before anyone can act on it. A PR inside
* the window is HELD with a neutral comment, never rejected, and proceeds once the window elapses.
* `0` disables the rule; null means the shipped default (30). */
priorityEligibilityWindowMinutes: number | null;
/** `gate.staleBaseAheadByThreshold` (#review-grounding stale-base fact): a commit count. When the repo's
* current default branch is at least this many commits ahead of a PR's own base commit, the pre-review
* readiness gate forces an `update_branch` (same action class as the existing `mergeableState: "behind"`
Expand Down Expand Up @@ -1386,6 +1392,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = {
dryRun: null,
premergeContentRecheck: null,
requireFreshRebaseWindowMinutes: null,
priorityEligibilityWindowMinutes: null,
staleBaseAheadByThreshold: null,
claMode: null,
claConsentPhrase: null,
Expand Down Expand Up @@ -1837,6 +1844,7 @@ const GATE_TOP_LEVEL_KEYS = new Set<string>([
"dryRun",
"premergeContentRecheck",
"requireFreshRebaseWindow",
"priorityEligibilityWindow",
"staleBaseAheadByThreshold",
"claMode",
"cla",
Expand Down Expand Up @@ -1941,6 +1949,8 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
dryRun: normalizeOptionalBoolean(record.dryRun, "gate.dryRun", warnings),
premergeContentRecheck: normalizeOptionalBoolean(record.premergeContentRecheck, "gate.premergeContentRecheck", warnings),
requireFreshRebaseWindowMinutes: normalizeOptionalPositiveInteger(record.requireFreshRebaseWindow, "gate.requireFreshRebaseWindow", warnings),
// Zero is MEANINGFUL here (it turns the rule off), so this cannot use normalizeOptionalPositiveInteger.
priorityEligibilityWindowMinutes: normalizeOptionalNonNegativeInteger(record.priorityEligibilityWindow, "gate.priorityEligibilityWindow", warnings, MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES),
staleBaseAheadByThreshold: normalizeOptionalPositiveInteger(record.staleBaseAheadByThreshold, "gate.staleBaseAheadByThreshold", warnings),
claMode: normalizeOptionalGateMode(record.claMode, "gate.claMode", warnings),
claConsentPhrase: parsePublicSafeText(claRecord?.consentPhrase, "gate.cla.consentPhrase", warnings),
Expand Down Expand Up @@ -2004,6 +2014,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu
gate.dryRun !== null ||
gate.premergeContentRecheck !== null ||
gate.requireFreshRebaseWindowMinutes !== null ||
gate.priorityEligibilityWindowMinutes !== null ||
gate.staleBaseAheadByThreshold !== null ||
gate.claMode !== null ||
gate.claConsentPhrase !== null ||
Expand Down Expand Up @@ -2122,6 +2133,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue {
if (gate.dryRun !== null) out.dryRun = gate.dryRun;
if (gate.premergeContentRecheck !== null) out.premergeContentRecheck = gate.premergeContentRecheck;
if (gate.requireFreshRebaseWindowMinutes !== null) out.requireFreshRebaseWindow = gate.requireFreshRebaseWindowMinutes;
if (gate.priorityEligibilityWindowMinutes !== null) out.priorityEligibilityWindow = gate.priorityEligibilityWindowMinutes;
if (gate.staleBaseAheadByThreshold !== null) out.staleBaseAheadByThreshold = gate.staleBaseAheadByThreshold;
if (gate.claMode !== null) out.claMode = gate.claMode;
if (gate.claConsentPhrase !== null || gate.claCheckRunName !== null || gate.claCheckRunAppSlug !== null) {
Expand Down Expand Up @@ -2221,6 +2233,17 @@ export function experimentalConfigToJson(experimental: FocusManifestExperimental
return out;
}

/** A NON-NEGATIVE integer, for a field where zero is a real setting rather than an absence -- e.g.
* `gate.priorityEligibilityWindow: 0` deliberately turns the window off, which `normalizeOptionalPositiveInteger`
* below would reject as invalid and silently replace with the shipped default (#9738). Bounded above so a typo
* cannot set a window that never opens. */
function normalizeOptionalNonNegativeInteger(value: JsonValue | undefined, field: string, warnings: string[], max: number): number | null {
if (value === undefined || value === null) return null;
if (typeof value === "number" && Number.isInteger(value) && value >= 0 && value <= max) return value;
warnings.push(`Manifest field "${field}" must be a whole number between 0 and ${max}; ignoring it.`);
return null;
}

/** A positive INTEGER count (not a score/confidence) — e.g. `contentLane.maxAppendedEntries` counts discrete
* surfaces[] entries, so a fractional value (a likely typo) would render a nonsensical contributor-facing close
* message ("append between 1 and 2.5 entries"). Rejects fractional and non-positive values alike. */
Expand All @@ -2233,6 +2256,9 @@ function normalizeOptionalPositiveInteger(value: JsonValue | undefined, field: s

const MAX_CONTRIBUTOR_OPEN_ITEM_CAP = 100;

/** A day. Long enough for any deliberate cool-off, short enough that a typo cannot park work indefinitely. */
const MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES = 24 * 60;

function normalizeOptionalContributorOpenItemCap(value: JsonValue | undefined, field: string, warnings: string[]): number | null {
const parsed = normalizeOptionalPositiveInteger(value, field, warnings);
if (parsed === null) return null;
Expand Down
4 changes: 4 additions & 0 deletions packages/loopover-engine/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,3 +952,7 @@ export {
type PlanProgress,
} from "./plan-dag.js";
export { isDone as isPlanStepDone, nextReadySteps } from "./plan-step-readiness.js";

// #9743: the priority label's single resolution, exported so the engine's OWN behaviour suite can test
// it -- the `engine` Codecov flag credits engine source from that suite, not from the root vitest run.
export { DEFAULT_PRIORITY_LABEL, resolvePriorityTypeLabel } from "./settings/pr-type-label.js";
22 changes: 21 additions & 1 deletion packages/loopover-engine/src/settings/pr-type-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,16 @@ export type { PrTypeLabelSet } from "../types/manifest-deps-types.js";
* assumption (#label-modularity): a self-hoster's `typeLabels` fully replaces the category set these
* keys are drawn from. The built-in categories are mutually exclusive by default (see
* `resolvePrTypeLabel`'s `removeLabels`) unless a propagation mapping is explicitly additive. */
/** The built-in PRIORITY label, named on its own so it reads as `string`. `PrTypeLabelSet` is an open
* `Record`, which makes `DEFAULT_TYPE_LABELS.priority` optional to the type system even though the
* built-in set always defines it -- naming the value is what lets `resolvePriorityTypeLabel` return it
* without a fallback branch nothing can ever take. */
export const DEFAULT_PRIORITY_LABEL = "gittensor:priority";

export const DEFAULT_TYPE_LABELS: PrTypeLabelSet = {
bug: "gittensor:bug",
feature: "gittensor:feature",
priority: "gittensor:priority",
priority: DEFAULT_PRIORITY_LABEL,
};

export const MAX_TYPE_LABEL_CATEGORIES = 32;
Expand Down Expand Up @@ -142,6 +148,20 @@ export type PrTypeLabelDecision = {
* misconfigured additive mapping's `prLabel` happens to collide with a type-label-set name (it is
* excluded from removal since it is also being applied). Pure + total.
*/
/**
* The PRIORITY label as this repo names it, falling back to the built-in default.
*
* One resolution rather than the ad-hoc `settings.typeLabels?.priority ?? DEFAULT_TYPE_LABELS.priority`
* each caller used to spell for itself: the label-author rule (#9737) and the eligibility window (#9738)
* both act on this exact label, and a repo that renamed it would be enforced inconsistently if the two
* ever disagreed about which label they mean.
*/
export function resolvePriorityTypeLabel(labels: PrTypeLabelSet | null | undefined): string {
const configured = labels?.priority;
if (typeof configured === "string" && configured.trim().length > 0) return configured;
return DEFAULT_PRIORITY_LABEL;
}

export function resolvePrTypeLabel(input: {
title: string | undefined;
linkedIssueLabels?: string[] | undefined;
Expand Down
37 changes: 37 additions & 0 deletions packages/loopover-engine/test/priority-type-label.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { DEFAULT_PRIORITY_LABEL, gateConfigToJson, parseFocusManifest, resolvePriorityTypeLabel } from "../dist/index.js";

// The engine's own behaviour suite for the two surfaces #9738/#9743 added here. The root vitest suite
// covers them too, but the `engine` Codecov flag is fed by THIS suite -- and, more to the point, these are
// engine semantics, so they belong beside the rest of the engine's behaviour tests.

test("resolvePriorityTypeLabel: a repo's configured name wins (#9743)", () => {
assert.equal(resolvePriorityTypeLabel({ bug: "b", feature: "f", priority: "team:top" }), "team:top");
});

test("resolvePriorityTypeLabel: falls back to the built-in default when unconfigured", () => {
assert.equal(resolvePriorityTypeLabel(undefined), DEFAULT_PRIORITY_LABEL);
assert.equal(resolvePriorityTypeLabel(null), DEFAULT_PRIORITY_LABEL);
assert.equal(resolvePriorityTypeLabel({}), DEFAULT_PRIORITY_LABEL);
});

test("resolvePriorityTypeLabel: a blank configured name is unconfigured, not an empty label", () => {
// An empty label would match nothing and silently disable both rules that key on it.
for (const priority of ["", " "]) {
assert.equal(resolvePriorityTypeLabel({ bug: "b", feature: "f", priority }), DEFAULT_PRIORITY_LABEL);
}
});

test("gateConfigToJson round-trips priorityEligibilityWindow (#9738)", () => {
// The setting used to parse but never serialize, so it was silently dropped on the way back out.
const parsed = parseFocusManifest({ gate: { priorityEligibilityWindow: 45 } });
assert.equal(parsed.gate.priorityEligibilityWindowMinutes, 45);
assert.equal((gateConfigToJson(parsed.gate) as { priorityEligibilityWindow?: number }).priorityEligibilityWindow, 45);
});

test("gateConfigToJson: 0 is an explicit OFF and must survive the round trip", () => {
const parsed = parseFocusManifest({ gate: { priorityEligibilityWindow: 0 } });
assert.equal(parsed.gate.priorityEligibilityWindowMinutes, 0);
assert.equal((gateConfigToJson(parsed.gate) as { priorityEligibilityWindow?: number }).priorityEligibilityWindow, 0);
});
1 change: 1 addition & 0 deletions scripts/check-docs-drift.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ export const SETTINGS_ALIAS_MANIFEST: AliasManifestRow[] = [
{ field: "aiReviewOnMerge", aliases: ["onMerge"] },
{ field: "aiReviewReviewers", aliases: ["reviewers:"] },
{ field: "requireFreshRebaseWindowMinutes", aliases: ["requireFreshRebaseWindow"] },
{ field: "priorityEligibilityWindowMinutes", aliases: ["priorityEligibilityWindow"] },
{ field: "sizeGateMaxFiles", aliases: ["maxFiles"] },
{ field: "sizeGateMaxLines", aliases: ["maxLines"] },
];
Expand Down
40 changes: 40 additions & 0 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3559,6 +3559,46 @@ export async function fetchLivePullRequestMergedAt(
return result === undefined ? undefined : (result.data.merged_at ?? null);
}

/**
* When a label FIRST landed on an issue, ISO-8601, or null when it never did / cannot be read (#9738).
*
* `first: N` on purpose: the eligibility window is anchored to the EARLIEST labeling so re-applying the label
* cannot reset the clock for anyone. GitHub returns timeline items chronologically, so the first LABELED_EVENT
* naming this label is the moment the issue became publicly valuable.
*
* Reads at most one page. A maintainer who has labeled and unlabeled an issue more times than that has an
* issue whose history is not what this rule is for, and a null here FAILS OPEN (no hold) rather than guessing.
*/
export async function fetchIssueLabelFirstAppliedAt(
env: Env,
repoFullName: string,
issueNumber: number,
labelName: string,
token: string | undefined,
admissionKey?: GitHubRateLimitAdmissionKey,
): Promise<string | null> {
if (!token || !labelName) return null;
const { owner, name } = repoParts(repoFullName);
if (!owner || !name) return null;
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 } } } } } } }`;
const result = await githubGraphQl<{
data?: {
repository?: {
issue?: { timelineItems?: { nodes?: Array<{ createdAt?: string | null; label?: { name?: string | null } | null } | null> | null } | null } | null;
} | null;
};
errors?: unknown[];
}>(env, query, token, admissionKey).catch(() => undefined);
if (result === undefined) return null;
if (Array.isArray(result.errors) && result.errors.length > 0) return null;
const wanted = labelName.toLowerCase();
for (const node of result.data?.repository?.issue?.timelineItems?.nodes ?? []) {
const label = node?.label?.name;
if (typeof label === "string" && label.toLowerCase() === wanted && typeof node?.createdAt === "string") return node.createdAt;
}
return null;
}

export type LinkedIssueClosureByPullRequestResult = "closed_by_pull_request" | "not_closed_by_pull_request" | "fetch_error";

/** Verifies whether GitHub attributes this issue's closure to the specific PR, via GraphQL's `ClosedEvent.closer`
Expand Down
3 changes: 3 additions & 0 deletions src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { AGENT_ACTION_CLASSES, AUTONOMY_LEVELS, FEASIBILITY_VERDICTS, PUBLIC_SUR
// #9773: the request bodies these routes really accept, from the one place they are defined.
import { checkBeforeStartSchema, slopRiskSchema, validateFocusManifestSchema, validateLinkedIssueSchema } from "@loopover/contract/api-requests";
import { MAX_REVIEW_NAG_COOLDOWN_DAYS } from "../settings/agent-actions";
import { MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES } from "../review/priority-eligibility-window";
import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP } from "../types";
import {
MAX_FIND_OPPORTUNITIES_TARGETS,
Expand Down Expand Up @@ -759,6 +760,8 @@ export const RepositorySettingsSchema = z
gateDryRun: z.boolean().optional(),
premergeContentRecheck: z.boolean().optional(),
requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(),
// #9738: non-negative, not positive -- 0 is the documented way to turn the window off.
priorityEligibilityWindowMinutes: z.number().int().min(0).max(MAX_PRIORITY_ELIGIBILITY_WINDOW_MINUTES).nullable().optional(),
staleBaseAheadByThreshold: z.number().int().positive().nullable().optional(),
mergeReadinessGateMode: z.enum(["off", "advisory", "block"]),
manifestPolicyGateMode: z.enum(["off", "advisory", "block"]),
Expand Down
Loading
Loading