From d30dfbed73481ff2395811d2079606389821a050 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Sun, 12 Jul 2026 06:18:34 -0700 Subject: [PATCH] fix(review): generated contributor docs no longer overclaim requireLinkedIssue enforcement requireLinkedIssue alone only surfaces the missing_linked_issue advisory finding -- it never blocks a PR on its own (the real authority is linkedIssueGateMode === "block", default "advisory"). Generated docs (repo-skill-render.ts, repo-doc-render.ts) rendered "Required: yes" from requireLinkedIssue alone, so the common "toggle requireLinkedIssue, never touch the gate-mode dropdown" case produced docs claiming enforcement that wasn't actually happening. Threads linkedIssueGateMode through RepoProfileContributionWorkflow and keys the rendered text off it instead. Also corrects the overstating db/schema.ts comment and adds a doc comment to requireLinkedIssue itself. Refs #5287 --- src/db/schema.ts | 7 +++++-- src/review/repo-doc-render.ts | 2 +- src/review/repo-profile.ts | 5 +++++ src/review/repo-skill-render.ts | 2 +- src/types.ts | 6 ++++++ test/unit/generated-doc-refresh.test.ts | 2 +- test/unit/repo-doc-render.test.ts | 16 ++++++++++++---- test/unit/repo-profile.test.ts | 1 + test/unit/repo-skill-render.test.ts | 22 ++++++++++++++++++++-- 9 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/db/schema.ts b/src/db/schema.ts index 6c7cf0ab4..2f2a6bf76 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -59,8 +59,11 @@ export const repositorySettings = sqliteTable("repository_settings", { autoProjectMilestoneMatchBackend: text("auto_project_milestone_match_backend").notNull().default("github"), gatePack: text("gate_pack").notNull().default("gittensor"), // Missing a linked issue is advisory-only by default -- issues aren't always available, so it only - // blocks when a repo explicitly opts in (linkedIssueGateMode: "block" or the requireLinkedIssue toggle; - // see resolveEffectiveSettings in signals/focus-manifest.ts). This default was "block" until #selfhost- + // blocks when a repo explicitly sets linkedIssueGateMode: "block". The sibling requireLinkedIssue + // toggle does NOT block on its own: it only surfaces the missing_linked_issue advisory finding, and + // the one path where it promotes to a real block (resolveEffectiveSettings in + // signals/focus-manifest.ts) only fires when linkedIssueGateMode is explicitly "off" -- never at this + // "advisory" default. This default was "block" until #selfhost- // linked-issue-gate-drift, which also backfills any row an earlier migration (0023/0025) persisted as // "block" without an explicit opt-in (migrations/0102_fix_linked_issue_gate_mode_default.sql). The raw // SQLite column still has a DEFAULT 'block' from migration 0023 -- SQLite has no ALTER COLUMN SET DEFAULT, diff --git a/src/review/repo-doc-render.ts b/src/review/repo-doc-render.ts index 09c5f7120..b7891b929 100644 --- a/src/review/repo-doc-render.ts +++ b/src/review/repo-doc-render.ts @@ -103,7 +103,7 @@ ${renderCommandList("Lint", commands.lintCommands, commands.packageManager)} - CI publishes a required check: ${contributionWorkflow.gatePublishesCheck ? "yes" : "no"} - Linked-issue policy: ${contributionWorkflow.linkedIssuePolicy} -- Requires a linked issue: ${contributionWorkflow.requireLinkedIssue ? "yes" : "no"} +- Requires a linked issue: ${contributionWorkflow.linkedIssueGateMode === "block" ? "yes" : "no"} - CI workflow files: ${renderCiWorkflowFiles(contributionWorkflow.ciWorkflowFiles)} diff --git a/src/review/repo-profile.ts b/src/review/repo-profile.ts index b6bb1e5d1..7a34d227b 100644 --- a/src/review/repo-profile.ts +++ b/src/review/repo-profile.ts @@ -22,6 +22,7 @@ import { countRepoChunks } from "./rag"; import { resolveRepositorySettings } from "../settings/repository-settings"; import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { nowIso } from "../utils/json"; +import type { GateRuleMode } from "../types"; /** Bumped whenever the profile SHAPE changes (not on every content tweak) -- at least three separate features * consume this profile and must be able to evolve independently of each other and of this extractor. */ @@ -62,6 +63,9 @@ export type RepoProfileContributionWorkflow = { gatePublishesCheck: boolean; linkedIssuePolicy: "required" | "preferred" | "optional"; requireLinkedIssue: boolean; + /** The actual enforcement authority for whether a missing linked issue blocks a PR -- `requireLinkedIssue` + * alone does not block (see its doc comment on {@link RepositorySettings}); only `"block"` does. */ + linkedIssueGateMode: GateRuleMode; /** Indexed `.github/workflows/*.yml`/`*.yaml` paths -- describes CI structure without hard-coding assumptions * about what workflows exist. Empty when the repo has no indexed workflow files (which may just mean RAG's * code-only filter or a small chunk budget hasn't reached them yet, not that none exist). */ @@ -271,6 +275,7 @@ export async function extractRepoProfile(env: Env, repoFullName: string, options gatePublishesCheck: settings.reviewCheckMode !== "disabled", linkedIssuePolicy: manifest.linkedIssuePolicy, requireLinkedIssue: settings.requireLinkedIssue, + linkedIssueGateMode: settings.linkedIssueGateMode, ciWorkflowFiles: deriveCiWorkflowFiles(paths), }, }; diff --git a/src/review/repo-skill-render.ts b/src/review/repo-skill-render.ts index 86533f5c4..ce1859c79 100644 --- a/src/review/repo-skill-render.ts +++ b/src/review/repo-skill-render.ts @@ -118,7 +118,7 @@ ${renderTriggerReasons(contributionWorkflow)} ## Linked issues - Policy: ${contributionWorkflow.linkedIssuePolicy} -- Required: ${contributionWorkflow.requireLinkedIssue ? "yes" : "no"} +- Required: ${contributionWorkflow.linkedIssueGateMode === "block" ? "yes" : "no"} ${REPO_SKILL_MARKER_END} `; } diff --git a/src/types.ts b/src/types.ts index 866922e3f..6acbb6fcf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -946,6 +946,12 @@ export type RepositorySettings = { advisoryAiRouting?: AdvisoryAiRoutingConfig | undefined; publicSurface: "off" | "comment_and_label" | "comment_only" | "label_only"; includeMaintainerAuthors: boolean; + /** Surfaces the `missing_linked_issue` advisory finding in the review comment -- does NOT block a PR on + * its own. The real blocking authority is {@link RepositorySettings.linkedIssueGateMode} `=== "block"` + * (default `"advisory"`); the one place this flag alone promotes to a real block + * (`resolveEffectiveSettings` in `src/signals/focus-manifest.ts`) only fires when `linkedIssueGateMode` + * is explicitly `"off"`, so at the default it never does. A maintainer wanting a real linked-issue + * requirement must set `linkedIssueGateMode: "block"`, not just this toggle. */ requireLinkedIssue: boolean; backfillEnabled: boolean; /** Opt-in for the public, unauthenticated README status badge (#541). Always populated by the DB layer diff --git a/test/unit/generated-doc-refresh.test.ts b/test/unit/generated-doc-refresh.test.ts index 5f59431b1..9a4be4e4c 100644 --- a/test/unit/generated-doc-refresh.test.ts +++ b/test/unit/generated-doc-refresh.test.ts @@ -18,7 +18,7 @@ function fixtureProfile(): RepoProfile { architecture: { indexedFileCount: 3, topLevelDirectories: [{ path: "src", fileCount: 3 }] }, conventions: { fileNamingStyle: "kebab-case", testFileConvention: "dot-test-suffix" }, commands: { packageManager: "npm", buildCommands: ["build"], testCommands: ["test"], lintCommands: [] }, - contributionWorkflow: { gatePublishesCheck: true, linkedIssuePolicy: "preferred", requireLinkedIssue: false, ciWorkflowFiles: [] }, + contributionWorkflow: { gatePublishesCheck: true, linkedIssuePolicy: "preferred", requireLinkedIssue: false, linkedIssueGateMode: "advisory", ciWorkflowFiles: [] }, }; } diff --git a/test/unit/repo-doc-render.test.ts b/test/unit/repo-doc-render.test.ts index bf79cee12..a537405e0 100644 --- a/test/unit/repo-doc-render.test.ts +++ b/test/unit/repo-doc-render.test.ts @@ -13,7 +13,7 @@ function presentProfile(overrides: Partial { }); it("renders 'none indexed' when no CI workflow files were found", () => { - const content = renderRepoDocContent(presentProfile({ contributionWorkflow: { gatePublishesCheck: false, linkedIssuePolicy: "optional", requireLinkedIssue: false, ciWorkflowFiles: [] } }), GITTENSORY_SITE_URL); + const content = renderRepoDocContent(presentProfile({ contributionWorkflow: { gatePublishesCheck: false, linkedIssuePolicy: "optional", requireLinkedIssue: false, linkedIssueGateMode: "advisory", ciWorkflowFiles: [] } }), GITTENSORY_SITE_URL); expect(content).toContain("CI publishes a required check: no"); expect(content).toContain("Requires a linked issue: no"); expect(content).toContain("- none indexed"); }); - it("renders 'yes' for requireLinkedIssue when the setting is on", () => { - const content = renderRepoDocContent(presentProfile({ contributionWorkflow: { gatePublishesCheck: true, linkedIssuePolicy: "required", requireLinkedIssue: true, ciWorkflowFiles: [] } }), GITTENSORY_SITE_URL); + it("renders 'yes' for requireLinkedIssue only when linkedIssueGateMode is 'block'", () => { + const content = renderRepoDocContent(presentProfile({ contributionWorkflow: { gatePublishesCheck: true, linkedIssuePolicy: "required", requireLinkedIssue: true, linkedIssueGateMode: "block", ciWorkflowFiles: [] } }), GITTENSORY_SITE_URL); expect(content).toContain("Requires a linked issue: yes"); }); + it("does not claim a linked issue is required when requireLinkedIssue is on but linkedIssueGateMode is the advisory default (#5287)", () => { + // requireLinkedIssue alone only surfaces an advisory finding -- it never blocks on its own, so the + // generated doc must not claim "yes" unless linkedIssueGateMode is actually "block". + const content = renderRepoDocContent(presentProfile({ contributionWorkflow: { gatePublishesCheck: true, linkedIssuePolicy: "required", requireLinkedIssue: true, linkedIssueGateMode: "advisory", ciWorkflowFiles: [] } }), GITTENSORY_SITE_URL); + expect(content).toContain("Requires a linked issue: no"); + expect(content).not.toContain("Requires a linked issue: yes"); + }); + const namingStyles: Array<[RepoProfileFileNamingStyle, string]> = [ ["kebab-case", "kebab-case (`my-file.ts`)"], ["camelCase", "camelCase (`myFile.ts`)"], diff --git a/test/unit/repo-profile.test.ts b/test/unit/repo-profile.test.ts index eb1cb6378..0a9fdf1d7 100644 --- a/test/unit/repo-profile.test.ts +++ b/test/unit/repo-profile.test.ts @@ -73,6 +73,7 @@ describe("extractRepoProfile (#2999)", () => { gatePublishesCheck: true, linkedIssuePolicy: "required", requireLinkedIssue: true, + linkedIssueGateMode: "advisory", ciWorkflowFiles: [".github/workflows/ci.yml"], }); }); diff --git a/test/unit/repo-skill-render.test.ts b/test/unit/repo-skill-render.test.ts index 80c1449b2..c210648cd 100644 --- a/test/unit/repo-skill-render.test.ts +++ b/test/unit/repo-skill-render.test.ts @@ -4,7 +4,7 @@ import type { RepoProfile, RepoProfileContributionWorkflow } from "../../src/rev import { REPO_PROFILE_SCHEMA_VERSION } from "../../src/review/repo-profile"; function contributionWorkflow(overrides: Partial = {}): RepoProfileContributionWorkflow { - return { gatePublishesCheck: false, linkedIssuePolicy: "optional", requireLinkedIssue: false, ciWorkflowFiles: [], ...overrides }; + return { gatePublishesCheck: false, linkedIssuePolicy: "optional", requireLinkedIssue: false, linkedIssueGateMode: "advisory", ciWorkflowFiles: [], ...overrides }; } function presentProfile(overrides: Partial> = {}): RepoProfile { @@ -83,7 +83,7 @@ describe("renderRepoSkillContent (#3001)", () => { it("renders the marker, frontmatter, trigger reasons, and command/linked-issue sections when the trigger fires", () => { const profile = presentProfile({ repoFullName: "owner/widgets", - contributionWorkflow: contributionWorkflow({ gatePublishesCheck: true, requireLinkedIssue: true, linkedIssuePolicy: "required", ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"] }), + 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(); @@ -102,6 +102,24 @@ describe("renderRepoSkillContent (#3001)", () => { expect(content).toContain("Required: yes"); }); + it("does not claim a linked issue is required when requireLinkedIssue is on but linkedIssueGateMode is the advisory default (#5287)", () => { + // requireLinkedIssue alone only surfaces an advisory finding -- it never blocks on its own, so the + // generated doc must not claim "Required: yes" unless linkedIssueGateMode is actually "block". + const profile = presentProfile({ + contributionWorkflow: contributionWorkflow({ + gatePublishesCheck: true, + requireLinkedIssue: true, + linkedIssuePolicy: "required", + linkedIssueGateMode: "advisory", + ciWorkflowFiles: [".github/workflows/a.yml", ".github/workflows/b.yml"], + }), + }); + const content = renderRepoSkillContent(profile); + expect(content).not.toBeNull(); + expect(content).toContain("Required: no"); + expect(content).not.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"] }),