Skip to content

feat: implement issue #903 — Add MCP/Context7 connectivity assertion to daily-pr-review-health - #904

Merged
don-petry merged 4 commits into
mainfrom
dev-lead/issue-903-20260622-0121
Jun 23, 2026
Merged

don-petry merged 4 commits into
mainfrom
dev-lead/issue-903-20260622-0121

Conversation

@don-petry

Copy link
Copy Markdown
Collaborator

Closes #903

Implemented by dev-lead agent. Please review.

@don-petry
don-petry requested a review from a team as a code owner June 22, 2026 01:28
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@don-petry, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 40 minutes and 12 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 231848f9-7518-4fd5-8288-cba23cce8c35

📥 Commits

Reviewing files that changed from the base of the PR and between e4b34af and f7053fc.

📒 Files selected for processing (4)
  • .github/workflows/daily-pr-review-health.yml
  • .github/workflows/lint.yml
  • scripts/mcp_connectivity_check.sh
  • tests/test_mcp_connectivity_check.bats
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead/issue-903-20260622-0121

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces scripts/mcp_connectivity_check.sh and its corresponding BATS tests to provide a durable, affirmative MCP connectivity assertion monitor. The review feedback identifies a critical issue where the script could prematurely exit under set -euo pipefail if grep finds no matching handshake lines, and suggests appending || true to the pipeline. Additionally, the reviewer recommends adding an explicit dependency check for jq to prevent silent failures, and points out a redundant assertion in the test suite.

Comment thread scripts/mcp_connectivity_check.sh Outdated
Comment thread scripts/mcp_connectivity_check.sh
Comment thread tests/test_mcp_connectivity_check.bats Outdated
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) June 22, 2026 01:37
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-22T02:38:59Z.

@don-petry
don-petry disabled auto-merge June 22, 2026 01:39
@don-petry
don-petry enabled auto-merge (squash) June 22, 2026 01:45
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 22, 2026
@don-petry
don-petry disabled auto-merge June 22, 2026 01:46
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (no-changes)

Agent reasoning
::warning::[claude] model claude-sonnet-4-6 throttled (rc=1) — trying next in chain
The background `find` for bats completed with no results — confirming bats isn't installed locally, consistent with relying on the green CI `bats` check. No further action needed; the thread is replied and resolved, and the work is complete.

@don-petry
don-petry enabled auto-merge (squash) June 22, 2026 01:52
@don-petry
don-petry disabled auto-merge June 22, 2026 03:00
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 22, 2026 03:02
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-22T04:03:56Z.

@don-petry
don-petry disabled auto-merge June 22, 2026 03:17
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 22, 2026 03:18
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-22T04:21:14Z.

@donpetry-bot donpetry-bot added the needs-human-review Flagged by automated PR review agent label Jun 22, 2026
@don-petry
don-petry disabled auto-merge June 22, 2026 05:36
@don-petry

Copy link
Copy Markdown
Collaborator Author

Auto-rebase failed — merge conflict — this branch has conflicts with main that must be resolved.

dev-lead will attempt to resolve this automatically. If it cannot, a follow-up comment will explain what needs manual attention.

To resolve manually instead:

git fetch origin
git merge origin/main
# resolve conflicts, then:
git add .
git commit
git push

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 22, 2026 05:40
@donpetry-bot

Copy link
Copy Markdown
Contributor

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-22T06:42:09Z.

@donpetry-bot

donpetry-bot commented Jun 22, 2026 •

Copy link
Copy Markdown
Contributor
Superseded by automated re-review at f7053fca944db12a5f09734cc058a7a130ebc635 — click to expand prior review.

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: LOW
Reviewed commit: a700cdab5b913b9ccd19758fa16dbc3beac5b515
Review mode: triage-approved (single reviewer)

Summary

Additive MCP/Context7 connectivity monitor for the daily PR-review health check (#903, part of #676). Adds scripts/mcp_connectivity_check.sh (pure, unit-tested helpers + an I/O main() that probes the configured server via the same engine.sh flags), a 248-line bats suite, a hashFiles-guarded step in daily-pr-review-health.yml, and the new bats file in lint.yml. Code is clean, well-scoped, and a no-op for non-MCP repos.

Linked issue analysis

Closes #903 — implements the durable, affirmative MCP-connectivity assertion the issue calls for (fails the health check when MCP is configured but unreachable; no-ops otherwise). Substantively addressed.

Findings

  • All three gemini-code-assist review threads are RESOLVED and the fixes are present in the head commit:
    • HIGH: success-signal grep pipeline under `set -euo pipefail` now appends `|| true` (and classify uses grep only inside `if`).
    • MEDIUM: an explicit `command -v jq` guard now errors instead of silently skipping the assertion.
    • LOW: redundant test assertion noted (cosmetic; non-blocking).
  • coderabbitai APPROVED at the reviewed head SHA; SonarCloud Quality Gate passed (0 new issues).
  • Blocker: the PR is `CONFLICTING` / `DIRTY` — a live merge conflict with `main`. Auto-rebase failed (comment 2026-06-22T05:40) and dev-lead returned no-changes. The conflict must be resolved before this can merge.

CI status

All required checks green: Lint, ShellCheck, bats, unit-tests, validate-agent-profiles, gh-aw-compile, CodeQL (actions+python), SonarCloud, Secret scan (gitleaks), AgentShield, Holdout Guard, Test-Deletion Guard. Dependency-audit ecosystem jobs and dependabot-automerge SKIPPED as expected. Merge state is DIRTY (conflict with main).


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@don-petry
don-petry disabled auto-merge June 23, 2026 16:48
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — rebase (applied)

Rebase completed and pushed.

@don-petry
don-petry enabled auto-merge (squash) June 23, 2026 16:56
@don-petry
don-petry disabled auto-merge June 23, 2026 16:56
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 23, 2026 17:00
@don-petry
don-petry disabled auto-merge June 23, 2026 18:24
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 23, 2026 18:28
@don-petry
don-petry disabled auto-merge June 23, 2026 18:29
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) June 23, 2026 18:32
@don-petry
don-petry disabled auto-merge June 23, 2026 18:33
@sonarqubecloud

Copy link
Copy Markdown

@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 23, 2026 18:36

@donpetry-bot donpetry-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: f7053fca944db12a5f09734cc058a7a130ebc635
Review mode: triage-approved (single reviewer)

Summary

Refactors scripts/mcp_connectivity_check.sh into pure, unit-tested helpers (classify_mcp_output / mcp_check_is_failure / derive_allowed_tools / mcp_check_report) plus an I/O main(), and wires the daily health-check step behind a hashFiles('.github/review-mcp.json') guard. Net +354/-326 across 4 files; CI-only monitoring tooling.

Linked issue analysis

Closes #903 (part of #676). The PR delivers the durable, affirmative MCP/Context7 connectivity assertion the issue calls for: it drives the configured server(s) through the same flags engine.sh uses (--mcp-config --strict-mcp-config --debug mcp) and fails the daily health check when MCP is configured but unreachable. .github/review-mcp.json is present in the repo, so the new hashFiles guard fires and the monitor actually runs rather than no-opping.

Findings

No blocking findings.

  • Advisory bot (gemini-code-assist) had flagged a set -euo pipefail premature-exit on the handshake grep, a missing jq dependency guard, and a redundant test assertion. The current head incorporates the fixes: '|| true' terminates the handshake-emit pipeline, an explicit jq guard returns 1 with ::error::, and a 'main: jq missing' test covers it.
  • Deliberate, internally-consistent behavior changes vs. the prior version: (a) an explicitly-set-but-missing REVIEW_MCP_CONFIG now no-ops (exit 0) instead of fail-loud (exit 1) — moot in practice behind the workflow hashFiles guard; (b) classify_mcp_output returns CONNECTED when any handshake is present even if a later line shows a failure (handshake-wins). For the single-server Context7 use case this is fine; worth a note only if multi-server configs are later added.
  • Security: no secrets/credentials; cfg/allowed/model passed positionally to claude/jq (no shell injection); workflow change is an if-guard tightening plus adding the bats file to the lint list — no GitHub Actions security smells.

CI status

All required checks green: shellcheck/ShellCheck, bats, unit-tests, Lint, CodeQL (actions+python), Secret scan (gitleaks), SonarCloud, AgentShield, validate-agent-profiles, holdout-guard, guard. dependency-audit ecosystem jobs and dependabot-automerge correctly SKIPPED. CodeRabbit check state SUCCESS.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

@don-petry
don-petry merged commit 2888396 into main Jun 23, 2026
29 checks passed
@don-petry
don-petry deleted the dev-lead/issue-903-20260622-0121 branch June 23, 2026 19:24
@donpetry-bot donpetry-bot removed the needs-human-review Flagged by automated PR review agent label Jun 23, 2026
don-petry added a commit that referenced this pull request Jun 23, 2026
…to daily-pr-review-health (#904)

* feat: implement issue #903 — Add MCP/Context7 connectivity assertion to daily-pr-review-health

* chore: apply manual instructions [skip ci-relay]

* fix(bot): address bot feedback [skip ci-relay]

---------

Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add MCP/Context7 connectivity assertion to daily-pr-review-health

2 participants