Create BATS tests for limactl-mcp

[jandubois]

I had an idea how to test limactl-mcp with BATS, so I wrote this PoC. I think it works well, but must obviously be fleshed out some more.

This PR depends on #3744 and assumes it has been merged already.

[jandubois]

One thing I found while writing the test was that run_shell_command returns an object that has additional JSON embedded as a string:

$ echo '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"run_shell_command","arguments":{"command":["cat","os-release"],"directory":"/etc"}}}' >&"${MCP[1]}"

$ read -t 1 -r line <&"${MCP[0]}"; jq . <<<"$line"
{
  "jsonrpc": "2.0",
  "id": 3,
  "result": {
    "content": [
      {
        "type": "text",
        "text": "{\"stdout\":\"PRETTY_NAME=\\\"Ubuntu 25.04\\\"\\nNAME=\\\"Ubuntu\\\"\\nVERSION_ID=\\\"25.04\\\"\\nVERSION=\\\"25.04 (Plucky Puffin)\\\"\\nVERSION_CODENAME=plucky\\nID=ubuntu\\nID_LIKE=debian\\nHOME_URL=\\\"https://www.ubuntu.com/\\\"\\nSUPPORT_URL=\\\"https://help.ubuntu.com/\\\"\\nBUG_REPORT_URL=\\\"https://bugs.launchpad.net/ubuntu/\\\"\\nPRIVACY_POLICY_URL=\\\"https://www.ubuntu.com/legal/terms-and-policies/privacy-policy\\\"\\nUBUNTU_CODENAME=plucky\\nLOGO=ubuntu-logo\\n\",\"stderr\":\"\",\"exit_code\":0}"
      }
    ]
  }
}

So you need to extract .text, unmarshall the result into JSON and then extract .stdout, which is quite cumbersome. I don't know if this will bother an LLM, but doesn't this also increase the number of tokens used for the content by escaping all the quotes and backslashes?

Is there a reason this can't be a normal nested object?

[jandubois]

Beyond the encoded JSON thing, I don't understand the schema of the output: Why is content an array? I don't understand how there can be more than one set of results given that the tool only accepts a single command.

And what does type: text mean? Especially given that text is a json string. Could it also be object? Isn't this all too complex for a tool command?

So I would expect the output of run_shell_command to look more like this:

{
  "jsonrpc": "2.0",
  "id": 3,
  "result": {
    "stdout": "...",
    "stderr": "",
    "exit_code": 0
  }
}

[jandubois]

The glob and list_directory tools seem redundant:

If you make the pattern for glob optional and have it default to *, then list_directory is just the same as glob.

[AkihiroSuda]

So I would expect the output of run_shell_command to look more like this:

MCP doesn't allow that schema: https://pkg.go.dev/github.com/modelcontextprotocol/[email protected]/mcp#CallToolResult

StructuredContent may be useful for a similar schema, but it might be relatively new and maybe not understood by some AI agents.

https://modelcontextprotocol.io/specification/2025-06-18/server/tools#structured-content also says "For backwards compatibility, a tool that returns structured content SHOULD also return the serialized JSON in a TextContent block."

I guess we can revisit this later.

The glob and list_directory tools seem redundant:

The current design is to follow Gemini's design
https://github.com/google-gemini/gemini-cli/blob/v0.5.5/docs/tools/shell.md

[jandubois]

I have more questions:

---

The glob description says:

The absolute path to the directory to search within. If omitted, searches the tool's root directory.

What is the tool's root directory? How is the LLM to know what this means?

If this command accepts relative patterns, why do the other tools insist on an absolute path? Why can't you pass a filename returned by glob to read_file? How do you turn the relative file into an absolute file?

---

The description for the run_in_shell command says:

The exact shell command to execute. Defined as a string slice, unlike Gemini's run_shell_command that defines it as a single string.

I think the negative explanation "this is not like something else" is not helpful. Also most LLMs will not even know what the Gemini run_shell_command does.

Describe what the tool does, and what parameters it expects in a prescriptive manner.

---

The write_file tool does not create the directory for the file to be written.

It returns an error file does not exist. I think it should create missing directories. But if it doesn't, the error should be directory does not exist.

[AkihiroSuda]

Opened an issue about StructuredContent for tracking purpose:

• MCP: switch from JSON TextContent to StructuredContent #4070

Please consider opening separate issues for other topics too.

What is the tool's root directory?

The prompt was just copied from https://github.com/google-gemini/gemini-cli/blob/v0.5.5/docs/tools/file-system.md#4-glob-findfiles
Needs to be reworded that it refers to the cwd of the agent, which should correspond to the cwd of the MCP tools too.

If this command accepts relative patterns, why do the other tools insist on an absolute path?

The prompt was just copied from Gemini, but glob seems different from other tools in the sense that it is specifically made for resolving a vague path pattern to an exact pattern.

Why can't you pass a filename returned by glob to read_file?

Bug. glob should return absolute paths that can be passed to read_file.
https://github.com/google-gemini/gemini-cli/blob/v0.5.5/docs/tools/file-system.md#4-glob-findfiles

How do you turn the relative file into an absolute file?

Maybe we should provide an additional MCP tool, if shelling out realpath does not work.

I think the negative explanation "this is not like something else" is not helpful. Also most LLMs will not even know what the Gemini run_shell_command does.

Gemini may know?

The write_file tool does not create the directory for the file to be written.

Bug.

[jandubois]

Why can't you pass a filename returned by glob to read_file?

Bug. glob should return absolute paths that can be passed to read_file.

I must have been confused about this; I can no longer reproduce the issue and get full path names now. I probably mixed it up with the results of list_directory, which has relative paths because the directory name is a required parameter.

[AkihiroSuda]

Needs rebase after merging:

• mcp: use StructuredContent; fix "path is empty" error #4119

[jandubois]

I think this is good enough for a first pass. There is always more that could be covered, but there is at least some coverage now for each tool.

There are 2 skipped tests that should be enabled when the bugs #4173 and #4174 have been fixed.

[AkihiroSuda]

Not necessarily needs to be covered in this PR, but we should consider reusing an instance across multiple bats files

lima/hack/bats/tests/preserve-env.bats

Line 20 in 8b1f07d

limactl start --yes --name "$NAME" template:default 3>&- 4>&-

[jandubois]

Yes, absolutely.

Just for working on the tests you can already do this:

❯ export LIMA_BATS_REUSE_INSTANCE=1

❯ ./hack/bats/lib/bats-core/bin/bats -T ./hack/bats/tests/mcp.bats -f search_file
mcp.bats
 ✓ search_file_content finds text inside files [507]
 ✓ search_file_content can find unicode characters above U+FFFF [510]
 ✓ search_file_content returns an empty string if it cannot find the pattern [502]

3 tests, 0 failures in 2 seconds

❯ ./hack/bats/lib/bats-core/bin/bats -T ./hack/bats/tests/mcp.bats -f search_file
mcp.bats
 ✓ search_file_content finds text inside files [516]
 ✓ search_file_content can find unicode characters above U+FFFF [512]
 ✓ search_file_content returns an empty string if it cannot find the pattern [501]

3 tests, 0 failures in 3 seconds

So you can iterate quickly without restarting the instance every time.

And both mcp.bats and preserve-env.bats are sharing the same bats instance, so it works for both.

[AkihiroSuda]

Nice, the code clone across the bats files should be moved to helpers

[jandubois]

I don't know if #4124 should be considered a bug or not. Right now this test assumes the current scheme is the way it is supposed to be.

[jandubois]

This may need to be changed to refute_output because I think $lines is undefined and not an empty array when there is no output. Can be fixed when the test is no longer skipped.