diff --git a/src/review/repo-skill-render.ts b/src/review/repo-skill-render.ts index 6cc14612e..05bfac205 100644 --- a/src/review/repo-skill-render.ts +++ b/src/review/repo-skill-render.ts @@ -21,10 +21,12 @@ function hasBlockingGate(contributionWorkflow: RepoProfileContributionWorkflow): return contributionWorkflow.gatePublishesCheck; } -/** A repo that both requires a linked issue AND has a policy stricter than "optional" has a real, non-obvious - * admission rule worth writing down -- mirrors the mismatch repo-policy-readiness.ts already treats as notable. */ +/** A repo whose linked-issue rule actually BLOCKS a PR has a real, non-obvious admission rule worth writing + * down. `linkedIssueGateMode` is the enforcement authority (repo-profile.ts's doc comment on that field) -- + * `requireLinkedIssue`/`linkedIssuePolicy` alone are advisory-only and must not decide this, matching the + * identical assertion in repo-doc-render.ts's "Requires a linked issue" line. */ function hasStrictLinkedIssueRule(contributionWorkflow: RepoProfileContributionWorkflow): boolean { - return contributionWorkflow.requireLinkedIssue && contributionWorkflow.linkedIssuePolicy !== "optional"; + return contributionWorkflow.linkedIssueGateMode === "block"; } function hasMultiStageCi(contributionWorkflow: RepoProfileContributionWorkflow): boolean { diff --git a/test/unit/repo-doc-pr.test.ts b/test/unit/repo-doc-pr.test.ts index 607ebb613..d2a0477b8 100644 --- a/test/unit/repo-doc-pr.test.ts +++ b/test/unit/repo-doc-pr.test.ts @@ -48,10 +48,13 @@ async function seedRepoDocGenerationConfig(env: ReturnType // #3001: shouldGenerateRepoSkill needs 2 of 3 named signals. Seeds a strict linked-issue rule (settings + // manifest) and 2 CI workflow files -- both in the SAME upsertRepoFocusManifest call as repoDocGeneration, since -// a second separate call would replace rather than merge with the first. +// a second separate call would replace rather than merge with the first. `gate.linkedIssue: "block"` is what +// actually makes the rule strict (#9671) -- requireLinkedIssue/linkedIssuePolicy alone are advisory-only; +// linkedIssueGateMode itself is config-as-code only, so it can only be set via the manifest's `gate` block, not +// upsertRepositorySettings (that field is a silent no-op there per repositories.ts's Batch A comment). async function seedSkillTriggerRepo(env: ReturnType, repoFullName: string, scope: string[] = ["agents", "skills"], overrides: { allowOverwriteExisting?: boolean } = {}): Promise { await upsertRepositorySettings(env, { repoFullName, requireLinkedIssue: true }); - await upsertRepoFocusManifest(env, repoFullName, { linkedIssuePolicy: "required", repoDocGeneration: { enabled: true, scope, ...overrides } }); + await upsertRepoFocusManifest(env, repoFullName, { linkedIssuePolicy: "required", gate: { linkedIssue: "block" }, repoDocGeneration: { enabled: true, scope, ...overrides } }); await seedChunk(env, ".github/workflows/ci.yml", "name: CI\non: push\n"); await seedChunk(env, ".github/workflows/lint.yml", "name: Lint\non: push\n"); } diff --git a/test/unit/repo-skill-render.test.ts b/test/unit/repo-skill-render.test.ts index c210648cd..ca01805f5 100644 --- a/test/unit/repo-skill-render.test.ts +++ b/test/unit/repo-skill-render.test.ts @@ -25,12 +25,12 @@ describe("shouldGenerateRepoSkill (#3001)", () => { it.each([ ["no signals", contributionWorkflow(), false], ["gate only", contributionWorkflow({ gatePublishesCheck: true }), false], - ["strict linked issue only", contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "required" }), false], + ["strict linked issue only", contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "block" }), false], ["multi-stage CI only", contributionWorkflow({ ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), false], - ["gate + strict linked issue (2 of 3)", contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required" }), true], + ["gate + strict linked issue (2 of 3)", contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "block" }), true], ["gate + multi-stage CI (2 of 3)", contributionWorkflow({ gatePublishesCheck: true, ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], - ["strict linked issue + multi-stage CI (2 of 3)", contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "preferred", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], - ["all three signals (3 of 3)", contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], + ["strict linked issue + multi-stage CI (2 of 3)", contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "preferred", linkedIssueGateMode: "block", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], + ["all three signals (3 of 3)", contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "block", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), true], ])("%s -> %s", (_label, workflow, expected) => { expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(expected); }); @@ -42,6 +42,24 @@ describe("shouldGenerateRepoSkill (#3001)", () => { expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(false); }); + it("only linkedIssueGateMode \"block\" counts as a strict linked-issue rule, not requireLinkedIssue/linkedIssuePolicy alone (#9671)", () => { + // requireLinkedIssue: true + linkedIssuePolicy: "required" used to be sufficient on their own; the actual + // enforcement authority is linkedIssueGateMode (repo-profile.ts's doc comment on that field), matching + // repo-doc-render.ts's identical "Requires a linked issue" assertion. + const gateOnlyAdvisoryLinkedIssue = contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "advisory" }); + expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: gateOnlyAdvisoryLinkedIssue }) as Extract)).toBe(false); + + const gateOnlyBlockLinkedIssue = contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "block" }); + expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: gateOnlyBlockLinkedIssue }) as Extract)).toBe(true); + }); + + it("a repo with only advisory linked-issue + multi-stage CI does not reach the 2-of-3 threshold", () => { + // linkedIssueGateMode "advisory" means the linked-issue signal must not count, even though + // requireLinkedIssue/linkedIssuePolicy look strict -- leaving only 1 real signal (multi-stage CI). + const workflow = contributionWorkflow({ requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "advisory", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }); + expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(false); + }); + it("exactly one CI workflow file does not count as multi-stage", () => { const workflow = contributionWorkflow({ gatePublishesCheck: true, ciWorkflowFiles: [".github/workflows/ci.yml"] }); expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract)).toBe(false); @@ -116,13 +134,30 @@ describe("renderRepoSkillContent (#3001)", () => { }); const content = renderRepoSkillContent(profile); expect(content).not.toBeNull(); + expect(content).not.toContain("A linked issue is required"); expect(content).toContain("Required: no"); expect(content).not.toContain("Required: yes"); }); + it("the trigger sentence and the Linked-issues section agree in block mode: both state a linked issue is required (#9671)", () => { + const profile = presentProfile({ + contributionWorkflow: contributionWorkflow({ + gatePublishesCheck: true, + requireLinkedIssue: true, + linkedIssuePolicy: "required", + linkedIssueGateMode: "block", + ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"], + }), + }); + const content = renderRepoSkillContent(profile); + expect(content).not.toBeNull(); + expect(content).toContain('A linked issue is required, with a "required" policy.'); + expect(content).toContain("Required: yes"); + }); + it("omits the gate-check reason line when the trigger fires via linked-issue + multi-stage CI alone (no blocking gate)", () => { const profile = presentProfile({ - contributionWorkflow: contributionWorkflow({ gatePublishesCheck: false, requireLinkedIssue: true, linkedIssuePolicy: "preferred", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), + contributionWorkflow: contributionWorkflow({ gatePublishesCheck: false, requireLinkedIssue: true, linkedIssuePolicy: "preferred", linkedIssueGateMode: "block", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), }); const content = renderRepoSkillContent(profile); expect(content).not.toBeNull(); @@ -133,7 +168,7 @@ describe("renderRepoSkillContent (#3001)", () => { it("omits the multi-stage-CI reason line when the trigger fires via gate + strict linked issue alone (single CI file)", () => { const profile = presentProfile({ - contributionWorkflow: contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", ciWorkflowFiles: [".github/workflows/ci.yml"] }), + contributionWorkflow: contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "block", ciWorkflowFiles: [".github/workflows/ci.yml"] }), }); const content = renderRepoSkillContent(profile); expect(content).not.toBeNull();