diff --git a/plugins/axiomcode/mcp/_fallback.py b/plugins/axiomcode/mcp/_fallback.py index 8eb322681..a00b95b61 100644 --- a/plugins/axiomcode/mcp/_fallback.py +++ b/plugins/axiomcode/mcp/_fallback.py @@ -18,7 +18,14 @@ """ import inspect, json, sys, typing -__all__ = ["MCPServer"] +__all__ = ["MCPServer", "ToolError"] + + +class ToolError(Exception): + """A failure the tool anticipated and put into words: the words are the result's text and the result is marked + isError, as the SDK's own ToolError is. Any other exception is marked the same way, but with its type in front, + because its words were never meant for the caller.""" + _JSON_TYPE = {str: "string", int: "integer", float: "number", bool: "boolean", list: "array", dict: "object"} @@ -132,6 +139,8 @@ def _handle(self, msg): try: out = t["fn"](**args) return {"content": [{"type": "text", "text": "" if out is None else str(out)}]} + except ToolError as e: + return {"content": [{"type": "text", "text": str(e)}], "isError": True} except Exception as e: return {"content": [{"type": "text", "text": f"{type(e).__name__}: {e}"}], "isError": True} if m == "ping": diff --git a/plugins/axiomcode/mcp/server.py b/plugins/axiomcode/mcp/server.py index b1ebc5e62..cf1c3bda4 100755 --- a/plugins/axiomcode/mcp/server.py +++ b/plugins/axiomcode/mcp/server.py @@ -126,11 +126,17 @@ def unknown_arguments(name, arguments): said = [f"{k}: unexpected argument" for k in extra] return f"invalid arguments for {name}: " + '; '.join(said) + f". {name} takes: {', '.join(params) or 'no arguments'}" -try: - import importlib - ToolError = importlib.import_module(MCPServer.__module__.rsplit('.', 1)[0] + '.exceptions').ToolError -except Exception: - ToolError = ValueError +# A FAILURE IS RAISED, NOT RETURNED. A client marks a result isError only when the tool raised, and only the server's +# own ToolError keeps the words: the SDK (mcp ≥ 2.3) replaces any other exception's message with a bare "Error +# executing tool", and the fallback puts the type in front. So run() raises this one class, from whichever half serves. +if MCPServer.__module__ == '_fallback': + from _fallback import ToolError +else: + try: + import importlib + ToolError = importlib.import_module(MCPServer.__module__.rsplit('.', 1)[0] + '.exceptions').ToolError + except Exception: + ToolError = ValueError # an SDK with no exceptions module: the result is still isError, wordless class Server(MCPServer): def tool(self, *a, **k): @@ -152,6 +158,9 @@ async def call_tool(self, name, arguments, *a, **k): # the SDK: refused as a # launch.js hands over the bash it chose, because on Windows a bare `bash` is WSL's or nothing (#1233). BASH = os.environ.get('AXIOMCODE_BASH') or 'bash' +# how long a call may take before it is given up as a failure (seconds; a first build is minutes, so the default is +# generous); AXIOMCODE_MCP_TIMEOUT exists for a test that must see the timeout in seconds, not in minutes +TIMEOUT = float(os.environ.get('AXIOMCODE_MCP_TIMEOUT') or 900) # THE TIMER (#1305). The hooks refresh the graph while an agent works; an edit made in an editor or a terminal while the # session sits idle is caught only at the next prompt. The server lives as long as the session, so once @@ -190,12 +199,36 @@ def _timer(interval): if time.time() - fresh.last_update(repo) >= interval: fresh.hook_kick(repo, 'the timer') except Exception: pass -def run(args, cwd=None, timeout=900): +def repository(args, cwd=None): + """the repository a call asks about: the last positional argument that is a directory (a flag's value, `--in `, + is not one), else the working directory""" + pos = [a for i, a in enumerate(args[1:], 1) if args[i - 1] not in VALUED] + return next((a for a in reversed(pos) if os.path.isdir(a)), cwd or os.getcwd()) + +# WHAT A FAILURE IS. The CLI's exits: 0 is an answer, however empty; 1 is `path` saying in words that no chain connects +# the two; 2 is a refusal with its correction (nothing named X and the nearest names, no graph, no such directory), +# and ALSO the note that the graph is still being built. A refusal is a failure of the call, as an argument the tool +# does not take already is (#1567): the client marks the result isError and the model still reads the words. The +# building note is not: it is told apart by the build lock, which a build holds for as long as it runs. A crash, a +# kill, an exit no verb uses, or silence is a failure whatever the code says. +def failed(r, repo): + """whether a finished call is a failure the client must see as one, not an answer""" + if r.returncode == 0: return False + if r.returncode < 0 or r.returncode not in (1, 2): return True + if 'Traceback (most recent call last)' in (r.stderr or ''): return True + if r.returncode == 1: return not ((r.stdout or '').strip() or (r.stderr or '').strip()) + try: return not scripts_module('ax_fresh').building(os.path.realpath(repo)) + except Exception: return True + +def run(args, cwd=None, timeout=None): + """the CLI's words for a call; a call that failed (failed, a bash that will not start, a timeout with no build + running) raises ToolError with the words, so the client marks the result isError and the model still reads them""" + if timeout is None: timeout = TIMEOUT for a in args[1:]: if os.path.isdir(a) and os.path.isdir(os.path.join(a, '.axiomcode')): SEEN.add(os.path.realpath(a)) # A QUERY ON A REPOSITORY WITH NO GRAPH YET does not hold the call for the length of a first build (minutes; past this - # server's timeout it came back as a bare "Error executing tool"): the build is started in the background and the - # answer says what stage it is at (ax_contract.ensure_graph). `index` is asked for a build and still waits for it. + # server's timeout it is a failure saying only that): the build is started in the background and the answer says + # what stage it is at (ax_contract.ensure_graph). `index` is asked for a build and still waits for it. env = dict(os.environ, AXIOMCODE_BUILD_NOWAIT='1', AXIOMCODE_SURFACE='mcp') if args and args[0] != 'index' else None root = plugin_root() ax = os.path.join(root, 'skills', 'axiomcode', 'scripts', 'axiomcode') @@ -204,6 +237,7 @@ def run(args, cwd=None, timeout=900): if o: return f"axiomcode: {o}." note = stale_note(root) head = (note + '\n') if note else '' + repo = repository(args, cwd) try: # On Windows the arguments travel in AXIOMCODE_ARGV, not on the command line: Git Bash parses a command line by # msys rules (`'` quotes, `\"` is no escape), so a question holding an apostrophe lost every argument after it. @@ -213,23 +247,22 @@ def run(args, cwd=None, timeout=900): argv, env = [BASH, ax], dict(env or os.environ, AXIOMCODE_ARGV=''.join(f'{a}\x1f' for a in args)) r = subprocess.run(argv, cwd=cwd or None, capture_output=True, text=True, timeout=timeout, env=env) except OSError as e: - return (f"axiomcode could not start bash ({BASH}): {e}. On Windows it needs the bash that comes with " - "Git for Windows; install it, or set AXIOMCODE_BASH to its bin\\bash.exe.") + raise ToolError(f"axiomcode could not start bash ({BASH}): {e}. On Windows it needs the bash that comes with " + "Git for Windows; install it, or set AXIOMCODE_BASH to its bin\\bash.exe.") from None except subprocess.TimeoutExpired: - # a flag's value (`--in `) is not the repository - pos = [a for i, a in enumerate(args[1:], 1) if args[i - 1] not in VALUED] - repo = next((a for a in reversed(pos) if os.path.isdir(a)), cwd or os.getcwd()) try: ax_fresh = scripts_module('ax_fresh'); ax_contract = scripts_module('ax_contract') if ax_fresh.building(os.path.realpath(repo)): return head + ax_contract.building_note(os.path.realpath(repo)) except Exception: pass - return head + (f"axiomcode {args[0] if args else ''} did not answer within {timeout} s. Nothing was changed; " - f"see {os.path.join(repo, '.axiomcode', 'build.log')} if a build was running, and ask again.") + raise ToolError(head + f"axiomcode {args[0] if args else ''} did not answer within {timeout:g} s. Nothing was changed; " + f"see {os.path.join(repo, '.axiomcode', 'build.log')} if a build was running, and ask again.") from None out = (r.stdout or '') + (('\n' + r.stderr.strip()) if r.returncode and r.stderr.strip() else '') # an answer given from a graph that predates some edit says so, and names the files (#1305); one given from a graph a # fallback engine built, in place of the checkout's own, names that engine if not r.returncode: out += ''.join('\n' + l for l in (r.stderr or '').splitlines() if l.startswith(('graph refresh:', 'graph built by:'))) - return head + (out.strip() or f"(no output, exit {r.returncode})") + text = head + (out.strip() or f"(no output, exit {r.returncode})") + if failed(r, repo): raise ToolError(text + (f"\n(exit {r.returncode})" if out.strip() else '')) + return text # THE TOOLS TAKE NO OPTIONS, so an answer never tells the agent to pass one. The notes the verbs add (a stale # graph, a refresh in flight) are kept for what they say; a clause that names a flag or a parameter to set is dropped. @@ -249,6 +282,13 @@ def plain(text): if keep and ''.join(keep).strip(): out.append(' '.join(keep).rstrip(' ;,')) return '\n'.join(out) +def answer(args): + """a tool's answer: the CLI's words with the option-naming clauses dropped. A failure run() raises goes the same + way — its words through plain(), then on as the ToolError the client marks isError — so the model reads the + correction (the nearest names, the fix) either way, and a flag it cannot pass is named in neither.""" + try: return plain(run(args)) + except ToolError as e: raise ToolError(plain(str(e))) from None + # THE SMALL SURFACE. Search is grep's job; the graph answers what grep cannot. Three questions, each answered as # numbered places with the code of the function each sits in, so a place is understood without opening its file, @@ -259,20 +299,20 @@ def context(task: str, source: bool = False) -> str: """How something works, from a task in words: the files and callables the task touches, and for a "how does X work" question the call FLOW — every step in the order the calls are written, with ⚠ where the graph lost a call. source=True asks for the flow with each step's code, so it is read without opening files.""" - return plain(run(['context', task] + (['--source'] if source else []) + [os.getcwd()])) + return answer(['context', task] + (['--source'] if source else []) + [os.getcwd()]) @srv.tool() def impact(name: str = '') -> str: """What a change reaches. With a name (as written in the code: Owner.method, function, Type, or file.py:123): who calls it, what depends on it further out, and which tests exercise it, each with its code. With no name: the same for the declarations your uncommitted edits changed.""" - return plain(run(['impact'] + ([name] if name.strip() else []) + [os.getcwd()])) + return answer(['impact'] + ([name] if name.strip() else []) + [os.getcwd()]) @srv.tool() def path(start: str, end: str) -> str: """How one declaration reaches another: every hop of the call chain with the code at the line the call is written on. start / end as written in the code (Owner.method, function, Type).""" - return plain(run(['path', start, end, os.getcwd()])) + return answer(['path', start, end, os.getcwd()]) @srv.tool() def link(site: str = '', target: str = '') -> str: @@ -282,12 +322,12 @@ def link(site: str = '', target: str = '') -> str: whether the graph took it (a link whose line changed is dropped, never trusted). target "-" removes the site's links; target "not:" rejects a lead (a by-name or one-of-a-set guess) at that site, which is then not walked. Never link a guess, and never link a candidate for its rank alone.""" - return plain(run(['link'] + ([site] if site.strip() else []) + ([target] if site.strip() and target.strip() else []) + [os.getcwd()])) + return answer(['link'] + ([site] if site.strip() else []) + ([target] if site.strip() and target.strip() else []) + [os.getcwd()]) @srv.tool() def tests() -> str: """The tests your uncommitted edits reach, each with its code, and the command that runs exactly those.""" - return plain(run(['tests', os.getcwd()])) + return answer(['tests', os.getcwd()]) if __name__ == '__main__': # catch up on whatever changed while no session was running (#1305): started, never waited on diff --git a/tests/corrupt_graph.py b/tests/corrupt_graph.py index b999e00d1..a37cd2e04 100644 --- a/tests/corrupt_graph.py +++ b/tests/corrupt_graph.py @@ -121,13 +121,16 @@ def check(ok, why, detail=''): open(os.path.realpath(ptr), 'wb').write(os.urandom(8192)) sys.path.insert(0, os.path.join(ROOT, 'plugins', 'axiomcode', 'mcp')); os.environ.update(AXIOMCODE_ENGINE=ROOT, AXIOMCODE_NO_REFRESH='1') import server - text = server.run(['impact', 'helper', repo]) + def words(args): # a refusal comes as the server's ToolError, words intact + try: return server.run(args) + except server.ToolError as e: return str(e) + text = words(['impact', 'helper', repo]) check('Traceback' not in text and 'corrupt' in text, 'mcp: the tool answer says the graph is corrupt, no traceback', text) sys.path.insert(0, SCRIPTS); import ax_fresh for _ in range(600): # the rebuild it started if os.path.exists(ptr) and not ax_fresh.building(repo): break time.sleep(1) - text = server.run(['impact', 'helper', repo]) + text = words(['impact', 'helper', repo]) check('caller' in text and 'corrupt' not in text, 'mcp: once rebuilt the tool answers again', text) # ── bad page ──────────────────────────────────────────────────────────────────────────────────────── diff --git a/tests/latency.py b/tests/latency.py index ad4064985..a908df6ef 100644 --- a/tests/latency.py +++ b/tests/latency.py @@ -313,9 +313,12 @@ def timeout_checks(): with contextlib.redirect_stderr(io.StringIO()): spec.loader.exec_module(m) work = tempfile.mkdtemp(prefix='axiomcode-timeout-') try: - try: out = m.run(['context', 'anything', work], timeout=0.01); raised = '' + # the words come as the server's own ToolError, which the client marks isError; a bare exception would lose them, + # and a plain return would pass the failure off as an answer + try: out = m.run(['context', 'anything', work], timeout=0.01); raised = 'returned as an answer' + except m.ToolError as e: out, raised = str(e), '' except Exception as e: out, raised = '', repr(e) - check("timeout: an MCP call past the server's timeout answers in words, not with an exception", + check("timeout: an MCP call past the server's timeout fails in words that say so, as a tool error", not raised and 'did not answer within' in out, raised or out) finally: shutil.rmtree(work, ignore_errors=True) diff --git a/tests/mcp.py b/tests/mcp.py index f85b37d37..1b4311dd6 100644 --- a/tests/mcp.py +++ b/tests/mcp.py @@ -190,11 +190,96 @@ def text(self, name, arguments): return ''.join(c.get('text', '') for c in self.ask('tools/call', {'name': name, 'arguments': arguments}).get('content', [])) def close(self): + """ends the server; returns what it wrote on stderr, which says which half served (the fallback's banner)""" self.timer.cancel() self.p.stdin.close() self.p.wait() + err = self.p.stderr.read() self.p.stdout.close() self.p.stderr.close() + return err + + +FALLBACK_BANNER = 'serving with the built-in fallback' +# holds a repository's build lock the way a build does (ax_fresh._flock: LOCK_EX on .axiomcode/build.lock), from a +# process of its own, and says so once it has it +HOLD = ("import fcntl, os, sys, time; fd = os.open(sys.argv[1], os.O_RDWR | os.O_CREAT, 0o644); " + "fcntl.flock(fd, fcntl.LOCK_EX); print('held', flush=True); time.sleep(120)") + + +def check_failures(label, cmd, cwd, env=None, workdir=None, sdk=None): + """A call the CLI could not carry out is a tool error (isError), never an answer: a bash that is not there, a verb + that crashed or was killed, an exit no verb uses, silence, a timeout with no build running, and a refusal (nothing + named X, no graph: exit 2, as an argument the tool lacks already is, #1567). The controls are what must NOT be an + error: an answer, an empty one, `path` saying "no chain" (exit 1, in words), and the note that a graph is still + being built (exit 2 while the build lock is held). The CLI is a fake bash that says what each case needs, so what is + tested is the server's own reading of it; the words must survive, and the SDK puts a prefix on them, so substrings. + `sdk` True or False asserts which half served the mode, read off the stderr banner.""" + if os.name == 'nt': + return [] + bad = [] + here = workdir or cwd # the server's cwd, which is the repository every tool asks about + fakes = tempfile.mkdtemp(prefix='ax-bash-') + base = {k: v for k, v in (env or os.environ).items() if k != 'AXIOMCODE_BASH'} + base.update(AXIOMCODE_MCP_TIMEOUT='2', AXIOMCODE_REFRESH_INTERVAL='0') + + def fake(name, body): + p = os.path.join(fakes, name) + with open(p, 'w') as f: + f.write('#!/bin/sh\n' + body + '\n') + os.chmod(p, 0o755) + return p + crash = r"""printf 'Traceback (most recent call last):\n File "x.py", line 1, in \nValueError: bad\n' >&2; exit 1""" + note = 'the graph for $PWD is being built (starting; 0m01s so far). Nothing else is needed: ask again in a minute or two' + cases = [('missing', os.path.join(fakes, 'no-bash'), True, ['could not start']), + ('boom', fake('boom', "printf 'boom\\n' >&2; exit 7"), True, ['boom', 'exit 7']), + ('slow', fake('slow', 'exec sleep 30'), True, ['did not answer within']), # exec: the kill reaches the sleeper + ('crash', fake('crash', crash), True, ['Traceback']), + ('no chain', fake('nochain', 'echo "no chain of resolved calls connects a and b in either direction"; exit 1'), False, ['no chain']), + ('empty ok', fake('empty', 'exit 0'), False, ['(no output, exit 0)']), + ('building', fake('building', f'echo "{note}"; exit 2'), False, ['being built']), + ('refusal', fake('refusal', 'echo "nothing named zz is declared in this graph. Exact names that are close: a, b"; exit 2'), True, ['close:', 'exit 2']), + ('silent 1', fake('silent', 'exit 1'), True, ['(no output, exit 1)'])] + served = None + try: + for name, bash, want, needles in cases: + s = Session(cmd, here, dict(base, AXIOMCODE_BASH=bash)) + holder = None + try: + if name == 'building': # after the start, so the server's own start-up sees no .axiomcode + os.makedirs(os.path.join(here, '.axiomcode'), exist_ok=True) + holder = subprocess.Popen([sys.executable, '-c', HOLD, os.path.join(here, '.axiomcode', 'build.lock')], + stdout=subprocess.PIPE, text=True) + holder.stdout.readline() + res = s.ask('tools/call', {'name': 'path', 'arguments': {'start': 'a', 'end': 'b'}}) + finally: + if holder: + holder.kill(); holder.wait(); holder.stdout.close() + shutil.rmtree(os.path.join(here, '.axiomcode'), ignore_errors=True) + err = s.close() + if served is None: + served = 'the fallback' if FALLBACK_BANNER in err else 'the SDK' + text = ' '.join(c.get('text', '') for c in res.get('content', [])) + if bool(res.get('isError')) != want: + bad.append(f"{label}, {name}: isError {res.get('isError')!r}, want {want}: {text[:300]!r}") + for n in needles: + if n not in text: + bad.append(f"{label}, {name}: the words lack {n!r}: {text[:300]!r}") + finally: + shutil.rmtree(fakes, ignore_errors=True) + print(f"{label}: failures checked, served by {served}") + if sdk is not None and served != ('the SDK' if sdk else 'the fallback'): + bad.append(f"{label}: served by {served}, but this mode was meant to test {'the SDK' if sdk else 'the fallback'}") + return bad + + +def sdk_python(): + """an interpreter with the MCP SDK, so the failure checks run on that half too: AXIOMCODE_SDK_PYTHON, else this + one; None when neither imports it""" + for py in filter(None, (os.environ.get('AXIOMCODE_SDK_PYTHON'), sys.executable)): + if subprocess.run([py, '-c', 'import mcp.server.mcpserver'], capture_output=True).returncode == 0: + return py + return None def check_install_move(work): @@ -309,6 +394,7 @@ def main(): bad += check('bin/axiomcode mcp', ['bash', CLI, 'mcp'], repo) # the SDK when the launcher finds one, which ignored an argument it did not know (#1567); else the fallback again bad += check_arguments('bin/axiomcode mcp', ['bash', CLI, 'mcp'], repo, lax=True) + bad += check_failures('bin/axiomcode mcp', ['bash', CLI, 'mcp'], repo) bad += check_front_door() if os.name != 'nt': bad += check_install_move(work) @@ -316,6 +402,15 @@ def main(): env_note = 'python3 -S server.py (fallback, no SDK)' bad += check(env_note, [sys.executable, '-S', SERVER], repo) bad += check_arguments(env_note, [sys.executable, '-S', SERVER], repo) + bad += check_failures(env_note, [sys.executable, '-S', SERVER], repo, sdk=False) + # the same failures through the SDK, when an interpreter has it (a venv with `pip install mcp`, named by + # AXIOMCODE_SDK_PYTHON); the skip is said out loud, so a run without the SDK is not mistaken for one with it + py = sdk_python() + if py: + bad += check_failures('server.py (SDK)', [py, SERVER], repo, sdk=True) + else: + print('SDK path skipped: mcp not importable (python3 -m venv && /bin/pip install mcp && ' + 'AXIOMCODE_SDK_PYTHON=/bin/python python3 tests/mcp.py)') # The plugin's own server entries, run as their hosts run them, from a copy of the plugin under a path # with a space, which splits an unquoted path into two words. @@ -329,6 +424,8 @@ def main(): server = json.load(f)['mcpServers']['axiomcode'] bad += check('.codex-plugin/mcp.json', [server['command'], *server['args']], repo, base, os.path.join(plugin, server.get('cwd', '.'))) + bad += check_failures('.codex-plugin/mcp.json', [server['command'], *server['args']], repo, base, + os.path.join(plugin, server.get('cwd', '.'))) # Cursor's own manifest names the server with ${CURSOR_PLUGIN_ROOT}, which Cursor replaces with the # plugin directory before it starts the command, and starts it in the user's project. @@ -338,6 +435,7 @@ def main(): cmd = [server['command'], *map(expand, server['args'])] env = dict(base, **{k: expand(v) for k, v in server.get('env', {}).items()}) bad += check('.cursor-plugin/plugin.json', cmd, repo, env, repo) + bad += check_failures('.cursor-plugin/plugin.json', cmd, repo, env, repo) with open(os.path.join(plugin, '.mcp.json')) as f: server = json.load(f)['mcpServers']['axiomcode'] @@ -345,6 +443,10 @@ def main(): cmd = [server['command'], *map(expand, server['args'])] env = dict(base, **{k: expand(v) for k, v in server.get('env', {}).items()}) bad += check('.mcp.json', cmd, repo, env, repo) + bad += check_failures('.mcp.json', cmd, repo, env, repo) + if py: # Claude Code's entry with the SDK's interpreter first in the launcher's search + sdk_env = dict(env, AXIOMCODE_PYTHON=py, PATH=os.path.dirname(py) + os.pathsep + env.get('PATH', '')) + bad += check_failures('.mcp.json (SDK python)', cmd, repo, sdk_env, repo, sdk=True) # A uv on PATH that cannot build its environment is passed over for the fallback, rather than started # and left to exit before `initialize` (#1249). python3 and python are shadowed by interpreters without