Skip to content

fix(rules): surface missing-linked-issue in predictor under composite mergeReadiness gate - #1447

Merged
JSONbored merged 3 commits into
JSONbored:mainfrom
joaovictor91123:fix/predicted-gate-merge-readiness-linked-issue
Jun 28, 2026
Merged

fix(rules): surface missing-linked-issue in predictor under composite mergeReadiness gate#1447
JSONbored merged 3 commits into
JSONbored:mainfrom
joaovictor91123:fix/predicted-gate-merge-readiness-linked-issue

Conversation

@joaovictor91123

Copy link
Copy Markdown
Contributor

Summary

Fixes #1446. The pre-submission gate predictor (buildPredictedGateVerdict, src/rules/predicted-gate.ts) derived requireLinkedIssue from gate.linkedIssue alone, omitting the composite merge-readiness term. Since applyMergeReadinessGate (src/rules/advisory.ts) forces the linked-issue sub-gate to block when mergeReadiness is enabled, and the live review path collects linked-issue evidence whenever merge-readiness is on:

// src/queue/processors.ts — shouldCollectLinkedIssueEvidence
return settings.requireLinkedIssue || settings.linkedIssueGateMode !== "off" || mergeReadinessGateEnabled(settings);

a repo with gate.mergeReadiness: block and gate.linkedIssue unset got a false success prediction while the live gate creates the missing_linked_issue finding and one-shot auto-closes the PR. The predictor's contract is to compute the same verdict the live gate would, so this defeats its purpose and causes the exact loss it exists to prevent.

The fix derives requireLinkedIssue from the merge-readiness term as well, mirroring shouldCollectLinkedIssueEvidence:

const requireLinkedIssue =
  (gate.linkedIssue !== null && gate.linkedIssue !== "off") || (gate.mergeReadiness !== null && gate.mergeReadiness !== "off");

A regression test pins both the new blocker path (mergeReadiness:block + linkedIssue unset → missing_linked_issue blocker → failure) and the false/false arm (neither set → no finding); it fails before the fix and passes after.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; both sides of the new || are covered (mergeReadiness term true via the new blocker test; both-false via the no-gate arm; the linkedIssue term remains covered by the existing linkedIssue:block test).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

If any required check was skipped, explain why:

  • Ran the full npm run test:ci gate locally; all checks green.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

This is a backend predictor-logic change with no UI/API surface impact, so the UI Evidence section is omitted.

Notes

  • Self-contained: one expression in predicted-gate.ts plus a regression test. No schema, OpenAPI, wrangler, or migration impact; applyMergeReadinessGate/evaluateGateCheck already do the right thing once the finding exists.

… mergeReadiness gate

The pre-submission gate predictor derived requireLinkedIssue from gate.linkedIssue alone, omitting the composite mergeReadiness term. Since applyMergeReadinessGate forces the linked-issue sub-gate to block when mergeReadiness is enabled, and the live path collects linked-issue evidence whenever merge-readiness is on (shouldCollectLinkedIssueEvidence), a repo with mergeReadiness:block and linkedIssue unset got a false 'success' prediction while the live gate one-shot auto-closes the PR on the missing-linked-issue blocker. Include the mergeReadiness term so the predictor matches the live gate, and add a regression test pinning both the new blocker path and the no-gate arm.
@dosubot dosubot Bot added the size:XS label Jun 26, 2026
@superagent-security

superagent-security Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Superagent did not find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.53%. Comparing base (59492e6) to head (a64dcd1).
⚠️ Report is 35 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1447   +/-   ##
=======================================
  Coverage   95.53%   95.53%           
=======================================
  Files         204      204           
  Lines       22090    22090           
  Branches     7975     7975           
=======================================
  Hits        21103    21103           
  Misses        412      412           
  Partials      575      575           
Files with missing lines Coverage Δ
src/rules/predicted-gate.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review — held for maintainer review

2 files · 1 AI reviewers · no blockers · readiness 73/100 · CI green · clean

⏸️ Held for maintainer review — Touches a guarded path — held for manual review

Review summary
The change correctly closes a real predictor/live-gate parity gap: when `gate.linkedIssue` is unset but `gate.mergeReadiness` is active, the old predictor derived `requireLinkedIssue = false`, never created the `missing_linked_issue` finding in the advisory, and returned a false `success` — while the live gate (via `applyMergeReadinessGate` + `shouldCollectLinkedIssueEvidence`) would collect linked-issue evidence, surface the finding, and one-shot close the PR. The new OR term `|| (gate.mergeReadiness !== null && gate.mergeReadiness !== 'off')` mirrors `shouldCollectLinkedIssueEvidence` exactly. The regression test is well-constructed: it exercises both the new blocker arm (mergeReadiness:block + linkedIssue unset → `missing_linked_issue` in blockers → `failure`) and the false/false base arm (neither set → no finding), and it would have failed on the old code.

Signal Result Evidence
Code review ✅ No blockers 1 reviewers, synthesized
Linked issue ✅ Linked #1446
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Review load ✅ 20/20 Readiness component derived from cached public PR metadata and labels; size label size:XS.
Validation evidence ❌ 5/25 Cached preflight status is hold.
Open PR queue ❌ 3/10 34 open PR(s), 17 likely reviewable, 17 unlinked.
Contributor context ✅ Confirmed Gittensor contributor joaovictor91123; Gittensor profile; 23 PR(s), 0 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Nits — 6 non-blocking
  • predicted-gate.ts:163-169: The 6-line comment block is disproportionately verbose for a one-line logic change — the PR description and commit message already tell the full story; condense to one line citing `#merge-readiness-parity` and `applyMergeReadinessGate`.
  • predicted-gate.ts:168: The `!== null && !== 'off'` idiom now appears twice in the same expression (once for `linkedIssue`, once for `mergeReadiness`); extracting a small inline helper `const isGateEnabled = (v: string | null | undefined) => v != null && v !== 'off'` would keep future parity additions DRY and make the condition easier to scan.
  • predicted-gate.test.ts:117-121: The 5-line test comment restates the PR description almost verbatim; a single sentence ('mergeReadiness:block with linkedIssue unset must surface missing_linked_issue blocker') is easier to read in a test listing.
  • predicted-gate.ts (general): `src/rules/advisory.ts` is not in the provided file content, so the fix's critical dependency — that `evaluateGateCheck` promotes `missing_linked_issue` to a hard blocker when `linkedIssueGateMode` is `undefined` but `mergeReadinessGateMode` is `'block'` — is validated only by the passing test, not by direct inspection; confirm `applyMergeReadinessGate` in advisory.ts handles the `undefined` `linkedIssueGateMode` case explicitly.
  • predicted-gate.ts:168: Replace the doubled `!== null && !== 'off'` pattern with `const isGateEnabled = (v: string | null | undefined): boolean => v != null && v !== 'off'` defined just above the `requireLinkedIssue` line, then write `const requireLinkedIssue = isGateEnabled(gate.linkedIssue) || isGateEnabled(gate.mergeReadiness)` — this makes every future gate-parity addition a one-liner and eliminates the risk of the two copies drifting.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Review context
  • Author: joaovictor91123
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 23 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Fix blocker.
  • Expect slower review.
  • Refresh registry data or choose a registered active repo.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

The change correctly closes a real predictor/live-gate parity gap: when `gate.linkedIssue` is unset but `gate.mergeReadiness` is active, the old predictor derived `requireLinkedIssue = false`, never created the `missing_linked_issue` finding in the advisory, and returned a false `success` — while the live gate (via `applyMergeReadinessGate` + `shouldCollectLinkedIssueEvidence`) would collect linked-issue evidence, surface the finding, and one-shot close the PR. The new OR term `|| (gate.mergeReadiness !== null && gate.mergeReadiness !== 'off')` mirrors `shouldCollectLinkedIssueEvidence` exactly. The regression test is well-constructed: it exercises both the new blocker arm (mergeReadiness:block + linkedIssue unset → `missing_linked_issue` in blockers → `failure`) and the false/false base arm (neither set → no finding), and it would have failed on the old code.

Nits (5)

  • predicted-gate.ts:163-169: The 6-line comment block is disproportionately verbose for a one-line logic change — the PR description and commit message already tell the full story; condense to one line citing `#merge-readiness-parity` and `applyMergeReadinessGate`.
  • predicted-gate.ts:168: The `!== null && !== 'off'` idiom now appears twice in the same expression (once for `linkedIssue`, once for `mergeReadiness`); extracting a small inline helper `const isGateEnabled = (v: string | null | undefined) => v != null && v !== 'off'` would keep future parity additions DRY and make the condition easier to scan.
  • predicted-gate.test.ts:117-121: The 5-line test comment restates the PR description almost verbatim; a single sentence ('mergeReadiness:block with linkedIssue unset must surface missing_linked_issue blocker') is easier to read in a test listing.
  • predicted-gate.ts (general): `src/rules/advisory.ts` is not in the provided file content, so the fix's critical dependency — that `evaluateGateCheck` promotes `missing_linked_issue` to a hard blocker when `linkedIssueGateMode` is `undefined` but `mergeReadinessGateMode` is `'block'` — is validated only by the passing test, not by direct inspection; confirm `applyMergeReadinessGate` in advisory.ts handles the `undefined` `linkedIssueGateMode` case explicitly.
  • predicted-gate.ts:168: Replace the doubled `!== null && !== 'off'` pattern with `const isGateEnabled = (v: string | null | undefined): boolean => v != null && v !== 'off'` defined just above the `requireLinkedIssue` line, then write `const requireLinkedIssue = isGateEnabled(gate.linkedIssue) || isGateEnabled(gate.mergeReadiness)` — this makes every future gate-parity addition a one-liner and eliminates the risk of the two copies drifting.

🟩 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.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jun 27, 2026
@dosubot dosubot Bot added the lgtm label Jun 28, 2026
@JSONbored
JSONbored merged commit f7764b6 into JSONbored:main Jun 28, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: gate predictor under-reports the missing-linked-issue blocker on mergeReadiness:block repos

3 participants