diff --git a/migrations/0120_pull_request_linked_issue_hard_rule_violated_at.sql b/migrations/0120_pull_request_linked_issue_hard_rule_violated_at.sql new file mode 100644 index 0000000000..5cca6e82e4 --- /dev/null +++ b/migrations/0120_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 d39b9bf315..cfc7131b83 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -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 { + 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 @@ -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, }; } diff --git a/src/db/schema.ts b/src/db/schema.ts index d0c5d07bef..cfd2adda9c 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 d75231c13f..58d93cd09c 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -68,6 +68,7 @@ import { isGlobalAgentFrozen, listReviewSuppressions, markGateOutcomeOverridden, + markPullRequestLinkedIssueHardRuleViolated, startActiveReviewTracking, terminalizeActiveReviewTracking, bumpPullRequestDraftConversionCount, @@ -497,6 +498,7 @@ import { } from "../github/pr-actions"; import { loadLinkedIssueHardRules, + mergeLinkedIssueHardRuleWithPersistedViolation, resolveLinkedIssueHardRule, resolveLinkedIssueHasOpenReference, } from "../review/linked-issue-hard-rules"; @@ -2707,7 +2709,7 @@ async function runAgentMaintenancePlanAndExecute( env, repoFullName, ); - const linkedIssueHardRule = await resolveLinkedIssueHardRule({ + const liveLinkedIssueHardRule = await resolveLinkedIssueHardRule({ env, repoFullName, repoOwner, @@ -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 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 b8e1e72a2e..f2644194e3 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 @@ -26747,6 +26948,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();