feat(mcp): let doctor --exit-code gate CI with a non-zero exit on failure

[glorydavid03023]

Summary

gittensory-mcp doctor already computes a status (ok / warnings / needs_attention), but the process always exits 0 — so it cannot gate a CI step or a pre-commit hook. This adds an opt-in --exit-code flag:

# In CI / a git hook: fail the step if the local MCP setup is broken.
gittensory-mcp doctor --exit-code

When a diagnostic check fails (status: "needs_attention"), the process exits 1. The diagnostic report is still printed — only the exit code changes.

Behavior / compatibility

• Default is unchanged. Without --exit-code, doctor still exits 0, so existing scripts that ignore the exit code keep working.
• Warnings stay exit 0. Only hard failures (needs_attention) produce a non-zero code, so transient/advisory warnings don't break pipelines.

Why no linked issue

Additive, opt-in CLI ergonomics with no change to default behavior. No public-behavior, auth/session, schema, deploy, or frontend-architecture change, so per CONTRIBUTING this does not require an issue first. Happy to file one if maintainers prefer.

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). doctor is the only command that returns a number; every other command resolves to undefined and therefore still exits 0.

Changes

• packages/gittensory-mcp/bin/gittensory-mcp.js — thread an optional exit code through the CLI entry point; doctor returns 1 only under --exit-code on needs_attention; help-text usage line.
• test/unit/mcp-cli.test.ts — cover (1) default exit-0 even when a check fails, (2) non-zero exit under --exit-code on failure, (3) exit 0 under --exit-code when checks pass.
• packages/gittensory-mcp/README.md — document the flag and its semantics.

Contract notes

No MCP tool or HTTP/OpenAPI contract changes. Local CLI behavior only; stdout output shape is unchanged.

Validation

Intended gate (CONTRIBUTING required checks):

npm run build:mcp
npm run test:mcp-pack
npm run typecheck
npm run test:coverage
npm run test:ci

Transparency note: my local authoring environment had no Node runtime available, so I could not execute the gate locally before opening this PR. The change was prepared against the existing CLI dispatch/doctor/test patterns and is intended to be validated by CI on this PR (I will also run the full gate locally). If any check needs adjustment I'll follow up promptly.

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.

Closes #1443

[JSONbored]

@glorydavid03023 the doctor --exit-code behavior is useful, but this branch is mechanically blocked.

A few notes:

• The default-exit compatibility behavior is the right compatibility choice.
• validate is failing in test/unit/mcp-cli.test.ts.
• The branch also conflicts with current MCP CLI tests.

Required changes:

• Fix the TypeScript errors in the new async execFile test.
• Rebase and resolve the test/unit/mcp-cli.test.ts conflict.
• Add the missing Safety section for MCP CLI stdout/stderr/JSON behavior.

Validation expected:

• Rerun npm run typecheck, the focused MCP CLI test file, npm run build:mcp, and npm run test:mcp-pack.

[glorydavid03023]

Rebased onto the latest main (resolved the test-file overlap with the merged version/completion commands) and validated locally on Node v20.18.1 (CI runs Node 22):

• npm run build:mcp → pass (exit 0)
• npm run typecheck → pass (exit 0)
• Focused test/unit/mcp-cli.test.ts → the 3 doctor --exit-code cases pass

While validating, I caught and fixed a real TypeScript error in the exit-code test: the async execFile callback left error/out implicitly any. I rewrote it to use execFileSync + try/catch (capturing .status/.stdout), matching the existing run() helper pattern. npm run typecheck is now clean. (This had not been caught before because the original PR was authored without a local Node runtime.)

The only locally-failing test is the pre-existing rejects unsafe server-provided packet markdown timeout, which is environment-only and passes on CI.

[ghost]

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal	Result	Evidence	Action
Gate result	⚠️ Skipped	#359 is no longer open.	No action.

---

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

[JSONbored]

@glorydavid03023 this is good to land, after fixing merge conflicts.

A few notes:

• The default doctor exit behavior stays compatible.
• --exit-code gives CI/pre-commit users the stricter behavior they need without changing existing scripts.
• The tests cover default exit 0, failure exit 1 under the flag, and passing exit 0 under the flag.
• Fix merge conflicts, then I'll approve once done.

[JSONbored]

Requesting changes for current status only. I did not find a new code-level blocker in the doctor --exit-code patch itself, but the PR is still unmergeable against current main and prior changes-requested/conflict state remains in force. Please rebase/resolve conflicts and rerun CI before rereview. No merge or close action taken.

[JSONbored]

@glorydavid03023 I do not see a new source-level objection to doctor --exit-code.

A few notes:

• The compatibility behavior is right: default doctor still exits 0, and strict non-zero behavior is opt-in.
• The current blocker is mechanical: the branch still conflicts in test/unit/mcp-cli.test.ts.
• The stale requested-changes state is only relevant because the conflict is still real.

Required changes:

• Rebase on current main and resolve test/unit/mcp-cli.test.ts.
• Keep the existing default-exit and --exit-code test coverage.

Validation expected:

• Rerun npm run typecheck, focused test/unit/mcp-cli.test.ts, npm run build:mcp, and npm run test:mcp-pack.

[JSONbored]

@glorydavid03023 the merge conflict is gone, but the rebase resolution dropped unrelated existing MCP CLI config coverage from test/unit/mcp-cli.test.ts.

A few notes:

• The doctor --exit-code behavior itself is reasonable.
• The current diff removes tests for config provenance, environment overrides, unsupported source upload reporting, profile config, and config-path override behavior. Those are current-main tests and should not disappear as part of this feature.
• This needs a proper rebase that preserves existing coverage and adds only the new doctor exit-code cases.

Required changes:

- Restore the unrelated mcp-cli config tests removed from test/unit/mcp-cli.test.ts.
- Keep the new doctor --exit-code tests alongside the existing config/profile/source-upload tests.
- Rebase cleanly onto current main without deleting unrelated MCP CLI coverage.

Validation expected:

- focused test/unit/mcp-cli.test.ts
- build:mcp / package validation if this repo requires it for MCP CLI changes
- full validate

[superagent-security]

P2: Security test coverage removed for config secret-leakage protections

PR deletes tests that verified config output never leaks secret tokens or local paths.

Restore the secret-leakage config tests or add equivalent coverage elsewhere.

AI prompt

Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="test/unit/mcp-cli.test.ts">
<violation number="1" location="test/unit/mcp-cli.test.ts:1183">
<priority>P2</priority>
<title>Security test coverage removed for config secret-leakage protections</title>
<evidence>The PR removes five config-related tests, including `attributes config values to environment overrides without leaking secrets`, which verified that `gittensory-mcp config` does not print token values (`expect(out).not.toContain('super-secret-token')`) or local absolute paths (`expect(out).not.toContain(secretDir)`). No equivalent replacement tests were added.</evidence>
<recommendation>Restore the deleted security tests or move them to a dedicated config test file. If the deletion was intentional during rebase, add equivalent tests that verify the config command never prints token values, local absolute paths, or other sensitive data.</recommendation>
</violation>
</file>

[JSONbored]

@glorydavid03023 the doctor --exit-code behavior itself is reasonable, but this branch still should be restarted from a clean rebase.

A few notes:

• The current diff still removes unrelated existing coverage from test/unit/mcp-cli.test.ts.
• The removed coverage includes config provenance, environment overrides, unsupported source-upload reporting, profile config, and config-path override behavior.
• That was the prior requested-change theme, and it is still present on the current head.
• Superagent is also action-required, so this is not just stale review state.

A replacement PR should:

• Rebase cleanly without dropping unrelated current-main MCP CLI tests.
• Add only the doctor --exit-code behavior, help text, README note, and focused tests for default exit 0, failing diagnostic exit 1, and passing diagnostic exit 0.
• Include a concise no-issue rationale or link a focused issue.
• Pass validate and Superagent.