diff --git a/.gittensory.yml.example b/.gittensory.yml.example index ab94e80503..b4d3333b2b 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -72,6 +72,9 @@ preferredLabels: # How strongly a linked issue is expected on a PR. # Values: required | preferred | optional. Default: optional. +# `required` also auto-promotes gate.linkedIssue to `block` if it's still `off` (same convergence as the +# dashboard "Require linked issue" toggle) -- this alone is enough to make a missing issue a real gate +# blocker, no need to separately configure gate.linkedIssue too. linkedIssuePolicy: optional # Test paths/areas expected to change alongside code. When code changes without @@ -145,7 +148,8 @@ gate: # Default: advisory (a missing issue is surfaced in the review panel but never blocks the gate; issues # aren't always available, e.g. small/self-evident fixes). Set `block` here, or turn on the dashboard # "Require linked issue" toggle, to make a missing issue an explicit opt-in blocker. - # (If the dashboard toggle is on but this is still `off`, it is auto-promoted to `block`.) + # (If the dashboard toggle is on, OR the top-level linkedIssuePolicy: required below is set, and this is + # still `off`, it is auto-promoted to `block` — all three knobs converge on this one gate.) # This is UNRELATED to closing a PR that links an INELIGIBLE issue (owner-assigned, wrong label, etc.) — # that is a separate, deterministic rule, not this gate. linkedIssue: advisory diff --git a/config/examples/gittensory.full.yml b/config/examples/gittensory.full.yml index 1667cbb52c..656ed73266 100644 --- a/config/examples/gittensory.full.yml +++ b/config/examples/gittensory.full.yml @@ -85,6 +85,9 @@ preferredLabels: # How strongly a linked issue is expected on a PR. # Values: required | preferred | optional. Default: optional. +# `required` also auto-promotes gate.linkedIssue to `block` if it's still `off` (same convergence as the +# dashboard "Require linked issue" toggle) -- this alone is enough to make a missing issue a real gate +# blocker, no need to separately configure gate.linkedIssue too. linkedIssuePolicy: optional # Test paths/areas expected to change alongside code. When code changes without @@ -158,7 +161,8 @@ gate: # Default: advisory (a missing issue is surfaced in the review panel but never blocks the gate; issues # aren't always available, e.g. small/self-evident fixes). Set `block` here, or turn on the dashboard # "Require linked issue" toggle, to make a missing issue an explicit opt-in blocker. - # (If the dashboard toggle is on but this is still `off`, it is auto-promoted to `block`.) + # (If the dashboard toggle is on, OR the top-level linkedIssuePolicy: required below is set, and this is + # still `off`, it is auto-promoted to `block` — all three knobs converge on this one gate.) # This is UNRELATED to closing a PR that links an INELIGIBLE issue (owner-assigned, wrong label, etc.) — # that is a separate, deterministic rule, not this gate. linkedIssue: advisory diff --git a/packages/gittensory-engine/src/advisory/gate-advisory.ts b/packages/gittensory-engine/src/advisory/gate-advisory.ts index f67ea2052a..62fd986eae 100644 --- a/packages/gittensory-engine/src/advisory/gate-advisory.ts +++ b/packages/gittensory-engine/src/advisory/gate-advisory.ts @@ -576,11 +576,20 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli // when the maintainer configured an enforced check). The advisory variant (`pre_merge_check_failed`) is a plain // warning and is never blocked here. No AI judgment is involved, so this can never cause an AI false-close. if (code === "pre_merge_check_required") return true; - // Focus-manifest policy (#555): linked-issue/test policy findings block ONLY when the maintainer opts into - // manifestPolicy: block. Path holds are intentionally separate and configured via hardGuardrailGlobs. - if (code === "manifest_linked_issue_required" || code === "manifest_missing_tests") { + // Focus-manifest missing-tests policy (#555): blocks ONLY when the maintainer opts into manifestPolicy: + // block. Path holds are intentionally separate and configured via hardGuardrailGlobs. + if (code === "manifest_missing_tests") { return gatePolicyBlocks(policy.manifestPolicyGateMode, "off"); } + // Focus-manifest linked-issue policy (#555, #4618): blocks when EITHER the manifest-policy gate OR the + // linked-issue gate is opted into block. resolveEffectiveSettings promotes linkedIssueGateMode to "block" + // whenever the yml-only `linkedIssuePolicy: required` knob is set (mirroring the requireLinkedIssue + // promotion), so this finding's own escalation must honor that gate too -- not just manifestPolicyGateMode + // -- or the promotion would have no actual blocking effect and the config-surface-reduction fix would be a + // no-op. + if (code === "manifest_linked_issue_required") { + return gatePolicyBlocks(policy.manifestPolicyGateMode, "off") || gatePolicyBlocks(policy.linkedIssueGateMode, "advisory"); + } // Self-authored linked-issue gate: blocks only when the maintainer opts in with `block`. Defaults to // advisory — the finding surfaces in the panel without ever closing the PR unless explicitly configured. if (code === "self_authored_linked_issue") return gatePolicyBlocks(policy.selfAuthoredLinkedIssueGateMode, "advisory"); diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 281004b7bf..c6265a4306 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -946,11 +946,20 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli // when the maintainer configured an enforced check). The advisory variant (`pre_merge_check_failed`) is a plain // warning and is never blocked here. No AI judgment is involved, so this can never cause an AI false-close. if (code === "pre_merge_check_required") return true; - // Focus-manifest policy (#555): linked-issue/test policy findings block ONLY when the maintainer opts into - // manifestPolicy: block. Path holds are intentionally separate and configured via hardGuardrailGlobs. - if (code === "manifest_linked_issue_required" || code === "manifest_missing_tests") { + // Focus-manifest missing-tests policy (#555): blocks ONLY when the maintainer opts into manifestPolicy: + // block. Path holds are intentionally separate and configured via hardGuardrailGlobs. + if (code === "manifest_missing_tests") { return gateMode(policy.manifestPolicyGateMode ?? "off") === "block"; } + // Focus-manifest linked-issue policy (#555, #4618): blocks when EITHER the manifest-policy gate OR the + // linked-issue gate is opted into block. resolveEffectiveSettings promotes linkedIssueGateMode to "block" + // whenever the yml-only `linkedIssuePolicy: required` knob is set (mirroring the requireLinkedIssue + // promotion), so this finding's own escalation must honor that gate too -- not just manifestPolicyGateMode + // -- or the promotion would have no actual blocking effect and the config-surface-reduction fix would be a + // no-op. + if (code === "manifest_linked_issue_required") { + return gateMode(policy.manifestPolicyGateMode ?? "off") === "block" || gateMode(policy.linkedIssueGateMode ?? "advisory") === "block"; + } // Self-authored linked-issue gate: blocks only when the maintainer opts in with `block`. Defaults to // advisory — the finding surfaces in the panel without ever closing the PR unless explicitly configured. if (code === "self_authored_linked_issue") return gateMode(policy.selfAuthoredLinkedIssueGateMode ?? "advisory") === "block"; diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 30b6e94685..be7118e998 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -590,7 +590,11 @@ export function resolveEffectiveSettings( applyGateConfigOverrides(effective, manifest.gate); // The dashboard "Require linked issue" toggle must not silently diverge from gate blocking: when the // boolean is on but linkedIssueGateMode is still off, treat it as a block requirement (#797). - if (effective.requireLinkedIssue && effective.linkedIssueGateMode === "off") { + // #4618: the yml-only top-level `linkedIssuePolicy: required` knob gets the same promotion -- previously a + // self-hoster who set ONLY this (never touching the differently-worded `gate.linkedIssue: block`) got an + // advisory `manifest_linked_issue_required` nudge but no real gate blocker, a silent no-op that could only + // be discovered by cross-referencing a completely different section of the config file. + if ((effective.requireLinkedIssue || manifest.linkedIssuePolicy === "required") && effective.linkedIssueGateMode === "off") { effective.linkedIssueGateMode = "block"; } // Readiness/quality can never hard-block a PR (buildQualityGateWarning is always advisory-severity; diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 01eb25d330..ba2a38ae4c 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -3079,6 +3079,30 @@ describe("parseFocusManifest settings override + resolveEffectiveSettings", () = expect(eff.linkedIssueGateMode).toBe("block"); }); + it("promotes yml linkedIssuePolicy: required to linkedIssueGateMode block when the gate mode is still off (#4618)", () => { + const eff = resolveEffectiveSettings( + { requireLinkedIssue: false, linkedIssueGateMode: "off" } as RepositorySettings, + parseFocusManifest({ linkedIssuePolicy: "required" }), + ); + expect(eff.linkedIssueGateMode).toBe("block"); + }); + + it("linkedIssuePolicy: preferred does NOT promote linkedIssueGateMode -- only required does (#4618)", () => { + const eff = resolveEffectiveSettings( + { requireLinkedIssue: false, linkedIssueGateMode: "off" } as RepositorySettings, + parseFocusManifest({ linkedIssuePolicy: "preferred" }), + ); + expect(eff.linkedIssueGateMode).toBe("off"); + }); + + it("linkedIssuePolicy: required does NOT downgrade an explicit gate.linkedIssue: advisory override (gate: wins, #4618)", () => { + const eff = resolveEffectiveSettings( + { requireLinkedIssue: false, linkedIssueGateMode: "off" } as RepositorySettings, + parseFocusManifest({ linkedIssuePolicy: "required", gate: { linkedIssue: "advisory" } }), + ); + expect(eff.linkedIssueGateMode).toBe("advisory"); + }); + it("REGRESSION: downgrades a pre-existing DB qualityGateMode: block to advisory, even with no gate.readiness.mode override (#2267)", () => { // Simulates a repo whose DB row already has quality_gate_mode = "block" from before the write-time guards // (the settings.qualityGateMode parser, the settings-write API routes) existed — the dashboard/API path's diff --git a/test/unit/gate-check-policy.test.ts b/test/unit/gate-check-policy.test.ts index 9815779dc3..91ad56bf2f 100644 --- a/test/unit/gate-check-policy.test.ts +++ b/test/unit/gate-check-policy.test.ts @@ -674,6 +674,50 @@ describe("focus-manifest policy gate (#555)", () => { }); } + describe("manifest_linked_issue_required also escalates via linkedIssueGateMode (#4618)", () => { + // #4618: resolveEffectiveSettings promotes linkedIssueGateMode to "block" whenever the yml-only + // linkedIssuePolicy: required knob is set (mirroring the requireLinkedIssue promotion). That promotion + // is a no-op unless this finding's own escalation actually honors linkedIssueGateMode too -- it must NOT + // require manifestPolicyGateMode: block as well, or a self-hoster who only ever sets linkedIssuePolicy + // still gets no real blocker. manifest_missing_tests is unaffected -- it stays keyed to manifestPolicyGateMode alone. + it("blocks a confirmed contributor via linkedIssueGateMode: block ALONE, with manifestPolicyGateMode entirely omitted (exercises its ?? 'off' fallback)", () => { + const result = evaluateGateCheck(manifestAdvisory("manifest_linked_issue_required"), { linkedIssueGateMode: "block", confirmedContributor: true }); + expect(result.conclusion).toBe("failure"); + expect(result.blockers.map((finding) => finding.code)).toContain("manifest_linked_issue_required"); + }); + + it("still blocks via manifestPolicyGateMode: block ALONE, with linkedIssueGateMode advisory (back-compat)", () => { + const result = evaluateGateCheck(manifestAdvisory("manifest_linked_issue_required"), { manifestPolicyGateMode: "block", linkedIssueGateMode: "advisory", confirmedContributor: true }); + expect(result.conclusion).toBe("failure"); + expect(result.blockers.map((finding) => finding.code)).toContain("manifest_linked_issue_required"); + }); + + it("stays advisory-only when BOTH gates are off/advisory", () => { + expect(evaluateGateCheck(manifestAdvisory("manifest_linked_issue_required"), { manifestPolicyGateMode: "off", linkedIssueGateMode: "advisory", confirmedContributor: true }).conclusion).toBe("success"); + }); + + it("manifest_missing_tests is NOT escalated by linkedIssueGateMode -- only manifestPolicyGateMode governs it", () => { + const result = evaluateGateCheck(manifestAdvisory("manifest_missing_tests"), { manifestPolicyGateMode: "off", linkedIssueGateMode: "block", confirmedContributor: true }); + expect(result.conclusion).toBe("success"); + }); + + it("manifest_missing_tests stays non-blocking under the manifestPolicyGateMode ?? 'off' fallback when the policy field is entirely omitted", () => { + expect(evaluateGateCheck(manifestAdvisory("manifest_missing_tests"), { confirmedContributor: true }).conclusion).toBe("success"); + }); + + it("manifest_linked_issue_required stays non-blocking under the linkedIssueGateMode ?? 'advisory' fallback when that field is entirely omitted (manifestPolicyGateMode off)", () => { + expect(evaluateGateCheck(manifestAdvisory("manifest_linked_issue_required"), { manifestPolicyGateMode: "off", confirmedContributor: true }).conclusion).toBe("success"); + }); + + it("end-to-end: linkedIssuePolicy: required alone (no gate.linkedIssue, no manifestPolicy) produces a real gate blocker", () => { + const eff = resolveEffectiveSettings(settings({ linkedIssueGateMode: "off", manifestPolicyGateMode: "off" }), parseFocusManifest({ linkedIssuePolicy: "required" })); + expect(eff.linkedIssueGateMode).toBe("block"); + const result = evaluateGateCheck(manifestAdvisory("manifest_linked_issue_required"), gateCheckPolicy(eff, null, true)); + expect(result.conclusion).toBe("failure"); + expect(result.blockers.map((finding) => finding.code)).toContain("manifest_linked_issue_required"); + }); + }); + it("ignores legacy manifest_blocked_path findings even when manifestPolicy:block is enabled", () => { const advisory: Advisory = { ...missingIssueAdvisory(),