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
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
-- Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence). resolveLinkedIssueHardRule
-- is a PURE, fully-re-evaluated-from-scratch function: linked issues are re-parsed from the PR's CURRENT body
-- every pass, with no memory of a prior pass's finding. Two ways that let a confirmed violation dodge the
-- flag-then-close verification window (settings.linkedIssueHardRules.closeDelaySeconds): (1) editing the PR
-- body during the grace window to strip the closing reference, so the next pass sees zero linked issues and
-- resolveLinkedIssueHardRule returns undefined; (2) the linked issue's LIVE state changing between the
-- violating pass and the verification pass (e.g. the assignee is removed), so the same issue number
-- re-evaluates clean. Either way, clearLinkedIssueFlag (settings/agent-actions.ts) then removes the
-- pending-closure label as if the violation never happened.
--
-- linked_issue_hard_rule_violated_at is the FIRST time this PR NUMBER was confirmed to violate a hard rule --
-- set once, NEVER cleared, and deliberately NOT scoped to head SHA (mirrors draft_conversion_count, 0118: a
-- fresh commit or an edited body is still the same PR that already proved itself in violation once).
-- linked_issue_hard_rule_violation_reason carries the specific rule text so a later close can still cite it
-- even if the live re-parse can no longer reproduce it (mirrors merge_blocked_reason, 0052's pairing).
ALTER TABLE pull_requests ADD COLUMN linked_issue_hard_rule_violated_at TEXT;
ALTER TABLE pull_requests ADD COLUMN linked_issue_hard_rule_violation_reason TEXT;
22 changes: 22 additions & 0 deletions src/db/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3570,6 +3570,26 @@ export async function markPullRequestMergeBlocked(env: Env, fullName: string, nu
.where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
}

// Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence).

/** Record the FIRST confirmed linked-issue hard-rule violation for a PR. Deliberately NOT scoped to headSha
* (unlike markPullRequestMergeBlocked) and NEVER overwritten once set (mirrors bumpPullRequestDraftConversionCount's
* own "never resets" discipline) -- COALESCE keeps whichever value was written first, so a contributor editing
* the body or the linked issue's state changing after this call is a no-op here: the PR already proved itself in
* violation once and stays that way for its lifetime. A no-op (0 rows affected) when the PR row doesn't exist yet
* is safe -- the caller only reaches this after a live violation was just evaluated against an existing row. */
export async function markPullRequestLinkedIssueHardRuleViolated(env: Env, fullName: string, number: number, reason: string): Promise<void> {
const db = getDb(env.DB);
await db
.update(pullRequests)
.set({
linkedIssueHardRuleViolatedAt: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolatedAt}, ${nowIso()})`,
linkedIssueHardRuleViolationReason: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolationReason}, ${reason.slice(0, 280)})`,
updatedAt: nowIso(),
})
.where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number)));
}

/** Re-approval idempotency: record the head SHA the bot just auto-approved. The planner skips the `approve`
* disposition while approved_head_sha == headSha (this commit is already approved by the bot). Scoped to
* headSha so a later commit (the live head no longer matches) lets the bot re-approve the new code without
Expand Down Expand Up @@ -5680,6 +5700,8 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull
// Read straight from the row, NEVER the GitHub payload — this is a gittensory-internal sweep marker.
lastRegatedAt: row.lastRegatedAt,
lastPublishedSurfaceSha: row.lastPublishedSurfaceSha,
linkedIssueHardRuleViolatedAt: row.linkedIssueHardRuleViolatedAt,
linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason,
};
}

Expand Down
14 changes: 14 additions & 0 deletions src/db/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,20 @@ export const pullRequests = sqliteTable(
// be stale or partial while this marker matches headSha. gittensory-computed (publish-written), omitted from
// the GitHub-sync SET clause so a later sync cannot clobber it. (Mirrors approved_head_sha.)
lastPublishedSurfaceSha: text("last_published_surface_sha"),
// Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence). The FIRST time this PR NUMBER
// was confirmed to violate a hard rule (owner-assigned / assigned-to-another / maintainer-only / missing
// point-label) -- set once, NEVER cleared, and deliberately NOT scoped to head SHA (mirrors
// draft_conversion_count: an edited body or a fresh commit doesn't undo an already-proven violation). Checked
// ADDITIONALLY alongside resolveLinkedIssueHardRule's own live re-parse so a contributor cannot dodge the
// flag-then-close verification window by stripping the closing reference from the body, or by the linked
// issue's live state changing (e.g. unassigned), between the flagging pass and the verification pass.
// gittensory-computed (planner-written), omitted from the GitHub-sync SET clause so a later sync cannot clobber
// it.
linkedIssueHardRuleViolatedAt: text("linked_issue_hard_rule_violated_at"),
// The specific rule reason text captured at the moment of the FIRST violation (mirrors merge_blocked_reason's
// pairing with merge_blocked_sha) -- so a later close can still cite the concrete rule even if the live
// re-parse can no longer reproduce it (the issue was unlinked or its state changed).
linkedIssueHardRuleViolationReason: text("linked_issue_hard_rule_violation_reason"),
createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
},
Expand Down
17 changes: 16 additions & 1 deletion src/queue/processors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
isGlobalAgentFrozen,
listReviewSuppressions,
markGateOutcomeOverridden,
markPullRequestLinkedIssueHardRuleViolated,
startActiveReviewTracking,
terminalizeActiveReviewTracking,
bumpPullRequestDraftConversionCount,
Expand Down Expand Up @@ -497,6 +498,7 @@ import {
} from "../github/pr-actions";
import {
loadLinkedIssueHardRules,
mergeLinkedIssueHardRuleWithPersistedViolation,
resolveLinkedIssueHardRule,
resolveLinkedIssueHasOpenReference,
} from "../review/linked-issue-hard-rules";
Expand Down Expand Up @@ -2707,7 +2709,7 @@ async function runAgentMaintenancePlanAndExecute(
env,
repoFullName,
);
const linkedIssueHardRule = await resolveLinkedIssueHardRule({
const liveLinkedIssueHardRule = await resolveLinkedIssueHardRule({
env,
repoFullName,
repoOwner,
Expand All @@ -2718,6 +2720,19 @@ async function runAgentMaintenancePlanAndExecute(
prAuthorLogin: pr.authorLogin,
installationId,
});
// Violation-persistence backstop (#linked-issue-hard-rule-persistence): remember a CONFIRMED violation forever
// (markPullRequestLinkedIssueHardRuleViolated is a no-op once already set) so a LATER pass can't lose it to a
// body edit or a linked issue's live state changing -- see mergeLinkedIssueHardRuleWithPersistedViolation's own
// doc comment for the full dodge-window rationale. Best-effort write: a D1 hiccup here only means this ONE
// confirmed violation isn't remembered, matching every other gittensory-computed marker write in this file
// (mergeBlockedSha, draftConversionCount, lastRegatedAt).
if (liveLinkedIssueHardRule?.violated === true) {
await markPullRequestLinkedIssueHardRuleViolated(env, repoFullName, pr.number, liveLinkedIssueHardRule.reason ?? "the linked issue is not eligible for a community PR").catch(() => undefined);
}
const linkedIssueHardRule = mergeLinkedIssueHardRuleWithPersistedViolation(liveLinkedIssueHardRule, {
violatedAt: pr.linkedIssueHardRuleViolatedAt,
reason: pr.linkedIssueHardRuleViolationReason,
});

// Unlinked-issue guardrail (#unlinked-issue-guardrail, credibility-gate-farming defense): when this PR
// links NO issue and the repo opted in (settings.unlinkedIssueGuardrail.mode === "hold"), check whether the
Expand Down
28 changes: 28 additions & 0 deletions src/review/linked-issue-hard-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,34 @@ export function evaluateLinkedIssueHardRules(input: {
return NO_VIOLATION;
}

/**
* PURE merge of a freshly-recomputed (live) hard-rule result with a PR's persisted violation memory
* (#linked-issue-hard-rule-persistence). resolveLinkedIssueHardRule is fully re-evaluated from scratch every
* pass — it re-parses linked issues from the CURRENT PR body via regex and reads each linked issue's CURRENT
* live state, with no memory of a prior pass's finding. During the flag-then-close verification window
* (settings.linkedIssueHardRules.closeDelaySeconds), that statelessness lets a confirmed violation dodge the
* close two ways: (1) editing the PR body during the grace window to strip the closing reference, so the next
* pass sees zero linked issues and the live result is `undefined`; (2) the linked issue's live state changing
* between the violating pass and the verification pass (e.g. the assignee is removed), so the SAME issue
* number re-evaluates clean. Either way, `agent-actions.ts`'s `clearLinkedIssueFlag` would then remove the
* pending-closure label as if the violation never happened.
*
* `violatedAt` is the PR's persisted first-violation marker (`pullRequests.linkedIssueHardRuleViolatedAt`) —
* present (non-null) once ANY pass has ever confirmed a violation for this PR, and NEVER cleared. When present,
* the merged result is forced to `violated: true` regardless of what the live pass found THIS time, falling
* back to the persisted `reason` only when the live pass didn't also (re-)confirm one this pass. A live
* violation always wins for the `reason` text (freshest, most specific), so a persisted memory never masks new
* information — it only ever ADDS enforcement the live-only path would have missed.
*/
export function mergeLinkedIssueHardRuleWithPersistedViolation(
live: LinkedIssueHardRuleResult | undefined,
persisted: { violatedAt: string | null | undefined; reason: string | null | undefined },
): LinkedIssueHardRuleResult | undefined {
if (live?.violated === true) return live;
if (persisted.violatedAt == null) return live;
return { violated: true, reason: persisted.reason ?? "the linked issue is not eligible for a community PR" };
}

/**
* Orchestrate the per-PR linked-issue hard-rule decision (the testable core of maybeRunAgentMaintenance's
* linked-issue block). Returns the hard-rule result, or undefined when no rule applies. Takes the raw PR body +
Expand Down
10 changes: 10 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,16 @@ export type PullRequestRecord = {
* stale-surface diagnostics, not as a hard re-review skip: GitHub comments/checks can still be stale or partial
* while this marker matches headSha. Publish-written; read straight from the row. */
lastPublishedSurfaceSha?: string | null | undefined;
/** Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence): the FIRST time this PR NUMBER
* was confirmed to violate a hard rule. Set once, NEVER cleared, NOT scoped to head SHA (mirrors
* draftConversionCount) — checked ADDITIONALLY alongside resolveLinkedIssueHardRule's own live re-parse so an
* edited body or a changed linked-issue live state can't erase an already-confirmed violation. Planner-written;
* read straight from the row. */
linkedIssueHardRuleViolatedAt?: string | null | undefined;
/** The specific rule reason text captured at the moment of the first violation (mirrors mergeBlockedReason's
* pairing with mergeBlockedSha) — so a later close can still cite the concrete rule even when the live re-parse
* can no longer reproduce it. */
linkedIssueHardRuleViolationReason?: string | null | undefined;
/** File paths changed by this open PR, when the caller has already resolved them (e.g. from the
* `pull_request_files` cache). Absent/undefined when not resolved — callers must not assume an empty array
* means "no files changed". Mirrors {@link RecentMergedPullRequestRecord.changedFiles} so the same
Expand Down
60 changes: 60 additions & 0 deletions test/unit/linked-issue-hard-rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
evaluateLinkedIssueHardRules,
hasVerifiableOpenLinkedIssueReference,
loadLinkedIssueHardRules,
mergeLinkedIssueHardRuleWithPersistedViolation,
resolveLinkedIssueHardRule,
resolveLinkedIssueHasOpenReference,
type LinkedIssueFacts,
Expand Down Expand Up @@ -502,6 +503,65 @@ describe("resolveLinkedIssueHardRule (#1144 — overflow + orchestration)", () =
});
});

describe("mergeLinkedIssueHardRuleWithPersistedViolation (#linked-issue-hard-rule-persistence)", () => {
const notPersisted = { violatedAt: undefined, reason: undefined };

it("returns the live result unchanged when it is ALREADY a violation (persisted memory adds nothing new)", () => {
const live = { violated: true, reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs unless assigned by a maintainer." };
expect(mergeLinkedIssueHardRuleWithPersistedViolation(live, notPersisted)).toBe(live);
// A live violation's reason wins even when a DIFFERENT persisted reason also exists — freshest evidence.
expect(
mergeLinkedIssueHardRuleWithPersistedViolation(live, { violatedAt: "2026-06-01T00:00:00Z", reason: "a stale, different reason" }),
).toBe(live);
});

it("passes through undefined (no rule applies) when nothing is persisted", () => {
expect(mergeLinkedIssueHardRuleWithPersistedViolation(undefined, notPersisted)).toBeUndefined();
});

it("passes through a clean { violated: false } result unchanged when nothing is persisted", () => {
const clean = { violated: false, reason: null };
expect(mergeLinkedIssueHardRuleWithPersistedViolation(clean, notPersisted)).toBe(clean);
});

// REGRESSION (dodge 1): a contributor edits the PR body during the flag-then-close grace window to strip the
// "Closes #N" reference. The next pass's live re-parse then sees zero linked issues, so resolveLinkedIssueHardRule
// returns `undefined` -- exactly like this "live" input. Without the persisted memory, clearLinkedIssueFlag
// would remove the pending-closure label as if the violation never happened.
it("REGRESSION (body-edit-during-grace-window): a persisted violation is enforced even when the live re-parse now finds NO linked issues at all (undefined)", () => {
const merged = mergeLinkedIssueHardRuleWithPersistedViolation(undefined, {
violatedAt: "2026-06-01T12:00:00Z",
reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs unless assigned by a maintainer.",
});
expect(merged).toEqual({
violated: true,
reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs unless assigned by a maintainer.",
});
});

// REGRESSION (dodge 2): the linked issue's LIVE state changes between the violating pass and the verification
// pass (e.g. the assignee is removed, or the maintainer-only label is dropped) -- resolveLinkedIssueHardRule
// re-evaluates the SAME issue number cleanly and returns `{ violated: false, reason: null }`. Without the
// persisted memory, this is indistinguishable from "never violated" and the flag is cleared.
it("REGRESSION (live-issue-state-change-before-re-evaluation): a persisted violation is enforced even when the live re-parse now finds the SAME issue clean", () => {
const merged = mergeLinkedIssueHardRuleWithPersistedViolation(
{ violated: false, reason: null },
{ violatedAt: "2026-06-01T12:00:00Z", reason: "Linked issue #9 is already assigned to @claimed-dev — only the assignee or a maintainer can submit that work." },
);
expect(merged).toEqual({
violated: true,
reason: "Linked issue #9 is already assigned to @claimed-dev — only the assignee or a maintainer can submit that work.",
});
});

it("falls back to the generic reason when a persisted violation carries a null/missing reason", () => {
expect(mergeLinkedIssueHardRuleWithPersistedViolation(undefined, { violatedAt: "2026-06-01T00:00:00Z", reason: null })).toEqual({
violated: true,
reason: "the linked issue is not eligible for a community PR",
});
});
});

describe("hasVerifiableOpenLinkedIssueReference (#unlinked-issue-guardrail-followup — pure evaluator)", () => {
const found = (state: string): LinkedIssueFactsFetch => ({ status: "found", facts: { number: 1, state, labels: [], assignees: [], authorLogin: null } });
const notFound: LinkedIssueFactsFetch = { status: "not_found" };
Expand Down
Loading
Loading