Skip to content

[mcp] fix: classify startup unavailable errors - #128

Merged
Wangmerlyn merged 1 commit into
mainfrom
codex/controller-startup-unavailable-errors
Jun 28, 2026
Merged

[mcp] fix: classify startup unavailable errors#128
Wangmerlyn merged 1 commit into
mainfrom
codex/controller-startup-unavailable-errors

Conversation

@Wangmerlyn

@Wangmerlyn Wangmerlyn commented Jun 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • classify expected GlobalGPUController startup unavailability with controller-domain exceptions
  • map startup-unavailable starts to direct JSON-RPC -32000, REST 503, and MCP tool isError=true
  • preserve arbitrary startup/runtime failures as internal errors and add regression coverage for unsupported platform and zero visible GPU paths
  • update AGENTS.md, MCP/CLI docs, and the implementation plan

Verification

  • PYTHONPATH=$PWD/src pytest tests/mcp/test_server.py::test_jsonrpc_start_keep_startup_unavailable_returns_public_code tests/mcp/test_server.py::test_jsonrpc_start_keep_unsupported_platform_returns_public_code tests/mcp/test_server.py::test_jsonrpc_start_keep_zero_visible_gpus_returns_public_code tests/mcp/test_server.py::test_jsonrpc_start_keep_runtime_value_error_remains_internal_error tests/mcp/test_server.py::test_mcp_tools_call_startup_unavailable_returns_tool_error tests/mcp/test_http_api.py::test_http_post_sessions_startup_unavailable_returns_json_503 tests/mcp/test_http_api.py::test_http_post_sessions_runtime_value_error_returns_json_500 tests/mcp/test_http_api.py::test_http_post_sessions_runtime_type_error_returns_json_500 -q -> 8 passed
  • PYTHONPATH=$PWD/src pytest tests/mcp/test_server.py tests/mcp/test_http_api.py tests/global_controller -q -> 203 passed, 1 skipped
  • PYTHONPATH=$PWD/src pytest tests -q -> 493 passed, 11 skipped
  • PYTHONPATH=$PWD/src mkdocs build -> passed with existing Material/nav warnings
  • pre-commit run --all-files -> passed
  • git diff --check origin/main...HEAD -> passed

Local review

  • Spec reviewer: compliant, no missing or extra requirements
  • Quality reviewer: no critical/important issues; suggested optional zero-visible-GPU coverage
  • Follow-up reviewer: confirmed zero-visible-GPU coverage resolves the note and found no new issues

Summary by CodeRabbit

  • New Features

    • Added clearer startup error responses when the service can’t begin due to unsupported platforms or no available GPUs.
    • Session creation now returns consistent errors across JSON-RPC, REST, and MCP interfaces.
  • Bug Fixes

    • Startup failures are now reported as availability issues instead of generic internal errors.
    • REST session requests now return 503 for startup-unavailable cases.
  • Documentation

    • Updated guides and references to explain the new error behavior and response codes.
  • Tests

    • Expanded coverage for unsupported-platform, no-GPU, and MCP error handling scenarios.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a346e50-835f-42c5-badf-5cc2e2a08dfe

📥 Commits

Reviewing files that changed from the base of the PR and between 2aca5c3 and 3f35f2b.

📒 Files selected for processing (8)
  • AGENTS.md
  • docs/guides/mcp.md
  • docs/plans/controller-startup-unavailable-errors.md
  • docs/reference/cli.md
  • src/keep_gpu/global_gpu_controller/global_gpu_controller.py
  • src/keep_gpu/mcp/server.py
  • tests/mcp/test_http_api.py
  • tests/mcp/test_server.py

📝 Walkthrough

Walkthrough

Adds a ControllerStartupUnavailable exception hierarchy to GlobalGPUController, replacing bare ValueError/NotImplementedError raises. The server layer catches these as SessionStartupUnavailable and maps them to JSON-RPC code -32000, HTTP 503, and MCP tools/call result.isError=true. Tests and documentation are updated accordingly.

Changes

Controller Startup Unavailable Error Handling

Layer / File(s) Summary
Controller exception hierarchy
src/keep_gpu/global_gpu_controller/global_gpu_controller.py
Defines ControllerStartupUnavailable, NoGPUAvailableError, and UnsupportedControllerPlatformError; updates GlobalGPUController.__init__ raise sites to use the new types.
Server-layer mapping and dispatch
src/keep_gpu/mcp/server.py
Imports ControllerStartupUnavailable; adds JSONRPC_STARTUP_UNAVAILABLE = -32000 and SessionStartupUnavailable; converts the exception in start_keep; maps it to -32000 in _call_keepgpu_method and to HTTP 503 in do_POST.
Tests
tests/mcp/test_server.py, tests/mcp/test_http_api.py
Adds JSON-RPC tests for direct SessionStartupUnavailable, unsupported platform, and zero GPUs; adds MCP tools/call isError test; adds HTTP 503 test for POST /api/sessions.
Documentation
AGENTS.md, docs/guides/mcp.md, docs/reference/cli.md, docs/plans/controller-startup-unavailable-errors.md
Updates error-code mappings across all interface docs and adds the implementation plan document.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Wangmerlyn/KeepGPU#59: Introduced the MCP server's start_keep and JSON-RPC request-handling machinery that this PR extends with startup-unavailable error mapping.
  • Wangmerlyn/KeepGPU#86: Modified _call_keepgpu_method and tools/call dispatch in server.py, the same paths this PR adds the -32000 branch to.
  • Wangmerlyn/KeepGPU#96: Added structured JSON error responses to POST /api/sessions, the endpoint this PR now extends with 503 for startup-unavailable conditions.

Poem

🐇 Hoppity-hop through GPU land,
When no cards are found, I raise a clear hand!
ControllerStartupUnavailable — neat and precise,
Five-oh-three says "not ready," minus-thirty-two's nice.
No more bare ValueError lost in the code,
Every error now follows a well-labeled road! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: classifying startup-unavailable errors in MCP handling.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/controller-startup-unavailable-errors

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
Contributor

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 explicit, parseable error handling for expected controller startup unavailability, such as unsupported platforms or no visible GPUs. It defines custom exceptions (including ControllerStartupUnavailable, NoGPUAvailableError, UnsupportedControllerPlatformError, and SessionStartupUnavailable) and maps them to specific error responses: JSON-RPC -32000, REST HTTP 503, and MCP tool isError=true. Comprehensive unit tests and documentation updates have been added to verify and document these changes. There are no review comments, and we have no additional feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@Wangmerlyn
Wangmerlyn merged commit 4f503ca into main Jun 28, 2026
5 checks passed
@Wangmerlyn
Wangmerlyn deleted the codex/controller-startup-unavailable-errors branch June 28, 2026 18:12
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.

1 participant