fix(gate): downgrade readiness gate mode block to advisory - #2384
Conversation
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-01 19:17:40 UTC
✅ Suggested Action - Approve/Merge
Review summary Blockers
Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2384 +/- ##
=======================================
Coverage 95.84% 95.84%
=======================================
Files 224 224
Lines 25042 25048 +6
Branches 9103 9106 +3
=======================================
+ Hits 24001 24007 +6
Misses 427 427
Partials 614 614
🚀 New features to boost your workflow:
|
gate.readiness.mode accepted the shared off/advisory/block tri-state, but buildQualityGateWarning always produces a warning-severity finding and isConfiguredGateBlocker has no branch for it — readiness/quality is intentionally informational-only and can never hard-block a PR. A maintainer setting mode: block believed a real quality floor was enforced when the effective behavior was silently advisory-only. Add normalizeReadinessGateMode to downgrade block to advisory at the config-as-code parse layer, with a deprecation warning pointing at mergeReadiness/manifestPolicy for an enforceable floor. The shared normalizeOptionalGateMode is untouched since other gate fields legitimately support block.
…board/API normalizeReadinessGateMode downgrades gate.readiness.mode: block to advisory at parse time, but that only covered the YAML gate: mapping. The identically named qualityGateMode field reachable through .gittensory.yml's settings: override, the maintainer dashboard save, and the internal settings write endpoint all bypassed it, so a value supplied through any of those paths still persisted and resolved as "block" — preserving the exact false belief that a real quality floor is enforced. Close it at every point the value can enter or be read: - parseSettingsOverride now normalizes settings.qualityGateMode the same way as gate.readiness.mode (exported for reuse). - Both settings-write API routes (maintainer PATCH-style save and the internal full-settings write) downgrade a written "block" before it reaches the DB. - resolveEffectiveSettings downgrades "block" as a final step regardless of source, so a repo whose DB row already has quality_gate_mode = "block" from before these guards existed is also corrected wherever effective settings are read (the review/gate pipeline and the settings-preview dashboard both go through this same resolver).
1173ce8 to
2193bde
Compare
What
gate.readiness.modein.gittensory.yml(and the equivalent dashboard/APIqualityGateModefield) is documented and parsed as the sharedoff | advisory | blocktri-state used by every other gate dimension. In practice it can never block anything.Fix
buildQualityGateWarning(src/rules/advisory.ts) always produces a warning-severity finding, andisConfiguredGateBlockerhas no branch for the readiness/quality dimension at all — this is a deliberate, pre-existing design (readiness is intentionally informational-only). But the config parser silently acceptedmode: blockanyway, so a maintainer who set it believed a real quality floor was enforced, when the effective behavior was silently advisory-only.Added
normalizeReadinessGateModeinsrc/signals/focus-manifest.ts, wired intoparseGateConfigfor thegate.readiness.modefield only. It downgradesblock→advisoryat parse time and pushes a clear deprecation warning pointing atgate.mergeReadinessorgate.manifestPolicyfor an enforceable quality floor. The sharednormalizeOptionalGateModehelper itself is untouched, since it's reused by other gate fields (linkedIssue,duplicates,aiReview, etc.) that legitimately supportblock.Updated
.gittensory.yml.example'sreadiness.modedoc comment to state onlyoff | advisoryare accepted, with a note on the downgrade-with-warning behavior.Deliberately out of scope:
src/api/routes.ts(repositorySettingsSchema/maintainerSettingsSchema) andsrc/openapi/schemas.ts(RepositorySettingsSchema/RepoSettingsPreviewSchema) still accept"block"forqualityGateModein their Zod enums. These are read/response schemas as well as write-validation — narrowing them risks breaking API responses for any repo that already hasquality_gate_mode = 'block'stored in the DB (deserialization would fail). Since the runtime already treats"block"as fully inert regardless of what's stored, leaving those schemas as-is is a safe, intentional deferral rather than an oversight.Tests
parseFocusManifestdowngradesgate.readiness.mode: block→advisorywith a deprecation warning matching/gate\.readiness\.mode.*no longer accepts "block"/; an invalid mode still produces the normal "must be one of" warning and not the block-specific one.focus-manifest.test.tstests that previously usedmode: "block"as incidental input for unrelated assertions — switched to"advisory"to preserve their original intent.gate-check-policy.test.ts's "overlays the friendly gate alias over DB settings" test: keptreadiness: { mode: "block", minScore: 70 }as input (now a deliberate higher-level check) and assertedeff.qualityGateModeresolves to"advisory", confirming the downgrade propagates throughresolveEffectiveSettings.npx tsc --noEmitclean.focus-manifest.test.ts+gate-check-policy.test.ts— 222 passed.src/signals/focus-manifest.ts— 278 passed.src/signals/focus-manifest.ts: fully covered.npm run test:coverage: 5601 passed, 4 skipped (pre-existing/unrelated), 0 failed.npm audit --audit-level=moderate: 0 vulnerabilities.Advances #1936. Closes #2267.