From 11e5ef66540e9bda1e4d9b1ceb2f7462c42c4c0d Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:09:37 -0700 Subject: [PATCH 1/2] fix(review): persist linked-issue hard-rule violations past the grace window resolveLinkedIssueHardRule is fully re-evaluated from scratch every pass -- it re-parses linked issues from the PR's CURRENT body 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: editing the PR body to strip the closing reference so the next pass sees zero linked issues, or the linked issue's live state changing (e.g. unassigned) between the violating pass and the verification pass. Add linkedIssueHardRuleViolatedAt/linkedIssueHardRuleViolationReason columns (mirroring draftConversionCount's never-resets discipline and mergeBlockedReason's pairing with mergeBlockedSha) so a PR that ever confirms a violation stays flagged for its lifetime, merged with the live re-parse result rather than replacing it. --- ...est_linked_issue_hard_rule_violated_at.sql | 17 ++ src/db/repositories.ts | 22 ++ src/db/schema.ts | 14 + src/queue/processors.ts | 17 +- src/review/linked-issue-hard-rules.ts | 28 ++ src/types.ts | 10 + test/unit/linked-issue-hard-rules.test.ts | 60 ++++ test/unit/queue.test.ts | 284 ++++++++++++++++++ 8 files changed, 451 insertions(+), 1 deletion(-) create mode 100644 migrations/0119_pull_request_linked_issue_hard_rule_violated_at.sql diff --git a/migrations/0119_pull_request_linked_issue_hard_rule_violated_at.sql b/migrations/0119_pull_request_linked_issue_hard_rule_violated_at.sql new file mode 100644 index 0000000000..5cca6e82e4 --- /dev/null +++ b/migrations/0119_pull_request_linked_issue_hard_rule_violated_at.sql @@ -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; diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 53ae77a5c9..d1d10596c8 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -3503,6 +3503,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 { + 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 @@ -5561,6 +5581,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, }; } diff --git a/src/db/schema.ts b/src/db/schema.ts index 9da832425d..1dbb2d442a 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -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()), }, diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 22306fa55e..874adc3fa6 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -64,6 +64,7 @@ import { isGlobalAgentFrozen, listReviewSuppressions, markGateOutcomeOverridden, + markPullRequestLinkedIssueHardRuleViolated, startActiveReviewTracking, terminalizeActiveReviewTracking, bumpPullRequestDraftConversionCount, @@ -492,6 +493,7 @@ import { } from "../github/pr-actions"; import { loadLinkedIssueHardRules, + mergeLinkedIssueHardRuleWithPersistedViolation, resolveLinkedIssueHardRule, resolveLinkedIssueHasOpenReference, } from "../review/linked-issue-hard-rules"; @@ -2702,7 +2704,7 @@ async function runAgentMaintenancePlanAndExecute( env, repoFullName, ); - const linkedIssueHardRule = await resolveLinkedIssueHardRule({ + const liveLinkedIssueHardRule = await resolveLinkedIssueHardRule({ env, repoFullName, repoOwner, @@ -2713,6 +2715,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 diff --git a/src/review/linked-issue-hard-rules.ts b/src/review/linked-issue-hard-rules.ts index b746744e33..563af3a1bd 100644 --- a/src/review/linked-issue-hard-rules.ts +++ b/src/review/linked-issue-hard-rules.ts @@ -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 + diff --git a/src/types.ts b/src/types.ts index cd57544ad7..2f4111dae1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -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 diff --git a/test/unit/linked-issue-hard-rules.test.ts b/test/unit/linked-issue-hard-rules.test.ts index 48f3be9da2..12fe41d6ca 100644 --- a/test/unit/linked-issue-hard-rules.test.ts +++ b/test/unit/linked-issue-hard-rules.test.ts @@ -7,6 +7,7 @@ import { evaluateLinkedIssueHardRules, hasVerifiableOpenLinkedIssueReference, loadLinkedIssueHardRules, + mergeLinkedIssueHardRuleWithPersistedViolation, resolveLinkedIssueHardRule, resolveLinkedIssueHasOpenReference, type LinkedIssueFacts, @@ -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" }; diff --git a/test/unit/queue.test.ts b/test/unit/queue.test.ts index 9ab01ebac8..b515b0a9c1 100644 --- a/test/unit/queue.test.ts +++ b/test/unit/queue.test.ts @@ -67,6 +67,7 @@ import { } from "../../src/github/pr-freshness"; import { createTestEnv } from "../helpers/d1"; import { SWEEP_MAX_PRS } from "../../src/settings/agent-sweep"; +import { AGENT_LABEL_PENDING_CLOSURE, DEFAULT_LINKED_ISSUE_HARD_RULES } from "../../src/review/linked-issue-hard-rules"; vi.mock("../../src/github/pr-freshness", async (importOriginal) => { const actual = await importOriginal(); @@ -2028,6 +2029,206 @@ describe("queue processors", () => { } }); + describe("linked-issue hard-rule violation persistence (#linked-issue-hard-rule-persistence)", () => { + // Shared scaffold for both regression tests below: a repo with the owner-assigned hard rule ON and the + // flag-then-close verify window ON (defaults), autonomy acting on close + review_state_label. Each test + // drives TWO separate agent-regate-pr passes over the SAME PR/head, changing only what the live GitHub read + // reports between them -- exactly the two ways resolveLinkedIssueHardRule's own statelessness lets a + // confirmed Pass-1 violation dodge the Pass-2 close. + // `linkedIssueHardRules` (unlike most repository settings) has NO backing DB column (src/db/schema.ts) -- + // it is exclusively a `.gittensory.yml`-driven override (settings/repository-settings.ts's default is + // always the built-in all-off DEFAULT_LINKED_ISSUE_HARD_RULES; only resolveEffectiveSettings's manifest + // overlay can turn a rule on). So this scaffold enables it via a stubbed `.gittensory.yml` content fetch, + // not via upsertRepositorySettings. + const HARD_RULE_MANIFEST = JSON.stringify({ settings: { linkedIssueHardRules: { ownerAssignedClose: "block" } } }); + + async function seedHardRuleRepoAndPr(env: ReturnType): Promise { + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write", checks: "write" }, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + await upsertRepositorySettings(env, { + repoFullName: "owner/agent-repo", + autonomy: { close: "auto", review_state_label: "auto" }, + aiReviewMode: "off", + gatePack: "oss-anti-slop", + gateCheckMode: "enabled", + checkRunMode: "off", + commentMode: "off", + publicSurface: "off", + }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Ineligible linked issue", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, base: { ref: "main" }, labels: [], body: "Closes #9" }); + } + + // Stateful label set (mutated by the real ensurePullRequestLabel/removePullRequestLabel POST/DELETE calls, + // mirroring how a real GitHub repo's label state persists between two separate agent-regate-pr passes) -- + // the `/pulls/7` GET always reflects it, so Pass 2's own live re-sync of the stored PR sees the label Pass 1 + // actually applied, exactly like the real GitHub API would report it. `ruleEnabled` selects whether the + // stubbed `.gittensory.yml` fetch turns the owner-assigned hard rule on (the two regression tests) or + // resolves to a genuine 404 -- i.e. the rule genuinely OFF (the sanity-check test). + function stubHardRuleFetch(liveLabels: string[], opts: { prBody: string; issueState: string; issueAssignees: string[]; ruleEnabled: boolean }) { + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = (init?.method ?? "GET").toUpperCase(); + if (url === "https://api.gittensor.io/miners") return Response.json([]); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (/\/pulls\/7(?:\?|$)/.test(url) && method === "GET") return Response.json({ number: 7, title: "Ineligible linked issue", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, mergeable_state: "clean", labels: liveLabels.map((name) => ({ name })), body: opts.prBody }); + if (url.includes("/pulls/7/files")) return Response.json([{ filename: "src/a.ts", status: "modified", additions: 1, deletions: 0, changes: 1, patch: "@@\n+export const ok = true;" }]); + if (url.includes("/issues/9") && method === "GET") return Response.json({ number: 9, state: opts.issueState, labels: [], assignees: opts.issueAssignees.map((login) => ({ login })), user: { login: "reporter" } }); + if (url.includes("/issues/7/labels") && method === "GET") return Response.json(liveLabels.map((name) => ({ name }))); + if (url.includes("/issues/7/labels") && method === "POST") { + const body = init?.body ? (JSON.parse(String(init.body)) as { labels?: string[] }) : {}; + for (const label of body.labels ?? []) if (!liveLabels.includes(label)) liveLabels.push(label); + return Response.json(liveLabels.map((name) => ({ name })), { status: 200 }); + } + if (url.includes("/labels/") && method === "DELETE") { + const removed = decodeURIComponent(url.slice(url.lastIndexOf("/labels/") + "/labels/".length)); + const index = liveLabels.indexOf(removed); + if (index >= 0) liveLabels.splice(index, 1); + return new Response(null, { status: 204 }); + } + // The `.gittensory.yml`/`.json` content fetch (raw.githubusercontent.com) is the ONLY place + // linkedIssueHardRules can be turned on (see the comment above). + if (url.includes("raw.githubusercontent.com")) return opts.ruleEnabled ? new Response(HARD_RULE_MANIFEST, { status: 200 }) : new Response("not found", { status: 404 }); + return Response.json({}); + }); + } + + const requiredContextsMock = () => vi.spyOn(backfillModule, "fetchRequiredStatusContexts").mockResolvedValue(null); + const liveCiMock = () => + vi.spyOn(backfillModule, "fetchLiveCiAggregatePreferGraphQl").mockResolvedValue({ + ciState: "passed", + hasPending: false, + hasVisiblePending: false, + hasMissingRequiredContext: false, + failingDetails: [], + nonRequiredFailingDetails: [], + ciCompletenessWarning: null, + }); + + it("REGRESSION (body-edit-during-grace-window): Pass 1 flags a real owner-assigned violation; Pass 2's live re-parse finds NO linked issues (body edited) but the persisted violation still closes the PR", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedHardRuleRepoAndPr(env); + const requiredContextsSpy = requiredContextsMock(); + const liveCiSpy = liveCiMock(); + const liveLabels: string[] = []; + try { + // Pass 1: body links #9, issue #9 is genuinely assigned to the repo owner -> a REAL violation. Verify- + // before-close is on by default, so this pass FLAGS (pending-closure label) and does not close yet. + stubHardRuleFetch(liveLabels, { prBody: "Closes #9", issueState: "open", issueAssignees: ["owner"], ruleEnabled: true }); + await processJob(env, { type: "agent-regate-pr", deliveryId: "hard-rule-pass-1", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + + const afterPass1 = await getPullRequest(env, "owner/agent-repo", 7); + expect(afterPass1?.state).toBe("open"); // flagged, not closed yet + // The label MUTATION itself is asserted via the executor's own audit trail, not the DB's cached + // labels_json -- that cache is only refreshed by the NEXT sync (reReviewStoredPullRequest resyncs at + // the START of a pass, so a label applied DURING this pass isn't reflected in labels_json until the + // following pass reads it back from the live GitHub state, which stubHardRuleFetch's liveLabels does). + const flagAudit = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? and detail like ? order by rowid desc limit 1").bind("agent.action.label", "%linked-issue hard rule%").first<{ outcome: string; detail: string }>(); + expect(flagAudit?.outcome).toBe("completed"); + expect(liveLabels).toContain(AGENT_LABEL_PENDING_CLOSURE); + // The confirmed Pass-1 violation must already be persisted -- this is the fact Pass 2 depends on. + expect(afterPass1?.linkedIssueHardRuleViolatedAt).toEqual(expect.any(String)); + expect(afterPass1?.linkedIssueHardRuleViolationReason).toContain("#9"); + + // Between passes: GitHub echoes the Pass-1 label mutation back as its own `labeled` webhook in real + // operation, which the normal PR-sync path (upsertPullRequestFromGitHub) writes into labels_json -- + // reReviewStoredPullRequest's OWN resync only re-fetches on a head-SHA change (#sweep-resync), so it + // does not carry a same-pass label mutation forward on its own. Simulate that already-processed sync + // directly (same head, only the label list changed) rather than re-deriving the whole webhook pipeline. + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Ineligible linked issue", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, base: { ref: "main" }, labels: liveLabels.map((name) => ({ name })), body: "Closes #9" }); + + // Pass 2: the contributor edited the PR body during the grace window to remove the closing reference. + // The live re-parse now sees ZERO linked issues, so resolveLinkedIssueHardRule alone would return + // undefined -- WITHOUT the persisted-violation backstop, clearLinkedIssueFlag would remove the + // pending-closure label and the PR would survive with the flag silently cleared. + stubHardRuleFetch(liveLabels, { prBody: "no more linked issue here", issueState: "open", issueAssignees: ["owner"], ruleEnabled: true }); + await processJob(env, { type: "agent-regate-pr", deliveryId: "hard-rule-pass-2-body-edited", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + + // The disposition planner's `close` action is the observable proof the persisted violation was enforced + // (the executor's own closePullRequest mutation succeeds against GitHub directly; the PR row's `state` + // column only flips to "closed" once GitHub's OWN `closed` webhook round-trips back through the normal + // sync path -- a separate delivery this two-pass sweep test does not simulate, mirroring the identical + // gap for labels_json handled above). + const close = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? order by rowid desc limit 1").bind("agent.action.close").first<{ outcome: string; detail: string }>(); + expect(close?.outcome).toBe("completed"); + expect(close?.detail).toContain("#9"); + // WITHOUT the persisted-violation backstop this pass's live re-parse (undefined -- zero linked issues) + // would have cleared the pending-closure flag instead: confirm that never happened. + const clearedFlag = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and detail like ?").bind("agent.action.label", "%resolved%").first<{ n: number }>(); + expect(clearedFlag?.n).toBe(0); + } finally { + liveCiSpy.mockRestore(); + requiredContextsSpy.mockRestore(); + } + }); + + it("REGRESSION (live-issue-state-change-before-re-evaluation): Pass 1 flags a real violation; Pass 2's live re-parse re-evaluates the SAME issue as clean (assignee removed) but the persisted violation still closes the PR", async () => { + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await seedHardRuleRepoAndPr(env); + const requiredContextsSpy = requiredContextsMock(); + const liveCiSpy = liveCiMock(); + const liveLabels: string[] = []; + try { + // Pass 1: same real owner-assigned violation as the sibling test above -> FLAGS, does not close. + stubHardRuleFetch(liveLabels, { prBody: "Closes #9", issueState: "open", issueAssignees: ["owner"], ruleEnabled: true }); + await processJob(env, { type: "agent-regate-pr", deliveryId: "hard-rule-live-pass-1", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + + const afterPass1 = await getPullRequest(env, "owner/agent-repo", 7); + expect(afterPass1?.state).toBe("open"); + const flagAudit = await env.DB.prepare("select outcome from audit_events where event_type = ? and detail like ? order by rowid desc limit 1").bind("agent.action.label", "%linked-issue hard rule%").first<{ outcome: string }>(); + expect(flagAudit?.outcome).toBe("completed"); + expect(liveLabels).toContain(AGENT_LABEL_PENDING_CLOSURE); + expect(afterPass1?.linkedIssueHardRuleViolatedAt).toEqual(expect.any(String)); + + // Between passes: GitHub echoes the Pass-1 label mutation back as its own `labeled` webhook in real + // operation (see the sibling test's identical comment for the full rationale). + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "Ineligible linked issue", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, base: { ref: "main" }, labels: liveLabels.map((name) => ({ name })), body: "Closes #9" }); + + // Pass 2: the PR body is UNCHANGED (still "Closes #9"), but issue #9's LIVE state changed between passes + // -- the owner was unassigned. The live re-parse re-evaluates the SAME issue number cleanly + // ({ violated: false }), indistinguishable from "never violated" to resolveLinkedIssueHardRule alone. + // WITHOUT the persisted-violation backstop, clearLinkedIssueFlag would remove the flag and the PR + // would survive. + stubHardRuleFetch(liveLabels, { prBody: "Closes #9", issueState: "open", issueAssignees: [], ruleEnabled: true }); + await processJob(env, { type: "agent-regate-pr", deliveryId: "hard-rule-live-pass-2-unassigned", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + + // See the sibling test's identical comment: the `close` action's own audit outcome is the observable + // proof (the PR row's `state` column only flips once GitHub's `closed` webhook round-trips back). + const close = await env.DB.prepare("select outcome, detail from audit_events where event_type = ? order by rowid desc limit 1").bind("agent.action.close").first<{ outcome: string; detail: string }>(); + expect(close?.outcome).toBe("completed"); + expect(close?.detail).toContain("#9"); + const clearedFlag = await env.DB.prepare("select count(*) as n from audit_events where event_type = ? and detail like ?").bind("agent.action.label", "%resolved%").first<{ n: number }>(); + expect(clearedFlag?.n).toBe(0); + } finally { + liveCiSpy.mockRestore(); + requiredContextsSpy.mockRestore(); + } + }); + + it("does not persist anything (and Pass 2 clears the flag normally) when the violation is GENUINELY resolved before it was ever confirmed", async () => { + // Sanity check / non-regression: a hard rule that is OFF (never violates at all) must not write the + // persisted marker, and the plan/label state must stay byte-identical to today's pre-existing behavior. + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); + await upsertInstallation(env, { action: "created", installation: { id: 9001, account: { login: "owner", id: 1, type: "Organization" }, target_type: "Organization", repository_selection: "selected", permissions: { pull_requests: "write", issues: "write", checks: "write" }, events: [] } }); + await upsertRepositoryFromGitHub(env, { name: "agent-repo", full_name: "owner/agent-repo", private: false, owner: { login: "owner" } }, 9001); + await upsertRepositorySettings(env, { repoFullName: "owner/agent-repo", autonomy: { close: "auto", review_state_label: "auto" }, aiReviewMode: "off", gatePack: "oss-anti-slop", gateCheckMode: "enabled", checkRunMode: "off", commentMode: "off", publicSurface: "off" }); + await upsertPullRequestFromGitHub(env, "owner/agent-repo", { number: 7, title: "No hard rule enabled", state: "open", user: { login: "contributor" }, head: { sha: "a7" }, base: { ref: "main" }, labels: [], body: "Closes #9" }); + const requiredContextsSpy = requiredContextsMock(); + const liveCiSpy = liveCiMock(); + try { + stubHardRuleFetch([], { prBody: "Closes #9", issueState: "open", issueAssignees: ["owner"], ruleEnabled: false }); + await processJob(env, { type: "agent-regate-pr", deliveryId: "hard-rule-off", repoFullName: "owner/agent-repo", prNumber: 7, installationId: 9001 }); + + const after = await getPullRequest(env, "owner/agent-repo", 7); + expect(after?.state).toBe("open"); + expect(after?.labels ?? []).not.toContain(AGENT_LABEL_PENDING_CLOSURE); + expect(after?.linkedIssueHardRuleViolatedAt).toBeNull(); + } finally { + liveCiSpy.mockRestore(); + requiredContextsSpy.mockRestore(); + } + }); + }); + describe("durable CI-state snapshot cache (#selfhost-ci-verification, cross-job)", () => { async function seedRepoAndPr(headSha: string): Promise<{ env: ReturnType }> { // GITTENSORY_REVIEW_REPOS (review/cutover-gate.ts) gates maybeReReviewOnCiCompletion's whole invalidation @@ -26626,6 +26827,89 @@ describe("review-evasion protection (#review-evasion-protection)", () => { }); }); +describe("markPullRequestLinkedIssueHardRuleViolated (#linked-issue-hard-rule-persistence)", () => { + it("sets violatedAt + the reason on the first call and never overwrites them on a later call", async () => { + const env = createTestEnv({}); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await repositoriesModule.upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + id: 5151, + number: 88, + state: "open", + title: "Some PR", + user: { login: "contributor" }, + head: { sha: "sha-1", ref: "fix", repo: { full_name: "contributor/gittensory", owner: { login: "contributor" } } }, + base: { sha: "base123", ref: "main", repo: { full_name: "JSONbored/gittensory", owner: { login: "JSONbored" } } }, + draft: false, + merged: false, + created_at: "2026-05-27T00:00:00Z", + updated_at: "2026-05-27T00:00:00Z", + } as never); + + const before = await repositoriesModule.getPullRequest(env, "JSONbored/gittensory", 88); + expect(before?.linkedIssueHardRuleViolatedAt).toBeNull(); + expect(before?.linkedIssueHardRuleViolationReason).toBeNull(); + + await repositoriesModule.markPullRequestLinkedIssueHardRuleViolated(env, "JSONbored/gittensory", 88, "Linked issue #7 is assigned to the maintainer (@JSONbored)"); + const afterFirst = await repositoriesModule.getPullRequest(env, "JSONbored/gittensory", 88); + expect(afterFirst?.linkedIssueHardRuleViolatedAt).toEqual(expect.any(String)); + expect(afterFirst?.linkedIssueHardRuleViolationReason).toBe("Linked issue #7 is assigned to the maintainer (@JSONbored)"); + + // A SECOND confirmed violation (e.g. against a different linked issue, or a re-detected same one) must not + // move the timestamp or replace the reason -- the FIRST confirmed violation is what's remembered forever. + await repositoriesModule.markPullRequestLinkedIssueHardRuleViolated(env, "JSONbored/gittensory", 88, "Linked issue #9 is already assigned to @someone-else"); + const afterSecond = await repositoriesModule.getPullRequest(env, "JSONbored/gittensory", 88); + expect(afterSecond?.linkedIssueHardRuleViolatedAt).toBe(afterFirst?.linkedIssueHardRuleViolatedAt); + expect(afterSecond?.linkedIssueHardRuleViolationReason).toBe("Linked issue #7 is assigned to the maintainer (@JSONbored)"); + + // A fresh push (new head SHA) between violations must NOT reset either field -- unlike mergeBlockedSha, + // this marker is deliberately not scoped to head SHA. + await repositoriesModule.upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + id: 5151, + number: 88, + state: "open", + title: "Some PR", + user: { login: "contributor" }, + head: { sha: "sha-2", ref: "fix", repo: { full_name: "contributor/gittensory", owner: { login: "contributor" } } }, + base: { sha: "base123", ref: "main", repo: { full_name: "JSONbored/gittensory", owner: { login: "JSONbored" } } }, + draft: false, + merged: false, + created_at: "2026-05-27T00:00:00Z", + updated_at: "2026-05-27T00:00:00Z", + } as never); + const afterNewHead = await repositoriesModule.getPullRequest(env, "JSONbored/gittensory", 88); + expect(afterNewHead?.linkedIssueHardRuleViolatedAt).toBe(afterFirst?.linkedIssueHardRuleViolatedAt); + expect(afterNewHead?.linkedIssueHardRuleViolationReason).toBe("Linked issue #7 is assigned to the maintainer (@JSONbored)"); + }); + + it("truncates an overlong reason to 280 chars, mirroring markPullRequestMergeBlocked", async () => { + const env = createTestEnv({}); + await upsertRepositoryFromGitHub(env, { name: "gittensory", full_name: "JSONbored/gittensory", private: false, owner: { login: "JSONbored" } }, 123); + await repositoriesModule.upsertPullRequestFromGitHub(env, "JSONbored/gittensory", { + id: 5152, + number: 89, + state: "open", + title: "Some PR", + user: { login: "contributor" }, + head: { sha: "sha-1", ref: "fix", repo: { full_name: "contributor/gittensory", owner: { login: "contributor" } } }, + base: { sha: "base123", ref: "main", repo: { full_name: "JSONbored/gittensory", owner: { login: "JSONbored" } } }, + draft: false, + merged: false, + created_at: "2026-05-27T00:00:00Z", + updated_at: "2026-05-27T00:00:00Z", + } as never); + + const longReason = "x".repeat(400); + await repositoriesModule.markPullRequestLinkedIssueHardRuleViolated(env, "JSONbored/gittensory", 89, longReason); + const row = await repositoriesModule.getPullRequest(env, "JSONbored/gittensory", 89); + expect(row?.linkedIssueHardRuleViolationReason).toHaveLength(280); + }); + + it("is a safe no-op when the PR row does not exist yet", async () => { + const env = createTestEnv({}); + await expect(repositoriesModule.markPullRequestLinkedIssueHardRuleViolated(env, "JSONbored/gittensory", 999999, "unreachable")).resolves.toBeUndefined(); + }); +}); + describe("recordAgentCommandUsage (signal-snapshot fail-safe)", () => { afterEach(() => { vi.restoreAllMocks(); From 418ffe09cc5434eb8c3e2ad6263993f5eed9990a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 7 Jul 2026 04:31:46 -0700 Subject: [PATCH 2/2] fix(db): renumber linked-issue hard-rule migration to 0120 0119 was claimed by #4025 (ai_slop_cache), which merged first -- renumbered this migration to the next free number. --- ...l => 0120_pull_request_linked_issue_hard_rule_violated_at.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename migrations/{0119_pull_request_linked_issue_hard_rule_violated_at.sql => 0120_pull_request_linked_issue_hard_rule_violated_at.sql} (100%) diff --git a/migrations/0119_pull_request_linked_issue_hard_rule_violated_at.sql b/migrations/0120_pull_request_linked_issue_hard_rule_violated_at.sql similarity index 100% rename from migrations/0119_pull_request_linked_issue_hard_rule_violated_at.sql rename to migrations/0120_pull_request_linked_issue_hard_rule_violated_at.sql