Skip to content

Commit cf0298b

Browse files
committed
docs(acp): document the async factory, drop the default model, add Zed setup
Review follow-ups on #143. `MCPManager.create_stdio_server` existed with a one-line docstring that said what it does and not why it has to exist: `create_from_server` is sync, and called with a loop already running it offloads to a thread and blocks on the result, stalling the caller's loop for the whole connect. A host serving over that same loop — the ACP server on stdin/stdout — goes unresponsive while an MCP server starts. `_create_tool_instance` was extracted without a docstring, and the extraction dropped the comment explaining why refresh_ctx is stashed. Both documented. `--model` no longer defaults to a specific NVIDIA model. Baking a model choice into the package is not the package's call; it is now required via the flag or NOOA_MODEL, and the READMEs set it explicitly. Added a Zed setup section, including the one thing that will otherwise waste someone an afternoon: remote MCP servers authenticated inside Zed are invisible to ACP agents (zed-industries/zed#54410). Signed-off-by: Paul Furgale <pfurgale@nvidia.com>
1 parent 7a5949a commit cf0298b

7 files changed

Lines changed: 175 additions & 6 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ its working directory:
203203

204204
```bash
205205
uv add nooa-acp
206+
export NOOA_MODEL=nvidia_nim/nvidia/nemotron-3-super-120b-a12b
206207
export NVIDIA_API_KEY=nvapi-...
207208

208209
# Agent command for the ACP client

packages/nooa-acp/README.md

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,67 @@ Set the model and its provider credentials, then configure the client to launch
1313
this command with the repository as its working directory:
1414

1515
```bash
16+
export NOOA_MODEL=nvidia_nim/nvidia/nemotron-3-super-120b-a12b
1617
export NVIDIA_API_KEY=nvapi-...
1718
uvx nooa-acp
1819
```
1920

21+
There is no default model. Pass `--model` or set `NOOA_MODEL`; the command
22+
exits with a usage error if neither is set.
23+
2024
From this repository, use the workspace package as the client command:
2125

2226
```bash
2327
uv run --project "$PWD" --package nooa-acp -- nooa-acp
2428
```
2529

30+
## Configuring Zed
31+
32+
Zed launches ACP agents as "external agents". Add NOOA to `settings.json`:
33+
34+
```json
35+
{
36+
"agent_servers": {
37+
"NOOA": {
38+
"type": "custom",
39+
"command": "uvx",
40+
"args": ["nooa-acp"],
41+
"env": {
42+
"NOOA_MODEL": "nvidia_nim/nvidia/nemotron-3-super-120b-a12b",
43+
"NVIDIA_API_KEY": "nvapi-..."
44+
}
45+
}
46+
}
47+
}
48+
```
49+
50+
Pick NOOA from the `+` menu in the agent panel. Zed runs the command with the
51+
worktree as its working directory, so repository instructions and project
52+
sessions resolve against the open project.
53+
54+
Credentials have to go in `env` here rather than Zed's own settings: the agent
55+
is a separate process and inherits only what Zed passes it. Use a secret
56+
manager wrapper as the `command` if you would rather not put a key in
57+
`settings.json`.
58+
59+
### MCP servers do not carry over from Zed
60+
61+
**Remote MCP servers you authenticated inside Zed are not usable from an ACP
62+
agent.** Zed holds those OAuth tokens itself and does not pass them down, so a
63+
server showing a green indicator in Zed's own UI arrives at the agent either
64+
with no tools at all or with nothing but its `authenticate` /
65+
`__complete_authentication` stubs. Local stdio MCP servers are unaffected.
66+
67+
This is a known Zed limitation, tracked in
68+
[zed-industries/zed#54410](https://github.com/zed-industries/zed/issues/54410)
69+
(open, labelled `area:ai/mcp` + `area:ai/acp`). A maintainer has said the
70+
plumbing largely exists and the work is queued, but as of this writing it is
71+
unresolved.
72+
73+
Configure the MCP server directly for NOOA instead — through NOOA's own
74+
`.mcp.json` — and it works normally, because the agent then owns the
75+
connection and its credentials rather than borrowing Zed's.
76+
2677
ACP uses standard input and output for JSON-RPC. Diagnostics are written to
2778
standard error. The agent can execute generated Python and shell commands, so
2879
use an OS-level sandbox for untrusted tasks. Generated code shares the agent's

packages/nooa-acp/src/nooa_acp/cli.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@
1616
@click.option(
1717
"--model",
1818
envvar="NOOA_MODEL",
19-
default="nvidia_nim/nvidia/nemotron-3-super-120b-a12b",
20-
show_default=True,
21-
help="LiteLLM model name or configured NOOA model alias.",
19+
required=True,
20+
help="LiteLLM model name or configured NOOA model alias. Or set NOOA_MODEL.",
2221
)
2322
@click.option(
2423
"--client-type",
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
# SPDX-License-Identifier: Apache-2.0
3+
"""Model selection for the ACP entry point."""
4+
5+
import click.testing
6+
import pytest
7+
from nooa_acp.cli import command
8+
9+
10+
@pytest.fixture
11+
def stubbed_serve(monkeypatch):
12+
"""Capture the llm_factory the command builds instead of serving."""
13+
captured = {}
14+
15+
def fake_serve(llm_factory):
16+
captured["llm_factory"] = llm_factory
17+
return "coroutine-placeholder"
18+
19+
monkeypatch.setattr("nooa_acp.server.serve", fake_serve)
20+
monkeypatch.setattr("nooa_acp.cli.asyncio.run", lambda coro: coro)
21+
monkeypatch.setattr("nooa.secrets.load_secrets_into_env", lambda *a, **k: None)
22+
return captured
23+
24+
25+
def test_model_is_required(monkeypatch):
26+
monkeypatch.delenv("NOOA_MODEL", raising=False)
27+
28+
result = click.testing.CliRunner().invoke(command, [])
29+
30+
# No default model: the caller has to choose one.
31+
assert result.exit_code == 2
32+
assert "--model" in result.output
33+
34+
35+
def test_model_is_read_from_the_environment(monkeypatch, stubbed_serve):
36+
monkeypatch.setenv("NOOA_MODEL", "openai/gpt-4o-mini")
37+
requested = {}
38+
monkeypatch.setattr(
39+
"nooa.unifiedllm.get_llm_client",
40+
lambda name, **kwargs: requested.setdefault("name", name),
41+
)
42+
43+
result = click.testing.CliRunner().invoke(command, [])
44+
45+
assert result.exit_code == 0, result.output
46+
stubbed_serve["llm_factory"]()
47+
assert requested["name"] == "openai/gpt-4o-mini"
48+
49+
50+
def test_explicit_flag_overrides_the_environment(monkeypatch, stubbed_serve):
51+
monkeypatch.setenv("NOOA_MODEL", "openai/gpt-4o-mini")
52+
requested = {}
53+
monkeypatch.setattr(
54+
"nooa.unifiedllm.get_llm_client",
55+
lambda name, **kwargs: requested.setdefault("name", name),
56+
)
57+
58+
result = click.testing.CliRunner().invoke(command, ["--model", "anthropic/claude-sonnet-4-5"])
59+
60+
assert result.exit_code == 0, result.output
61+
stubbed_serve["llm_factory"]()
62+
assert requested["name"] == "anthropic/claude-sonnet-4-5"

packages/nooa-acp/tests/test_server.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,19 @@ def test_acp_command_is_discovered_as_cli_plugin():
4646
assert dict(discover_commands())["acp"] is command
4747

4848

49-
def test_acp_command_defaults_to_public_nvidia_model():
49+
def test_acp_command_passes_the_nvidia_key_for_nvidia_models():
5050
runner = CliRunner()
5151
with (
5252
patch("nooa.secrets.load_secrets_into_env"),
5353
patch("nooa.unifiedllm.get_llm_client") as get_llm_client,
5454
patch("nooa_acp.server.serve") as serve,
5555
patch("nooa_acp.cli.asyncio.run"),
5656
):
57-
result = runner.invoke(command, env={"NVIDIA_API_KEY": "nvapi-test"})
57+
result = runner.invoke(
58+
command,
59+
["--model", "nvidia_nim/nvidia/nemotron-3-super-120b-a12b"],
60+
env={"NVIDIA_API_KEY": "nvapi-test"},
61+
)
5862

5963
assert result.exit_code == 0
6064
llm_factory = serve.call_args.args[0]
@@ -66,6 +70,25 @@ def test_acp_command_defaults_to_public_nvidia_model():
6670
)
6771

6872

73+
def test_acp_command_leaves_the_key_alone_for_other_providers():
74+
runner = CliRunner()
75+
with (
76+
patch("nooa.secrets.load_secrets_into_env"),
77+
patch("nooa.unifiedllm.get_llm_client") as get_llm_client,
78+
patch("nooa_acp.server.serve") as serve,
79+
patch("nooa_acp.cli.asyncio.run"),
80+
):
81+
result = runner.invoke(
82+
command,
83+
["--model", "openai/gpt-4o-mini"],
84+
env={"NVIDIA_API_KEY": "nvapi-test"},
85+
)
86+
87+
assert result.exit_code == 0
88+
serve.call_args.args[0]()
89+
get_llm_client.assert_called_once_with("openai/gpt-4o-mini", client_type=None)
90+
91+
6992
class _MCPTools:
7093
async def lookup(self, query: str) -> str:
7194
"""Look up a value in the test MCP server."""

packages/nooa-cli/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ run the NOOA coding agent from an ACP-compatible client:
2727

2828
```bash
2929
uv add nooa-acp
30+
export NOOA_MODEL=nvidia_nim/nvidia/nemotron-3-super-120b-a12b
3031
export NVIDIA_API_KEY=nvapi-...
3132
uv run nooa-acp
3233
```

src/nooa/mcp/tool.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,15 @@ def _create_tool_instance(
698698
tools_result: Any,
699699
refresh_ctx: dict[str, Any] | None = None,
700700
) -> MCPTool:
701+
"""Build the dynamic per-server tool class from a completed ``list_tools``.
702+
703+
Shared by the sync and async factories: from here on the work is pure CPU
704+
on an already-connected client, so it is identical either way.
705+
706+
``refresh_ctx`` is stashed on the instance so ``_call_tool`` can
707+
transparently refresh the OAuth token and retry once on a 401 mid-session —
708+
cached access tokens can expire between connect and call.
709+
"""
701710
tool_specs = []
702711
for tool in tools_result.tools:
703712
input_schema = tool.inputSchema if isinstance(tool.inputSchema, dict) else {}
@@ -763,7 +772,30 @@ async def create_stdio_server(
763772
env: dict[str, str] | None = None,
764773
tool_call_timeout: timedelta = timedelta(seconds=60),
765774
) -> MCPTool:
766-
"""Create an MCP tool from explicit stdio configuration without blocking the event loop."""
775+
"""Create an MCP tool from explicit stdio config, from inside an event loop.
776+
777+
:meth:`create_from_server` is synchronous. Called with a loop already
778+
running, it hands the coroutine to a worker thread and blocks on the
779+
result, which stalls the caller's loop for the whole connect and
780+
``list_tools`` round trip. A host that is itself serving over that loop
781+
— the ACP server on stdin/stdout, for instance — goes unresponsive for
782+
as long as the MCP server takes to start.
783+
784+
This awaits the connection in the caller's loop instead, so the host
785+
keeps serving while an MCP server comes up. It covers stdio only: no
786+
config file lookup and no OAuth, because neither applies to a local
787+
subprocess. Use :meth:`create_from_server` for everything else.
788+
789+
Args:
790+
server_name: Name for the generated tool class and its methods.
791+
command: Executable to launch.
792+
args: Arguments passed to ``command``.
793+
env: Extra environment for the subprocess.
794+
tool_call_timeout: Per-call timeout for the generated methods.
795+
796+
Returns:
797+
An MCPTool instance with one method per tool on the server.
798+
"""
767799
client = create_mcp_client(
768800
transport="stdio",
769801
command=command,

0 commit comments

Comments
 (0)