mirror of
https://github.com/NomaDamas/k-skill.git
synced 2026-06-24 02:04:11 +00:00
Keep MyRealTrip MCP calls bounded and covered
Add a timeout guard around the remote Streamable HTTP MCP call and focused wrapper tests so the new skill fails predictably under upstream stalls without touching unrelated PR scope. Constraint: Task scope allowed fixes only inside PR #229 files; package.json test wiring was left unchanged because it is outside this PR's original file set. Rejected: Editing root package scripts to auto-run the new unittest | outside assigned PR file scope without leader approval. Confidence: high Scope-risk: narrow Directive: Keep MyRealTrip live calls bounded; do not remove the timeout without replacing it with an equivalent cancellation mechanism. Tested: python3 -m py_compile myrealtrip-search/scripts/myrealtrip_mcp.py myrealtrip-search/scripts/test_myrealtrip_mcp.py; PYTHONPATH=myrealtrip-search/scripts python3 -m unittest myrealtrip-search/scripts/test_myrealtrip_mcp.py; node --test scripts/skill-docs.test.js; ./scripts/validate-skills.sh; npm run typecheck; git diff --check; npm test Not-tested: Live MyRealTrip MCP tools smoke because local environment lacks the optional mcp Python package and live network dependency is upstream-owned.
This commit is contained in:
Jeffrey (Dongkyu) Kim
parent
568b9256be
commit
78e505bfc0
2 changed files with 145 additions and 2 deletions
|
|
@ -17,6 +17,7 @@ import sys
|
|||
from typing import Any, Sequence
|
||||
|
||||
DEFAULT_ENDPOINT = "https://mcp-servers.myrealtrip.com/mcp"
|
||||
DEFAULT_TIMEOUT_SECONDS = 30.0
|
||||
|
||||
|
||||
class MyRealTripMcpError(RuntimeError):
|
||||
|
|
@ -33,6 +34,16 @@ def parse_json_object(raw: str, *, arg_name: str) -> dict[str, Any]:
|
|||
return value
|
||||
|
||||
|
||||
def parse_positive_float(raw: str) -> float:
|
||||
try:
|
||||
value = float(raw)
|
||||
except ValueError as exc:
|
||||
raise argparse.ArgumentTypeError("timeout은 숫자여야 합니다") from exc
|
||||
if value <= 0:
|
||||
raise argparse.ArgumentTypeError("timeout은 0보다 커야 합니다")
|
||||
return value
|
||||
|
||||
|
||||
def parse_kv_pairs(pairs: Sequence[str]) -> dict[str, Any]:
|
||||
args: dict[str, Any] = {}
|
||||
for pair in pairs:
|
||||
|
|
@ -66,6 +77,12 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
|
|||
default=os.getenv("MYREALTRIP_MCP_ENDPOINT", DEFAULT_ENDPOINT),
|
||||
help="마이리얼트립 MCP 엔드포인트(기본값: %(default)s).",
|
||||
)
|
||||
parser.add_argument(
|
||||
"--timeout-seconds",
|
||||
type=parse_positive_float,
|
||||
default=DEFAULT_TIMEOUT_SECONDS,
|
||||
help="MCP 연결/호출 전체 제한 시간(기본값: %(default)s초).",
|
||||
)
|
||||
subparsers = parser.add_subparsers(dest="command", required=True)
|
||||
|
||||
subparsers.add_parser("tools", help="사용 가능한 MCP 도구와 입력 스키마를 JSON으로 출력합니다.")
|
||||
|
|
@ -90,7 +107,7 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
|
|||
return parser.parse_args(argv)
|
||||
|
||||
|
||||
async def run_mcp(endpoint: str, command: str, tool: str | None = None, arguments: dict[str, Any] | None = None) -> Any:
|
||||
async def _run_mcp_once(endpoint: str, command: str, tool: str | None, arguments: dict[str, Any] | None) -> Any:
|
||||
try:
|
||||
from mcp import ClientSession
|
||||
from mcp.client.streamable_http import streamablehttp_client
|
||||
|
|
@ -121,6 +138,25 @@ async def run_mcp(endpoint: str, command: str, tool: str | None = None, argument
|
|||
raise MyRealTripMcpError(f"지원하지 않는 명령입니다: {command}")
|
||||
|
||||
|
||||
async def run_mcp(
|
||||
endpoint: str,
|
||||
command: str,
|
||||
tool: str | None = None,
|
||||
arguments: dict[str, Any] | None = None,
|
||||
*,
|
||||
timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS,
|
||||
) -> Any:
|
||||
try:
|
||||
return await asyncio.wait_for(
|
||||
_run_mcp_once(endpoint, command, tool, arguments),
|
||||
timeout=timeout_seconds,
|
||||
)
|
||||
except TimeoutError as exc:
|
||||
raise MyRealTripMcpError(
|
||||
f"마이리얼트립 MCP 엔드포인트 호출 시간이 {timeout_seconds:g}초를 초과했습니다: {endpoint}"
|
||||
) from exc
|
||||
|
||||
|
||||
def jsonable(value: Any) -> Any:
|
||||
if hasattr(value, "model_dump"):
|
||||
return value.model_dump(mode="json")
|
||||
|
|
@ -137,7 +173,15 @@ def main(argv: Sequence[str] | None = None) -> int:
|
|||
tool_args.update(parse_kv_pairs(args.kv_args))
|
||||
|
||||
try:
|
||||
result = asyncio.run(run_mcp(args.endpoint, args.command, getattr(args, "tool", None), tool_args))
|
||||
result = asyncio.run(
|
||||
run_mcp(
|
||||
args.endpoint,
|
||||
args.command,
|
||||
getattr(args, "tool", None),
|
||||
tool_args,
|
||||
timeout_seconds=args.timeout_seconds,
|
||||
)
|
||||
)
|
||||
except MyRealTripMcpError as exc:
|
||||
print(f"myrealtrip_mcp.py: {exc}", file=sys.stderr)
|
||||
return 2
|
||||
|
|
|
|||
99
myrealtrip-search/scripts/test_myrealtrip_mcp.py
Normal file
99
myrealtrip-search/scripts/test_myrealtrip_mcp.py
Normal file
|
|
@ -0,0 +1,99 @@
|
|||
import argparse
|
||||
import asyncio
|
||||
import importlib.util
|
||||
import pathlib
|
||||
import unittest
|
||||
from unittest import mock
|
||||
|
||||
MODULE_PATH = pathlib.Path(__file__).with_name("myrealtrip_mcp.py")
|
||||
spec = importlib.util.spec_from_file_location("myrealtrip_mcp", MODULE_PATH)
|
||||
myrealtrip_mcp = importlib.util.module_from_spec(spec)
|
||||
assert spec.loader is not None
|
||||
spec.loader.exec_module(myrealtrip_mcp)
|
||||
|
||||
|
||||
class ParseHelpersTest(unittest.TestCase):
|
||||
def test_parse_json_object_requires_object(self):
|
||||
self.assertEqual(myrealtrip_mcp.parse_json_object('{"gid": 123}', arg_name="--json"), {"gid": 123})
|
||||
|
||||
with self.assertRaises(argparse.ArgumentTypeError):
|
||||
myrealtrip_mcp.parse_json_object('["not", "object"]', arg_name="--json")
|
||||
|
||||
def test_parse_kv_pairs_json_decodes_values_when_possible(self):
|
||||
self.assertEqual(
|
||||
myrealtrip_mcp.parse_kv_pairs(["query=오사카", "perPage=5", "directFlightOnly=true"]),
|
||||
{"query": "오사카", "perPage": 5, "directFlightOnly": True},
|
||||
)
|
||||
|
||||
def test_parse_kv_pairs_rejects_malformed_pairs(self):
|
||||
with self.assertRaises(argparse.ArgumentTypeError):
|
||||
myrealtrip_mcp.parse_kv_pairs(["missing_separator"])
|
||||
|
||||
with self.assertRaises(argparse.ArgumentTypeError):
|
||||
myrealtrip_mcp.parse_kv_pairs(["=empty_key"])
|
||||
|
||||
def test_timeout_must_be_positive_number(self):
|
||||
self.assertEqual(myrealtrip_mcp.parse_positive_float("1.5"), 1.5)
|
||||
|
||||
for raw in ["0", "-1", "not-a-number"]:
|
||||
with self.subTest(raw=raw):
|
||||
with self.assertRaises(argparse.ArgumentTypeError):
|
||||
myrealtrip_mcp.parse_positive_float(raw)
|
||||
|
||||
|
||||
class CliAssemblyTest(unittest.TestCase):
|
||||
def test_json_and_arg_inputs_are_merged_before_call(self):
|
||||
captured = {}
|
||||
|
||||
async def fake_run_mcp(endpoint, command, tool=None, arguments=None, *, timeout_seconds):
|
||||
captured.update(
|
||||
endpoint=endpoint,
|
||||
command=command,
|
||||
tool=tool,
|
||||
arguments=arguments,
|
||||
timeout_seconds=timeout_seconds,
|
||||
)
|
||||
return {"ok": True}
|
||||
|
||||
with mock.patch.object(myrealtrip_mcp, "run_mcp", side_effect=fake_run_mcp):
|
||||
exit_code = myrealtrip_mcp.main(
|
||||
[
|
||||
"--endpoint",
|
||||
"https://example.invalid/mcp",
|
||||
"--timeout-seconds",
|
||||
"2.5",
|
||||
"call",
|
||||
"searchTnas",
|
||||
"--json",
|
||||
'{"query":"오사카","perPage":3}',
|
||||
"--arg",
|
||||
"perPage=5",
|
||||
]
|
||||
)
|
||||
|
||||
self.assertEqual(exit_code, 0)
|
||||
self.assertEqual(captured["endpoint"], "https://example.invalid/mcp")
|
||||
self.assertEqual(captured["command"], "call")
|
||||
self.assertEqual(captured["tool"], "searchTnas")
|
||||
self.assertEqual(captured["arguments"], {"query": "오사카", "perPage": 5})
|
||||
self.assertEqual(captured["timeout_seconds"], 2.5)
|
||||
|
||||
|
||||
class TimeoutTest(unittest.TestCase):
|
||||
def test_run_mcp_reports_timeout(self):
|
||||
async def never_finishes(*args, **kwargs):
|
||||
await asyncio.sleep(60)
|
||||
|
||||
with mock.patch.object(myrealtrip_mcp, "_run_mcp_once", side_effect=never_finishes):
|
||||
with self.assertRaisesRegex(myrealtrip_mcp.MyRealTripMcpError, "초과"):
|
||||
asyncio.run(
|
||||
myrealtrip_mcp.run_mcp(
|
||||
"https://example.invalid/mcp",
|
||||
"tools",
|
||||
timeout_seconds=0.001,
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Loading…
Add table
Add a link
Reference in a new issue