fix(engine): correct mergeReadinessGateMode doc comment's sub-gate count#7288
Conversation
The doc comment claimed the merge-readiness composite overrides four sub-gates (linked-issue, duplicate, quality/readiness, slop), but applyMergeReadinessGate only ever escalates three (linked-issue, duplicate, slop) — buildQualityGateWarning can only ever produce an advisory warning, never a blocker, so quality/readiness always stays advisory-only regardless of this field's mode. Closes JSONbored#7242
…host twin src/rules/advisory.ts's mergeReadinessGateMode field carried the same incorrect claim as its packages/loopover-engine twin (that the composite overrides all four sub-gates including quality/readiness), which contradicts applyMergeReadinessGate's own inline comment a few lines below stating quality/readiness is deliberately excluded.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7288 +/- ##
=======================================
Coverage 93.81% 93.81%
=======================================
Files 704 704
Lines 69462 69462
Branches 18896 18896
=======================================
Hits 65167 65167
Misses 3302 3302
Partials 993 993
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-19 12:52:33 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
GateCheckPolicy.mergeReadinessGateMode's doc comment (packages/loopover-engine/src/advisory/gate-advisory.ts) claimed the merge-readiness composite (feat(github-app): merge-readiness aggregate gate #551) overrides four sub-gates when set: linked-issue, duplicate, quality/readiness, and slop.applyMergeReadinessGate(same file) only ever escalates three — linked-issue, duplicate, slop. Quality/readiness findings come frombuildQualityGateWarning, which can only ever produce an advisorywarningsentry, never ablockersentry, regardless of any gate mode — so it always stays advisory-only.src/rules/advisory.ts(the hand-duplicated host twin of the engine file, perscripts/check-engine-parity.ts'sGATE_DECISION_TWIN_PAIR) carried the identical incorrect claim on its ownmergeReadinessGateModefield — its ownapplyMergeReadinessGatefunction a few lines below already states "Readiness/quality is intentionally excluded" in a code comment, directly contradicting the field's doc comment above it. Mirrored the same correction there.src/types.ts(mergeReadinessGateMode: GateRuleMode, line 881) — its doc comment is terse ("Merge-readiness gate (#merge-readiness).off/advisory/block. No min-score. Defaultoff.") but doesn't make the same incorrect claim, so it needed no change.test/unit/gate-check-policy.test.ts'sdescribe("merge-readiness composite gate (#551)", ...)block asserting slop escalates into a hard blocker undermergeReadinessGateMode: "block"— the block already had dedicated tests for linked-issue and duplicate escalation (and for quality/readiness staying advisory-only), but not slop.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #7242
Validation
git diff --check— clean.npm run actionlint— clean.npm run typecheck— the whole-repotsc --noEmitreliably OOMs on this shared, memory-constrained sandbox regardless of diff size (documented pattern from prior PRs in this repo's history). Verified instead with: (1)tsc --noEmit -p packages/loopover-engine/tsconfig.json— the wholeloopover-engineworkspace package (small enough to avoid the OOM), clean; (2) a standalone scopedtsc --noEmitagainstsrc/rules/advisory.tsand the changed test files using this project's exact roottsconfig.jsoncompiler options (--strict --exactOptionalPropertyTypes --noUncheckedIndexedAccess --noImplicitOverride --noFallthroughCasesInSwitch --forceConsistentCasingInFileNames), clean.npx vitest run test/unit/gate-check-policy.test.ts test/unit/check-engine-parity-script.test.ts— 155/155 passing, including the new slop-escalation regression test and the full engine-parity suite.packages/loopover-engine/**,src/rules/**, andtest/**don't add any new logic paths — no coverage delta beyond the new test's own assertions.npm run test:workers/npm run build:mcp/npm run test:mcp-pack/npm run ui:openapi:check(not applicable — no Worker route, MCP, UI, or OpenAPI surface changed)npm audit --audit-level=moderate(no dependency changes)If any required check was skipped, explain why:
npm run typecheckOOMs on this specific sandbox under current memory pressure regardless of diff size; the two scopedtsc --noEmitchecks above (full package + exact-flags scoped check) are the local proxy, and CI's isolated runner performs the real whole-repotsc --noEmit.packages/loopover-engineside) was closed by CI:scripts/check-engine-parity.ts'scheckGateDecisionVersionBumptripwire (test/unit/check-engine-parity-script.test.ts) requires a single-sided edit to either of theGATE_DECISION_TWIN_PAIRfiles (src/rules/advisory.ts/packages/loopover-engine/src/advisory/gate-advisory.ts) to be matched by an edit to the other twin, or apackages/loopover-engine/package.jsonversion bump — my scoped local verification only ran the engine-packagetscbuild and the directly-relevant unit test file, and didn't include this repo-wide drift-tripwire test, so it missed the failure CI's full suite caught. Root-caused and fixed by mirroring the same doc-comment correction intosrc/rules/advisory.ts(a genuine fix, not a workaround — that file had the identical incorrect claim), then re-verified viacheck-engine-parity-script.test.ts's full 38-test suite locally — clean.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots. (N/A — no UI changes.)