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
6 changes: 4 additions & 2 deletions .gittensory.yml.example
Original file line number Diff line number Diff line change
Expand Up @@ -619,8 +619,10 @@ settings:
# output, or existing PR labels, only ever copied from a linked/closing issue ("Fixes #123") that
# ALREADY carries the configured issue label. Generic beyond the priority use case: any issue label
# can map to any PR label. `removeOtherTypeLabels: true` marks the mapping EXCLUSIVE -- it REPLACES
# the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; only
# the FIRST-configured exclusive match wins when more than one applies. `false` (e.g. `gittensor:priority`,
# the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; when
# more than one applies, the LAST-configured exclusive match wins -- declare exclusive mappings in
# ASCENDING precedence order (lowest-value category first, e.g. bug before feature) so the highest-
# precedence one you actually want is declared last. `false` (e.g. `gittensor:priority`,
# which is a reward tag that coexists WITH whichever type already applies, not a type of its own) applies
# the mapped label ADDITIVELY alongside whichever exclusive match (or the normal title-based bug/feature
# label) already won -- every additive match composes together rather than competing for a single slot.
Expand Down
6 changes: 4 additions & 2 deletions config/examples/gittensory.full.yml
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,10 @@ settings:
# output, or existing PR labels, only ever copied from a linked/closing issue ("Fixes #123") that
# ALREADY carries the configured issue label. Generic beyond the priority use case: any issue label
# can map to any PR label. `removeOtherTypeLabels: true` marks the mapping EXCLUSIVE -- it REPLACES
# the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; only
# the FIRST-configured exclusive match wins when more than one applies. `false` (e.g. `gittensor:priority`,
# the type label entirely (bug/feature are removed), for genuinely mutually-exclusive categories; when
# more than one applies, the LAST-configured exclusive match wins -- declare exclusive mappings in
# ASCENDING precedence order (lowest-value category first, e.g. bug before feature) so the highest-
# precedence one you actually want is declared last. `false` (e.g. `gittensor:priority`,
# which is a reward tag that coexists WITH whichever type already applies, not a type of its own) applies
# the mapped label ADDITIVELY alongside whichever exclusive match (or the normal title-based bug/feature
# label) already won -- every additive match composes together rather than competing for a single slot.
Expand Down
28 changes: 17 additions & 11 deletions packages/gittensory-engine/src/settings/pr-type-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ export type PrTypeLabelDecision = {
/**
* Resolve the TYPE label decision for a PR.
* 1. Linked-issue label PROPAGATION (config-driven, #priority-linked-issue-gate): when enabled, the
* FIRST configured mapping whose `issueLabel` appears (case-insensitively) among the
* ALREADY-FETCHED `linkedIssueLabels` wins. This is the ONLY way a label like `gittensor:priority`
* LAST configured EXCLUSIVE mapping whose `issueLabel` appears (case-insensitively) among the
* ALREADY-FETCHED `linkedIssueLabels` wins (#5385 -- declare exclusive mappings in ascending
* precedence order). This is the ONLY way a label like `gittensor:priority`
* can ever be chosen — this function does no I/O and never infers it from title, changed files,
* AI output, or PR labels; the caller must fetch `linkedIssueLabels` itself (see
* `fetchLinkedIssueLabelsForPropagation` in `review/linked-issue-label-propagation-fetch.ts`).
Expand Down Expand Up @@ -152,19 +153,24 @@ export function resolvePrTypeLabel(input: {
if (input.propagation?.enabled) {
const wanted = new Set((input.linkedIssueLabels ?? []).map((label) => label.toLowerCase()));
// Collect EVERY mapping the linked issue's labels satisfy, not just the first. An exclusive mapping
// (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) still only
// ever lets the FIRST-configured match win, same precedence as before. But an additive mapping (e.g.
// priority -- a maintainer-hand-picked reward tag that coexists WITH whichever type already applies, not a
// type of its own) must compose with that winner instead of being skipped just because an earlier mapping
// in the array already matched and returned. Before this, an additive match was unreachable whenever the
// SAME linked issue also carried a label an earlier (exclusive) mapping matched -- the overwhelmingly common
// case for gittensor:priority, which is applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never
// instead of it (#priority-linked-issue-gate).
// (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) lets the
// LAST-configured match win, not the first (#5385 fix -- was first-match-wins, which meant a linked issue
// carrying BOTH gittensor:bug and gittensor:feature always resolved to bug, the lower-value label, purely
// because bug is declared before feature in `.gittensory.yml`). Operators must declare exclusive mappings
// in ASCENDING precedence order (lowest-value category first, e.g. bug then feature) so the last match
// encountered while iterating is the highest-precedence one that actually applies -- this mirrors the
// repo's own default mapping order, which is already bug/feature/priority (ascending multiplier value).
// An additive mapping (e.g. priority -- a maintainer-hand-picked reward tag that coexists WITH whichever
// type already applies, not a type of its own) must compose with that winner instead of being skipped just
// because an earlier mapping in the array already matched. Before the original #priority-linked-issue-gate
// composition fix, an additive match was unreachable whenever the SAME linked issue also carried a label an
// earlier (exclusive) mapping matched -- the overwhelmingly common case for gittensor:priority, which is
// applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never instead of it.
let exclusiveMatch: LinkedIssueLabelPropagationMapping | undefined;
const additiveMatches: LinkedIssueLabelPropagationMapping[] = [];
for (const mapping of input.propagation.mappings) {
if (!wanted.has(mapping.issueLabel.toLowerCase())) continue;
if (mapping.removeOtherTypeLabels) exclusiveMatch ??= mapping;
if (mapping.removeOtherTypeLabels) exclusiveMatch = mapping;
else additiveMatches.push(mapping);
}
if (exclusiveMatch || additiveMatches.length > 0) {
Expand Down
45 changes: 31 additions & 14 deletions src/github/backfill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3239,16 +3239,20 @@ export async function fetchLivePullRequestMergedAt(

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

function timelineEventClosesIssueFromPullRequest(
event: { event?: string | null; source?: { issue?: { number?: number | null; pull_request?: unknown } | null } | null },
prNumber: number,
): boolean {
return event.event === "closed" && event.source?.issue?.number === prNumber && event.source.issue.pull_request !== undefined;
}

/** Verifies whether GitHub's issue timeline attributes this issue close to the specific PR. Timestamp ordering
* alone only proves the issue closed after the PR merged; the timeline's closing-reference source binds the
* closure to THIS PR and prevents borrowing labels from an unrelated issue that happened to close later. */
/** Verifies whether GitHub attributes this issue's closure to the specific PR, via GraphQL's `ClosedEvent.closer`
* field -- the one place GitHub's API actually records "what closed this issue" (a `PullRequest` or a `Commit`).
* Timestamp ordering alone only proves the issue closed after the PR merged; `closer` binds the closure to THIS
* PR and prevents borrowing labels from an unrelated issue that happened to close later (#4528).
*
* #5385 (production incident): a prior version of this check read REST's `GET /issues/{n}/timeline` and looked
* for a `closed`-type event carrying a `source.issue` field matching this PR. That field NEVER appears on a
* `closed` event -- confirmed against three live production examples -- it only exists on `cross-referenced`
* events, a structurally different type. The REST check therefore always returned `not_closed_by_pull_request`,
* even for a completely legitimate same-PR close, silently converting every "Closes #N" merge into a title-
* heuristic mislabel. `closer` is GraphQL-only; there is no equivalent REST field to fall back to.
*
* `last: 1` deliberately reads only the MOST RECENT closing event, not history: an issue that was closed,
* reopened, and closed again by something else must be judged on its current closer, not a superseded one. */
export async function fetchLinkedIssueClosedByPullRequest(
env: Env,
repoFullName: string,
Expand All @@ -3257,11 +3261,24 @@ export async function fetchLinkedIssueClosedByPullRequest(
token: string | undefined,
admissionKey?: GitHubRateLimitAdmissionKey,
): Promise<LinkedIssueClosureByPullRequestResult> {
const result = await githubJsonWithHeaders<
Array<{ event?: string | null; source?: { issue?: { number?: number | null; pull_request?: unknown } | null } | null }>
>(env, repoFullName, `/issues/${issueNumber}/timeline?per_page=100`, token, githubRateLimitOptions(admissionKey)).catch(() => undefined);
if (!token) return "fetch_error";
const { owner, name } = repoParts(repoFullName);
if (!owner || !name) return "fetch_error";
const query = `query GittensoryIssueCloser { repository(owner: ${JSON.stringify(owner)}, name: ${JSON.stringify(name)}) { issue(number: ${issueNumber}) { timelineItems(last: 1, itemTypes: [CLOSED_EVENT]) { nodes { __typename ... on ClosedEvent { closer { __typename ... on PullRequest { number } } } } } } } }`;
const result = await githubGraphQl<{
data?: {
repository?: {
issue?: { timelineItems?: { nodes?: Array<{ closer?: { __typename?: string | null; number?: number | null } | null } | null> | null } | null } | null;
} | null;
};
errors?: unknown[];
}>(env, query, token, admissionKey).catch(() => undefined);
if (result === undefined) return "fetch_error";
return result.data.some((event) => timelineEventClosesIssueFromPullRequest(event, prNumber)) ? "closed_by_pull_request" : "not_closed_by_pull_request";
if (Array.isArray(result.errors) && result.errors.length > 0) return "fetch_error";
const nodes = result.data?.repository?.issue?.timelineItems?.nodes;
if (!Array.isArray(nodes)) return "fetch_error";
const closer = nodes[0]?.closer;
return closer?.__typename === "PullRequest" && closer.number === prNumber ? "closed_by_pull_request" : "not_closed_by_pull_request";
}

/** The issue's LIVE state ("open" / "closed") via REST `GET /issues/{n}`. Mirrors {@link fetchLivePullRequestState}
Expand Down
9 changes: 8 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4086,7 +4086,14 @@ async function maybeReReviewOnLinkedIssueChange(
const installationId = getInstallationId(payload);
const issueNumber = payload.issue?.number;
if (!repoFullName || !installationId || !issueNumber) return false;
if (isConvergenceRepoAllowed(env, repoFullName)) {
// #5385: mirrors sweepRepoRegate's own gate exactly -- a repo with acting autonomy configured but NOT in the
// GITTENSORY_REVIEW_REPOS allowlist (e.g. removed during a rollback, or a self-hoster who configured autonomy
// without also updating the env allowlist) used to silently never wake affected PRs here, leaving a stale
// type label (or any other issue-driven verdict) until the sweep eventually reached it, cycles later.
// Short-circuited deliberately: resolveRepositorySettings does a live manifest fetch, so the allowlisted
// common case (this repo is already in GITTENSORY_REVIEW_REPOS) must never pay for it -- this handler's own
// doc comment promises "never doing the expensive live re-review inline", and that includes this gate check.
if (isConvergenceRepoAllowed(env, repoFullName) || isAgentConfigured((await resolveRepositorySettings(env, repoFullName)).autonomy)) {
const openPullRequests = await listOpenPullRequests(env, repoFullName);
// Issue-side label/assignment changes can flip linked-issue hard-rule verdicts from mergeable to close.
// Wake affected PRs promptly: the issue-side signal can invalidate public gate state, so dropping every
Expand Down
28 changes: 17 additions & 11 deletions src/settings/pr-type-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ export type PrTypeLabelDecision = {
/**
* Resolve the TYPE label decision for a PR.
* 1. Linked-issue label PROPAGATION (config-driven, #priority-linked-issue-gate): when enabled, the
* FIRST configured mapping whose `issueLabel` appears (case-insensitively) among the
* ALREADY-FETCHED `linkedIssueLabels` wins. This is the ONLY way a label like `gittensor:priority`
* LAST configured EXCLUSIVE mapping whose `issueLabel` appears (case-insensitively) among the
* ALREADY-FETCHED `linkedIssueLabels` wins (#5385 -- declare exclusive mappings in ascending
* precedence order). This is the ONLY way a label like `gittensor:priority`
* can ever be chosen — this function does no I/O and never infers it from title, changed files,
* AI output, or PR labels; the caller must fetch `linkedIssueLabels` itself (see
* `fetchLinkedIssueLabelsForPropagation` in `review/linked-issue-label-propagation-fetch.ts`).
Expand Down Expand Up @@ -152,19 +153,24 @@ export function resolvePrTypeLabel(input: {
if (input.propagation?.enabled) {
const wanted = new Set((input.linkedIssueLabels ?? []).map((label) => label.toLowerCase()));
// Collect EVERY mapping the linked issue's labels satisfy, not just the first. An exclusive mapping
// (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) still only
// ever lets the FIRST-configured match win, same precedence as before. But an additive mapping (e.g.
// priority -- a maintainer-hand-picked reward tag that coexists WITH whichever type already applies, not a
// type of its own) must compose with that winner instead of being skipped just because an earlier mapping
// in the array already matched and returned. Before this, an additive match was unreachable whenever the
// SAME linked issue also carried a label an earlier (exclusive) mapping matched -- the overwhelmingly common
// case for gittensor:priority, which is applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never
// instead of it (#priority-linked-issue-gate).
// (removeOtherTypeLabels: true -- e.g. bug/feature, genuinely mutually-exclusive categories) lets the
// LAST-configured match win, not the first (#5385 fix -- was first-match-wins, which meant a linked issue
// carrying BOTH gittensor:bug and gittensor:feature always resolved to bug, the lower-value label, purely
// because bug is declared before feature in `.gittensory.yml`). Operators must declare exclusive mappings
// in ASCENDING precedence order (lowest-value category first, e.g. bug then feature) so the last match
// encountered while iterating is the highest-precedence one that actually applies -- this mirrors the
// repo's own default mapping order, which is already bug/feature/priority (ascending multiplier value).
// An additive mapping (e.g. priority -- a maintainer-hand-picked reward tag that coexists WITH whichever
// type already applies, not a type of its own) must compose with that winner instead of being skipped just
// because an earlier mapping in the array already matched. Before the original #priority-linked-issue-gate
// composition fix, an additive match was unreachable whenever the SAME linked issue also carried a label an
// earlier (exclusive) mapping matched -- the overwhelmingly common case for gittensor:priority, which is
// applied ALONGSIDE gittensor:bug/gittensor:feature on the issue, never instead of it.
let exclusiveMatch: LinkedIssueLabelPropagationMapping | undefined;
const additiveMatches: LinkedIssueLabelPropagationMapping[] = [];
for (const mapping of input.propagation.mappings) {
if (!wanted.has(mapping.issueLabel.toLowerCase())) continue;
if (mapping.removeOtherTypeLabels) exclusiveMatch ??= mapping;
if (mapping.removeOtherTypeLabels) exclusiveMatch = mapping;
else additiveMatches.push(mapping);
}
if (exclusiveMatch || additiveMatches.length > 0) {
Expand Down
Loading