Skip to content

#25: handle MCP startup failure#33

Open
akmhatey-ai wants to merge 3 commits into
zerocracy:masterfrom
akmhatey-ai:codex/handle-server-connect-failure-25
Open

#25: handle MCP startup failure#33
akmhatey-ai wants to merge 3 commits into
zerocracy:masterfrom
akmhatey-ai:codex/handle-server-connect-failure-25

Conversation

@akmhatey-ai

Copy link
Copy Markdown

Closes #25.

This adds a catch handler around the MCP server startup promise so a server.connect(...) rejection is reported with context and exits with status 1 instead of surfacing as an unhandled promise rejection.

Changes:

  • report startup failure with Failed to start MCP server:
  • exit with status 1 after reporting the failure
  • add a regression test that mocks server.connect rejection and asserts console.error plus process.exit(1)

Validation:

  • npm exec jest -- --config jest.config.ts --no-color --ci test/index.test.ts --coverage=false -> 1 passed
  • npm exec eslint -- index.ts test/index.test.ts --config eslint.config.mjs -> passed
  • npm exec tsc -- --noEmit -> passed
  • git diff --cached --check -> passed before commit
  • git diff --cached | gitleaks stdin --no-banner --redact -> no leaks before commit

Limitations:

  • Full npm exec jest -- --config jest.config.ts --no-color --ci --coverage=false is not clean locally because existing live Zerocracy API tests fail with dummy-token HTTP error 303/400 responses in test/baza.test.ts and test/server.test.ts. The new startup test and mocked local tests pass.

@GHX5T-SOL GHX5T-SOL 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.

Verified the startup failure handling for #25. The top-level async startup now reports server.connect(...) rejection with context and exits with status 1 instead of leaving an unhandled rejection.

Validation:

  • npm ci -> completed; npm reports existing audit findings, and this PR does not change dependencies
  • npm exec jest -- --config jest.config.ts --no-color --ci test/index.test.ts --coverage=false -> 1 suite passed, 1 test passed
  • npm exec eslint -- index.ts test/index.test.ts --config eslint.config.mjs -> passed
  • npm exec tsc -- --noEmit -> passed
  • git diff --check origin/master...HEAD -> passed
  • git diff origin/master...HEAD | gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found

Live pre-review state: open, non-draft, mergeable/UNSTABLE only because no GitHub checks are reported, no existing comments/reviews. Same-scope open PR search for server connect failure startup returned no competing PR. I did not run the full live endpoint suite.

@kreinba kreinba 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.

The catch handler matches the fix proposed in issue #25: rejections from server.connect now log with context and exit 1 instead of bubbling up as unhandled rejections. The regression test mocks the rejection, asserts both console.error and process.exit(1) are called, and confirms no unhandled rejection escapes. Targeted jest run passes locally.

@0crat

0crat commented Jun 23, 2026

Copy link
Copy Markdown

@kreinba Thanks for the review! You've earned +10 points for this: +18 as a basis; -8 for absolutely no comments posted. Your running score is +1446; don't forget to check your Zerocracy account too).

@akmhatey-ai

Copy link
Copy Markdown
Author

Merged current master into this branch and pushed head 8817484; GitHub now reports the PR as CLEAN and MERGEABLE.

Notes:

  • Kept the startup failure change scoped to index.ts and test/index.test.ts.
  • Updated the entrypoint imports and regression test to match the current ESM/Nodenext setup from master.

Validation:

  • npm ci -> completed. npm reports existing audit/deprecation findings; this PR does not change dependencies.
  • node --experimental-vm-modules .\node_modules\jest\bin\jest.js --config jest.config.ts --no-color --ci test/index.test.ts --coverage=false -> 1 suite, 1 test passed.
  • node --experimental-vm-modules .\node_modules\jest\bin\jest.js --config jest.config.ts --no-color --ci --coverage=false -> 7 suites, 22 tests passed.
  • npm exec -- eslint index.ts test/index.test.ts --config eslint.config.mjs -> passed.
  • npm exec -- eslint . --config eslint.config.mjs -> passed.
  • Makefile-equivalent TypeScript compile for non-test sources -> passed.
  • git diff --check origin/master...HEAD -> clean.
  • git merge-tree --write-tree origin/master HEAD -> b4af11434c4f1aceaaa95b3c1408bc61be60c96e.
  • Diff-scoped gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found.

Hosted checks on 8817484 pass: actionlint, checkmake, copyrights, eslint, make, markdown-lint, pdd, reuse, typos, xcop, and yamllint.

Limitation: this Windows shell does not have GNU make; the hosted Linux make job passed on the pushed head.

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.

Server crashes due to unhandled promise rejection in top-level IIFE on server.connect failure

4 participants