Skip to content

fix(mcp): map review preflight status#3972

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-review-pr-preflight-status-mapping
Jul 7, 2026
Merged

fix(mcp): map review preflight status#3972
JSONbored merged 1 commit into
mainfrom
codex/fix-review-pr-preflight-status-mapping

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The review-pr composition layer was incorrectly mapping preflight statuses using fail/warn, which does not match the actual preflight contract of ready | needs_work | hold, causing blocking/warning states to be reported as passing.
  • This change prevents needs_work or hold preflight results from producing an overall pass when other checks (e.g. slop-risk, pr-text-lint) pass.

Description

  • Replace the inline ternary mapping in packages/gittensory-mcp/bin/gittensory-mcp.js with a dedicated preflightSectionStatus helper that maps holdfail, needs_workwarn, and otherwise pass and wire it into the sections array used by reviewLocalPr.
  • Add regression tests in test/unit/mcp-cli-review-pr.test.ts that assert needs_work produces a warn section and hold produces a fail section so the composed overallStatus is correct.
  • Keep other section mappings (slop_risk, pr_text_lint) unchanged and preserve the per-check degradation behavior when remote endpoints fail.

Testing

  • Ran npx vitest run test/unit/mcp-cli-review-pr.test.ts and all tests in that file passed (10 tests) indicating the new mappings and regressions are covered and green.
  • Ran npm run build:mcp and npm run test:mcp-pack to validate the MCP bundle and packaging, both commands succeeded.
  • Ran git diff --check to ensure no whitespace/conflict markers and it succeeded.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 7, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 846dc79 Commit Preview URL

Branch Preview URL
Jul 07 2026, 08:15 AM

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-07 08:18:05 UTC

2 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This fixes a real bug: the old ternary compared `result.analysis.preflight?.status` against the literal strings "fail"/"warn", but the preflight contract actually returns `ready`/`needs_work`/`hold`, so the comparison could never match and preflight always reported "pass" regardless of its true status. The new `preflightSectionStatus` helper correctly maps `hold`→fail, `needs_work`→warn, and defaults to pass for `ready`, and is wired into the `sections` array exactly as described. The two new regression tests exercise the fixture server with `needs_work` and `hold` preflight statuses and assert both the section status and the resulting `overallStatus`, directly covering the previously-broken path.

Nits — 5 non-blocking
  • packages/gittensory-mcp/bin/gittensory-mcp.js: `preflightSectionStatus` could use a lookup object (e.g. `{hold:"fail", needs_work:"warn"}[status] ?? "pass"`) to mirror the style of `slopRiskSectionStatus`/`prTextLintSectionStatus`, though the if-chain is equally clear.
  • test/unit/mcp-cli-review-pr.test.ts: much of the diff is pure reformatting (wrapping existing `runAsync` calls across multiple lines) unrelated to the fix, which inflates the diff size without functional change.
  • No test asserts the `ready` preflight status still maps to "pass" alongside the other checks passing, which would fully round out coverage of the helper's three branches.
  • Consider adding a small `ready` → `pass` case test alongside the new `needs_work`/`hold` tests for symmetry, even though it's implicitly covered by the earlier passing-scenario test.
  • Revert the incidental Prettier-style reformatting in unrelated test blocks to keep the diff focused on the preflight fix.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 343 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 343 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 343 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.68%. Comparing base (b8a1186) to head (846dc79).
⚠️ Report is 18 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3972   +/-   ##
=======================================
  Coverage   93.68%   93.68%           
=======================================
  Files         373      373           
  Lines       34895    34895           
  Branches    12768    12768           
=======================================
  Hits        32692    32692           
  Misses       1584     1584           
  Partials      619      619           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 0b6bc4b into main Jul 7, 2026
12 checks passed
@JSONbored
JSONbored deleted the codex/fix-review-pr-preflight-status-mapping branch July 7, 2026 08:25
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.

Development

Successfully merging this pull request may close these issues.

1 participant