Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/review/repo-skill-render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions test/unit/repo-doc-pr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,13 @@ async function seedRepoDocGenerationConfig(env: ReturnType<typeof createTestEnv>

// #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<typeof createTestEnv>, repoFullName: string, scope: string[] = ["agents", "skills"], overrides: { allowOverwriteExisting?: boolean } = {}): Promise<void> {
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");
}
Expand Down
47 changes: 41 additions & 6 deletions test/unit/repo-skill-render.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RepoProfile, { present: true }>)).toBe(expected);
});
Expand All @@ -42,6 +42,24 @@ describe("shouldGenerateRepoSkill (#3001)", () => {
expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: workflow }) as Extract<RepoProfile, { present: true }>)).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<RepoProfile, { present: true }>)).toBe(false);

const gateOnlyBlockLinkedIssue = contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "block" });
expect(shouldGenerateRepoSkill(presentProfile({ contributionWorkflow: gateOnlyBlockLinkedIssue }) as Extract<RepoProfile, { present: true }>)).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<RepoProfile, { present: true }>)).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<RepoProfile, { present: true }>)).toBe(false);
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down