feat(mcp): let doctor --exit-code gate CI with a non-zero exit on failure#1444
Conversation
…lure `gittensory-mcp doctor` computes a status (ok / warnings / needs_attention) but always exits 0, so it cannot gate a CI step or pre-commit hook. Add an opt-in `--exit-code` flag: when a diagnostic check fails (status "needs_attention"), the process exits 1. The diagnostic report is still printed; only the exit code changes. The default is unchanged — without `--exit-code`, doctor still exits 0 — so existing scripts keep working. Warnings also stay at exit 0; only hard failures are surfaced as a non-zero code. runCli's command result now carries an optional numeric exit code (doctor is the only command that returns one); every other command still resolves to exit 0. Tests (test/unit/mcp-cli-doctor.test.ts) cover the default exit-0-on-failure behavior, the non-zero exit under --exit-code on failure, and exit 0 under --exit-code when checks pass. Closes JSONbored#1443 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 #1444 +/- ##
=======================================
Coverage 95.51% 95.51%
=======================================
Files 204 204
Lines 22205 22205
Branches 8028 8028
=======================================
Hits 21210 21210
Misses 415 415
Partials 580 580 🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review — held for maintainer review
⏸️ Held for maintainer review — This is a first-time contribution to this repo, so the gate stays advisory rather than blocking. The findings remain visible, and the gate will apply normally once this author has merge history here. Review summary
Nits — 6 non-blocking
Review context
Contributor next steps
Signal definitions
Review detailsGenerated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative. The CLI change correctly threads an optional numeric result from runCli into process.exit and has doctor return 1 only for --exit-code plus needs_attention. The doctor status is computed from the existing check list, so warnings keep the default zero exit and no other command gets a nonzero path from this diff. The notable weakness is test isolation around ambient auth environment variables, not the implementation itself. Nits (5)
🟩 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.
|
Root cause traced as far as possible: release-please's own createReleases() (the step that actually tags a merged Release PR) sometimes finds nothing to tag, and createPullRequests() then aborts every subsequent run without ever completing the tag -- confirmed live across two separate linked-versions release cycles this session (JSONbored#7127, JSONbored#7133), both requiring a manual dispatch of each publish-*.yml workflow plus manually flipping the merged PR's autorelease:pending label. Matches multiple long-open, unresolved upstream issues (googleapis/release-please#1946/JSONbored#1444/JSONbored#1406), so it isn't something fixable from this repo's config alone. Adds a reconcile-stale-releases job that runs after every release-please attempt, independent of its outputs: compares each package's committed package.json version against what's actually live on npm, dispatches that package's publish workflow whenever they disagree (the same documented human-override path, automated), and flips any merged PR's autorelease label once every package is confirmed live -- unsticking release-please's own tracking for the next run. Also cleaned up one ancient, silently-stuck instance of the exact same bug (engine v1.0.0, autorelease:pending since long before this session).
Summary
Replacement for #359 (closed). Rebased cleanly onto current
main— it adds only thedoctor --exit-codebehavior and does not drop any of the existing MCP CLI coverage (config provenance, completion, version, profile, etc. are all untouched).gittensory-mcp doctorcomputes astatus(ok/warnings/needs_attention) but always exits 0, so it cannot gate a CI step or pre-commit hook:gittensory-mcp doctor --exit-code # exits 1 if the local MCP setup is brokenWhen a check fails (
status: "needs_attention"), the process exits1. The diagnostic report is still printed — only the exit code changes.Closes #1443.
Behavior / compatibility
--exit-code,doctorstill exits 0, so existing scripts keep working.needs_attention) produce a non-zero code.Implementation
runCli's command result now carries an optional numeric exit code, and the entry point uses it:process.exit(typeof exitCode === "number" ? exitCode : 0).doctoris the only command that returns a number; every other command resolves toundefinedand still exits 0.Changes (only-adds, no removed coverage)
packages/gittensory-mcp/bin/gittensory-mcp.js— thread an optional exit code through the entry point;doctorreturns1only under--exit-codeonneeds_attention; help-text flag.test/unit/mcp-cli-doctor.test.ts— adds three focused tests (default exit 0 on failure; non-zero under--exit-codeon failure; exit 0 under--exit-codewhen passing). Uses the sharedmcp-cli-harness.packages/gittensory-mcp/README.md— document the flag.Validation
Run locally on Node v22.22.0 (matches the project's engine):
npm run build:mcp→ passtest/unit/mcp-cli-doctor.test.ts→ my three--exit-codecases pass:keeps doctor exit code 0 by default even when a check fails✓exits non-zero from doctor --exit-code when a check fails✓keeps doctor --exit-code at 0 when checks pass✓gittensory-mcp doctor --exit-codehelp/smoke verified.Honest caveats from my local box (both unrelated to this change):
tsc --noEmitcould not complete locally because my localnode_modulesis missing some newly-added deps (pg,ioredis,pngjs,@cloudflare/puppeteer,pixelmatch) used bysrc/selfhost/*,src/visual-agent/*, etc. There are zero type errors in the files this PR touches. CI runs a clean install and gives the authoritative result.runs doctor against a local health/session fixture, a local-scorer fixture assertion, flaky/retry x1) fails locally; it is untouched by this change (which only adds areturnat the end ofdoctor).Note re #359: it was rebased onto a
mainsnapshot that predated theconfigmerge, so its diff appeared to delete config tests. This branch is cut from currentmain, so the diff is strictly additive.Security / privacy
No auth, cookie, CORS, GitHub App output, identity, or contributor-evidence changes. Only an exit-code signal is added; no new data is printed or transmitted.