Skip to content

Commit 8b15c8a

Browse files
authored
Merge branch 'main' into feat/merge-train-fifo-gate
2 parents b8c8df5 + aa94659 commit 8b15c8a

8 files changed

Lines changed: 451 additions & 1 deletion

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
-- Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence). resolveLinkedIssueHardRule
2+
-- is a PURE, fully-re-evaluated-from-scratch function: linked issues are re-parsed from the PR's CURRENT body
3+
-- every pass, with no memory of a prior pass's finding. Two ways that let a confirmed violation dodge the
4+
-- flag-then-close verification window (settings.linkedIssueHardRules.closeDelaySeconds): (1) editing the PR
5+
-- body during the grace window to strip the closing reference, so the next pass sees zero linked issues and
6+
-- resolveLinkedIssueHardRule returns undefined; (2) the linked issue's LIVE state changing between the
7+
-- violating pass and the verification pass (e.g. the assignee is removed), so the same issue number
8+
-- re-evaluates clean. Either way, clearLinkedIssueFlag (settings/agent-actions.ts) then removes the
9+
-- pending-closure label as if the violation never happened.
10+
--
11+
-- linked_issue_hard_rule_violated_at is the FIRST time this PR NUMBER was confirmed to violate a hard rule --
12+
-- set once, NEVER cleared, and deliberately NOT scoped to head SHA (mirrors draft_conversion_count, 0118: a
13+
-- fresh commit or an edited body is still the same PR that already proved itself in violation once).
14+
-- linked_issue_hard_rule_violation_reason carries the specific rule text so a later close can still cite it
15+
-- even if the live re-parse can no longer reproduce it (mirrors merge_blocked_reason, 0052's pairing).
16+
ALTER TABLE pull_requests ADD COLUMN linked_issue_hard_rule_violated_at TEXT;
17+
ALTER TABLE pull_requests ADD COLUMN linked_issue_hard_rule_violation_reason TEXT;

src/db/repositories.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3572,6 +3572,26 @@ export async function markPullRequestMergeBlocked(env: Env, fullName: string, nu
35723572
.where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number), eq(pullRequests.headSha, headSha)));
35733573
}
35743574

3575+
// Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence).
3576+
3577+
/** Record the FIRST confirmed linked-issue hard-rule violation for a PR. Deliberately NOT scoped to headSha
3578+
* (unlike markPullRequestMergeBlocked) and NEVER overwritten once set (mirrors bumpPullRequestDraftConversionCount's
3579+
* own "never resets" discipline) -- COALESCE keeps whichever value was written first, so a contributor editing
3580+
* the body or the linked issue's state changing after this call is a no-op here: the PR already proved itself in
3581+
* violation once and stays that way for its lifetime. A no-op (0 rows affected) when the PR row doesn't exist yet
3582+
* is safe -- the caller only reaches this after a live violation was just evaluated against an existing row. */
3583+
export async function markPullRequestLinkedIssueHardRuleViolated(env: Env, fullName: string, number: number, reason: string): Promise<void> {
3584+
const db = getDb(env.DB);
3585+
await db
3586+
.update(pullRequests)
3587+
.set({
3588+
linkedIssueHardRuleViolatedAt: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolatedAt}, ${nowIso()})`,
3589+
linkedIssueHardRuleViolationReason: sql`COALESCE(${pullRequests.linkedIssueHardRuleViolationReason}, ${reason.slice(0, 280)})`,
3590+
updatedAt: nowIso(),
3591+
})
3592+
.where(and(eq(pullRequests.repoFullName, fullName), eq(pullRequests.number, number)));
3593+
}
3594+
35753595
/** Re-approval idempotency: record the head SHA the bot just auto-approved. The planner skips the `approve`
35763596
* disposition while approved_head_sha == headSha (this commit is already approved by the bot). Scoped to
35773597
* headSha so a later commit (the live head no longer matches) lets the bot re-approve the new code without
@@ -5682,6 +5702,8 @@ function toPullRequestRecordFromRow(row: typeof pullRequests.$inferSelect): Pull
56825702
// Read straight from the row, NEVER the GitHub payload — this is a gittensory-internal sweep marker.
56835703
lastRegatedAt: row.lastRegatedAt,
56845704
lastPublishedSurfaceSha: row.lastPublishedSurfaceSha,
5705+
linkedIssueHardRuleViolatedAt: row.linkedIssueHardRuleViolatedAt,
5706+
linkedIssueHardRuleViolationReason: row.linkedIssueHardRuleViolationReason,
56855707
};
56865708
}
56875709

src/db/schema.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,20 @@ export const pullRequests = sqliteTable(
451451
// be stale or partial while this marker matches headSha. gittensory-computed (publish-written), omitted from
452452
// the GitHub-sync SET clause so a later sync cannot clobber it. (Mirrors approved_head_sha.)
453453
lastPublishedSurfaceSha: text("last_published_surface_sha"),
454+
// Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence). The FIRST time this PR NUMBER
455+
// was confirmed to violate a hard rule (owner-assigned / assigned-to-another / maintainer-only / missing
456+
// point-label) -- set once, NEVER cleared, and deliberately NOT scoped to head SHA (mirrors
457+
// draft_conversion_count: an edited body or a fresh commit doesn't undo an already-proven violation). Checked
458+
// ADDITIONALLY alongside resolveLinkedIssueHardRule's own live re-parse so a contributor cannot dodge the
459+
// flag-then-close verification window by stripping the closing reference from the body, or by the linked
460+
// issue's live state changing (e.g. unassigned), between the flagging pass and the verification pass.
461+
// gittensory-computed (planner-written), omitted from the GitHub-sync SET clause so a later sync cannot clobber
462+
// it.
463+
linkedIssueHardRuleViolatedAt: text("linked_issue_hard_rule_violated_at"),
464+
// The specific rule reason text captured at the moment of the FIRST violation (mirrors merge_blocked_reason's
465+
// pairing with merge_blocked_sha) -- so a later close can still cite the concrete rule even if the live
466+
// re-parse can no longer reproduce it (the issue was unlinked or its state changed).
467+
linkedIssueHardRuleViolationReason: text("linked_issue_hard_rule_violation_reason"),
454468
createdAt: text("created_at").notNull().$defaultFn(() => nowIso()),
455469
updatedAt: text("updated_at").notNull().$defaultFn(() => nowIso()),
456470
},

src/queue/processors.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import {
6868
isGlobalAgentFrozen,
6969
listReviewSuppressions,
7070
markGateOutcomeOverridden,
71+
markPullRequestLinkedIssueHardRuleViolated,
7172
startActiveReviewTracking,
7273
terminalizeActiveReviewTracking,
7374
bumpPullRequestDraftConversionCount,
@@ -497,6 +498,7 @@ import {
497498
} from "../github/pr-actions";
498499
import {
499500
loadLinkedIssueHardRules,
501+
mergeLinkedIssueHardRuleWithPersistedViolation,
500502
resolveLinkedIssueHardRule,
501503
resolveLinkedIssueHasOpenReference,
502504
} from "../review/linked-issue-hard-rules";
@@ -2707,7 +2709,7 @@ async function runAgentMaintenancePlanAndExecute(
27072709
env,
27082710
repoFullName,
27092711
);
2710-
const linkedIssueHardRule = await resolveLinkedIssueHardRule({
2712+
const liveLinkedIssueHardRule = await resolveLinkedIssueHardRule({
27112713
env,
27122714
repoFullName,
27132715
repoOwner,
@@ -2718,6 +2720,19 @@ async function runAgentMaintenancePlanAndExecute(
27182720
prAuthorLogin: pr.authorLogin,
27192721
installationId,
27202722
});
2723+
// Violation-persistence backstop (#linked-issue-hard-rule-persistence): remember a CONFIRMED violation forever
2724+
// (markPullRequestLinkedIssueHardRuleViolated is a no-op once already set) so a LATER pass can't lose it to a
2725+
// body edit or a linked issue's live state changing -- see mergeLinkedIssueHardRuleWithPersistedViolation's own
2726+
// doc comment for the full dodge-window rationale. Best-effort write: a D1 hiccup here only means this ONE
2727+
// confirmed violation isn't remembered, matching every other gittensory-computed marker write in this file
2728+
// (mergeBlockedSha, draftConversionCount, lastRegatedAt).
2729+
if (liveLinkedIssueHardRule?.violated === true) {
2730+
await markPullRequestLinkedIssueHardRuleViolated(env, repoFullName, pr.number, liveLinkedIssueHardRule.reason ?? "the linked issue is not eligible for a community PR").catch(() => undefined);
2731+
}
2732+
const linkedIssueHardRule = mergeLinkedIssueHardRuleWithPersistedViolation(liveLinkedIssueHardRule, {
2733+
violatedAt: pr.linkedIssueHardRuleViolatedAt,
2734+
reason: pr.linkedIssueHardRuleViolationReason,
2735+
});
27212736

27222737
// Unlinked-issue guardrail (#unlinked-issue-guardrail, credibility-gate-farming defense): when this PR
27232738
// links NO issue and the repo opted in (settings.unlinkedIssueGuardrail.mode === "hold"), check whether the

src/review/linked-issue-hard-rules.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,34 @@ export function evaluateLinkedIssueHardRules(input: {
130130
return NO_VIOLATION;
131131
}
132132

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

src/types.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,16 @@ export type PullRequestRecord = {
534534
* stale-surface diagnostics, not as a hard re-review skip: GitHub comments/checks can still be stale or partial
535535
* while this marker matches headSha. Publish-written; read straight from the row. */
536536
lastPublishedSurfaceSha?: string | null | undefined;
537+
/** Linked-issue hard-rule violation memory (#linked-issue-hard-rule-persistence): the FIRST time this PR NUMBER
538+
* was confirmed to violate a hard rule. Set once, NEVER cleared, NOT scoped to head SHA (mirrors
539+
* draftConversionCount) — checked ADDITIONALLY alongside resolveLinkedIssueHardRule's own live re-parse so an
540+
* edited body or a changed linked-issue live state can't erase an already-confirmed violation. Planner-written;
541+
* read straight from the row. */
542+
linkedIssueHardRuleViolatedAt?: string | null | undefined;
543+
/** The specific rule reason text captured at the moment of the first violation (mirrors mergeBlockedReason's
544+
* pairing with mergeBlockedSha) — so a later close can still cite the concrete rule even when the live re-parse
545+
* can no longer reproduce it. */
546+
linkedIssueHardRuleViolationReason?: string | null | undefined;
537547
/** File paths changed by this open PR, when the caller has already resolved them (e.g. from the
538548
* `pull_request_files` cache). Absent/undefined when not resolved — callers must not assume an empty array
539549
* means "no files changed". Mirrors {@link RecentMergedPullRequestRecord.changedFiles} so the same

test/unit/linked-issue-hard-rules.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
evaluateLinkedIssueHardRules,
88
hasVerifiableOpenLinkedIssueReference,
99
loadLinkedIssueHardRules,
10+
mergeLinkedIssueHardRuleWithPersistedViolation,
1011
resolveLinkedIssueHardRule,
1112
resolveLinkedIssueHasOpenReference,
1213
type LinkedIssueFacts,
@@ -502,6 +503,65 @@ describe("resolveLinkedIssueHardRule (#1144 — overflow + orchestration)", () =
502503
});
503504
});
504505

506+
describe("mergeLinkedIssueHardRuleWithPersistedViolation (#linked-issue-hard-rule-persistence)", () => {
507+
const notPersisted = { violatedAt: undefined, reason: undefined };
508+
509+
it("returns the live result unchanged when it is ALREADY a violation (persisted memory adds nothing new)", () => {
510+
const live = { violated: true, reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs unless assigned by a maintainer." };
511+
expect(mergeLinkedIssueHardRuleWithPersistedViolation(live, notPersisted)).toBe(live);
512+
// A live violation's reason wins even when a DIFFERENT persisted reason also exists — freshest evidence.
513+
expect(
514+
mergeLinkedIssueHardRuleWithPersistedViolation(live, { violatedAt: "2026-06-01T00:00:00Z", reason: "a stale, different reason" }),
515+
).toBe(live);
516+
});
517+
518+
it("passes through undefined (no rule applies) when nothing is persisted", () => {
519+
expect(mergeLinkedIssueHardRuleWithPersistedViolation(undefined, notPersisted)).toBeUndefined();
520+
});
521+
522+
it("passes through a clean { violated: false } result unchanged when nothing is persisted", () => {
523+
const clean = { violated: false, reason: null };
524+
expect(mergeLinkedIssueHardRuleWithPersistedViolation(clean, notPersisted)).toBe(clean);
525+
});
526+
527+
// REGRESSION (dodge 1): a contributor edits the PR body during the flag-then-close grace window to strip the
528+
// "Closes #N" reference. The next pass's live re-parse then sees zero linked issues, so resolveLinkedIssueHardRule
529+
// returns `undefined` -- exactly like this "live" input. Without the persisted memory, clearLinkedIssueFlag
530+
// would remove the pending-closure label as if the violation never happened.
531+
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)", () => {
532+
const merged = mergeLinkedIssueHardRuleWithPersistedViolation(undefined, {
533+
violatedAt: "2026-06-01T12:00:00Z",
534+
reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs unless assigned by a maintainer.",
535+
});
536+
expect(merged).toEqual({
537+
violated: true,
538+
reason: "Linked issue #9 is labeled `maintainer-only` — it is not open for community PRs unless assigned by a maintainer.",
539+
});
540+
});
541+
542+
// REGRESSION (dodge 2): the linked issue's LIVE state changes between the violating pass and the verification
543+
// pass (e.g. the assignee is removed, or the maintainer-only label is dropped) -- resolveLinkedIssueHardRule
544+
// re-evaluates the SAME issue number cleanly and returns `{ violated: false, reason: null }`. Without the
545+
// persisted memory, this is indistinguishable from "never violated" and the flag is cleared.
546+
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", () => {
547+
const merged = mergeLinkedIssueHardRuleWithPersistedViolation(
548+
{ violated: false, reason: null },
549+
{ 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." },
550+
);
551+
expect(merged).toEqual({
552+
violated: true,
553+
reason: "Linked issue #9 is already assigned to @claimed-dev — only the assignee or a maintainer can submit that work.",
554+
});
555+
});
556+
557+
it("falls back to the generic reason when a persisted violation carries a null/missing reason", () => {
558+
expect(mergeLinkedIssueHardRuleWithPersistedViolation(undefined, { violatedAt: "2026-06-01T00:00:00Z", reason: null })).toEqual({
559+
violated: true,
560+
reason: "the linked issue is not eligible for a community PR",
561+
});
562+
});
563+
});
564+
505565
describe("hasVerifiableOpenLinkedIssueReference (#unlinked-issue-guardrail-followup — pure evaluator)", () => {
506566
const found = (state: string): LinkedIssueFactsFetch => ({ status: "found", facts: { number: 1, state, labels: [], assignees: [], authorLogin: null } });
507567
const notFound: LinkedIssueFactsFetch = { status: "not_found" };

0 commit comments

Comments
 (0)