fix: clone gh: MCP servers into ~/.agents/mcp instead of the OS temp dir

docs/mcp.md

@@ -150,6 +150,20 @@ More agents will be added as their MCP standards solidify.
 | `cargo:` | `cargo:my-mcp-server` | Rust crate via `cargo run` |
 | Local path | `./my-mcp-server/index.js` | Run directly with `node` |
 
+### Where `gh:` servers are cloned
+
+`gh:` servers run from a path on disk, so that path has to survive. Each one is
+cloned to `~/.agents/mcp/<owner-repo>[@ref]/repo` — rolecraft's own directory,
+not the OS temp dir, which is swept on a schedule rolecraft does not control.
+
+Installing the same source again replaces its own clone rather than leaving the
+old one behind. A server name with a ref (`gh:owner/repo@v1.0.0`) gets its own
+directory, so two versions of one repo do not overwrite each other.
+
+`rolecraft mcp list` checks every absolute path in a server entry and marks the
+ones that no longer exist, so an entry whose files went missing is reported
+instead of listed as healthy. Removing a server leaves its clone on disk.
+
 ## Security Scanning
 
 When installing from `gh:` sources, rolecraft automatically scans the cloned repository for security issues before installation:

src/api/mcp.js

@@ -7,6 +7,8 @@ import {
   resolveMcpSource,
 } from '../utils/mcp.js'
 import { assertMcpScanAllowed } from '../utils/scan-gate.js'
+import { existsSync } from 'node:fs'
+import { isAbsolute } from 'node:path'
 
 let runFetch = globalThis.fetch
 
@@ -55,6 +57,28 @@ export async function apiMcpInstall(source, options = {}) {
   }
 }
 
+/**
+ * A `gh:` or local-path server runs from a path on disk, and that path is
+ * written into the agent config. If the directory is later gone the server
+ * cannot start, but the config still lists it, so `mcp list` used to report it
+ * as healthy — the agent silently lost the server with nothing on our side to
+ * say so. Only absolute args are checked: npm/uvx/pipx args are package names,
+ * not paths, and would read as missing.
+ */
+function describeServer(server) {
+  const entry = {
+    name: server.name,
+    command: server.command,
+    args: server.args,
+  }
+  const path = (server.args || []).find((a) => isAbsolute(a))
+  if (path && !existsSync(path)) {
+    entry.missing = true
+    entry.missingPath = path
+  }
+  return entry
+}
+
 export async function apiMcpList(options = {}) {
   const targets =
     options.agents && options.agents.length > 0
@@ -70,8 +94,7 @@ export async function apiMcpList(options = {}) {
     // but the reason for the gap is reported.
     try {
       const servers = await listMcpServers(agent)
-      for (const s of servers)
-        all.push({ agent, name: s.name, command: s.command, args: s.args })
+      for (const s of servers) all.push({ agent, ...describeServer(s) })
     } catch (error) {
       unreadable.push({ agent, error })
     }

src/api/mcp.test.js

@@ -162,6 +162,61 @@ describe('api mcp install/list/update/remove', () => {
     assert.match(result.unreadable[0].error.message, /not valid JSON/)
   })
 
+  // A gh: entry points at a path on disk. When the OS reclaimed that path the
+  // server stopped starting, but list still reported it as a normal server —
+  // the only visible symptom was an agent that had quietly lost it.
+  it('flags a server whose command path no longer exists', async () => {
+    await mkdir(join(tempDir, '.agents'), { recursive: true })
+    const gone = join(
+      tempDir,
+      '.agents',
+      'mcp',
+      'owner-repo',
+      'repo',
+      'index.js',
+    )
+    await writeFile(
+      join(tempDir, '.agents', 'mcp.json'),
+      JSON.stringify({
+        mcpServers: { 'gh-one': { command: 'node', args: [gone] } },
+      }),
+    )
+
+    const result = await apiMcpList({ agents: ['agents'] })
+
+    assert.equal(result.servers.length, 1)
+    assert.equal(result.servers[0].missing, true)
+    assert.match(result.servers[0].missingPath, /index\.js/)
+  })
+
+  it('flags a server only once the file is actually gone', async () => {
+    await mkdir(join(tempDir, '.agents', 'srv'), { recursive: true })
+    const script = join(tempDir, '.agents', 'srv', 'index.js')
+    await writeFile(script, '// present')
+    await writeFile(
+      join(tempDir, '.agents', 'mcp.json'),
+      JSON.stringify({
+        mcpServers: { 'gh-live': { command: 'node', args: [script] } },
+      }),
+    )
+
+    const result = await apiMcpList({ agents: ['agents'] })
+
+    assert.equal(result.servers[0].missing, undefined)
+  })
+
+  it('does not flag npm servers, whose args are not paths', async () => {
+    await apiMcpInstall('npm:@test/server', {
+      yes: true,
+      agents: ['agents'],
+      name: 'server',
+    })
+
+    const result = await apiMcpList({ agents: ['agents'] })
+
+    assert.equal(result.servers[0].missing, undefined)
+  })
+
   it('returns an empty list when no servers are configured', async () => {
     const result = await apiMcpList({ agents: ['agents'] })
     assert.deepEqual(result, { servers: [], total: 0, unreadable: [] })

src/commands/mcp.js

@@ -101,9 +101,23 @@ export async function mcpListCommand(options) {
     s.agent,
     s.name,
     `${s.command} ${(s.args || []).join(' ')}`,
+    s.missing ? `${ICONS.error} path gone` : ICONS.ok,
   ])
-  for (const line of renderTable(['AGENT', 'SERVER', 'COMMAND'], rows))
+  for (const line of renderTable(
+    ['AGENT', 'SERVER', 'COMMAND', 'STATUS'],
+    rows,
+  ))
     console.log(line)
+
+  const broken = result.servers.filter((s) => s.missing)
+  if (broken.length > 0) {
+    console.log(
+      `\n${ICONS.warn}  ${broken.length} server(s) point at a path that no longer exists.`,
+    )
+    for (const s of broken)
+      console.log(`   ${s.agent}/${s.name}: ${s.missingPath}`)
+    console.log('   Reinstall the source: rolecraft mcp install <source>')
+  }
 }
 
 export async function mcpUpdateCommand(source, options) {

src/commands/mcp.test.js

@@ -205,6 +205,52 @@ describe('mcp command', () => {
         restore()
 
         assert.ok(logs.some((l) => l.includes('list-test')))
+        assert.ok(!logs.some((l) => l.includes('path gone')))
+      }),
+    )
+
+    it(
+      'reports a server whose command path is gone instead of listing it as fine',
+      withTempDir(async () => {
+        const { mkdirSync, writeFileSync } = await import('node:fs')
+        const addModule = await import('../utils/mcp.js')
+        await addModule.addMcpServer('agents', 'gh-gone', {
+          command: 'node',
+          args: [
+            join(process.env.HOME, '.agents', 'mcp', 'owner-repo', 'index.js'),
+          ],
+        })
+        mkdirSync(join(process.env.HOME, '.agents', 'mcp', 'owner-repo'), {
+          recursive: true,
+        })
+        writeFileSync(
+          join(process.env.HOME, '.agents', 'mcp', 'owner-repo', 'index.js'),
+          '// here',
+        )
+
+        const { logs, restore } = capture('log')
+        await mcpModule.mcpListCommand({ agents: ['agents'] })
+        restore()
+        assert.ok(!logs.some((l) => l.includes('path gone')))
+
+        // Now take the clone away, the way an OS temp sweep would.
+        const { rmSync } = await import('node:fs')
+        rmSync(join(process.env.HOME, '.agents', 'mcp', 'owner-repo'), {
+          recursive: true,
+          force: true,
+        })
+
+        const second = capture('log')
+        await mcpModule.mcpListCommand({ agents: ['agents'] })
+        second.restore()
+
+        const output = second.logs.join('\n')
+        assert.match(
+          output,
+          /path gone/,
+          `expected the break to be reported: ${output}`,
+        )
+        assert.match(output, /rolecraft mcp install/, 'expected a repair hint')
       }),
     )
   })

src/utils/mcp.js

@@ -1,13 +1,5 @@
-import {
-  readFile,
-  writeFile,
-  mkdir,
-  rm,
-  mkdtemp,
-  readdir,
-} from 'node:fs/promises'
+import { readFile, writeFile, mkdir, rm, readdir } from 'node:fs/promises'
 import { join, dirname, relative } from 'node:path'
-import { tmpdir } from 'node:os'
 import {
   execSync as defaultExecSync,
   spawnSync as defaultSpawnSync,
@@ -16,7 +8,9 @@ import agents from '../agents.js'
 import { addServerToMcpLock, removeServerFromMcpLock } from './mcp-lock.js'
 import { parseFrontmatter } from './converter.js'
 import { UserError } from './errors.js'
-import { expandTilde } from './paths.js'
+import { normalizeSlug } from './lockfile.js'
+import { assertSafeSlug } from './installer.js'
+import { expandTilde, home } from './paths.js'
 
 let _runExec = defaultExecSync
 let runSpawnSync = defaultSpawnSync
@@ -244,6 +238,22 @@ export function classifyMcpSource(source) {
   return { label: 'local path', type: 'local' }
 }
 
+export function getMcpServerDir() {
+  return home('.agents', 'mcp')
+}
+
+/**
+ * Where a `gh:` server is cloned to. Deterministic per source, so reinstalling
+ * the same source replaces its own clone instead of leaking a new directory.
+ */
+export function getMcpCloneDir(repo, ref = null) {
+  const baseDir = getMcpServerDir()
+  const slug = normalizeSlug(ref ? `${repo}@${ref}` : repo)
+  const dir = join(baseDir, slug)
+  assertSafeSlug(slug, baseDir, dir)
+  return dir
+}
+
 export async function resolveMcpSource(source) {
   if (source.startsWith('npm:')) {
     let pkg = source.slice(4)
@@ -270,9 +280,15 @@ export async function resolveMcpSource(source) {
       ref = repo.slice(atIdx + 1)
       repo = repo.slice(0, atIdx)
     }
-    const tmpDir = await mkdtemp(join(tmpdir(), 'rolecraft-mcp-'))
-    const cloneDir = join(tmpDir, 'repo')
+    const cloneDir = join(getMcpCloneDir(repo, ref), 'repo')
     try {
+      // git clone refuses a non-empty target, so a reinstall of the same
+      // source has to clear the previous clone first. Cloning into the
+      // rolecraft-owned directory rather than the OS temp dir is what makes
+      // the entry survive: the path goes into the agent config, and /tmp is
+      // swept on a schedule rolecraft does not control.
+      await rm(cloneDir, { recursive: true, force: true })
+      await mkdir(dirname(cloneDir), { recursive: true })
       const cloneArgs = [
         'clone',
         '--depth',
@@ -321,8 +337,6 @@ export async function resolveMcpSource(source) {
       }
       await readFilesRecursive(cloneDir, cloneDir)
 
-      // Keep tmpDir on disk — MCP server needs the files at runtime.
-      // The OS cleans /tmp periodically; on error the catch block removes it.
       return {
         command: 'node',
         args: [command],
@@ -332,7 +346,7 @@ export async function resolveMcpSource(source) {
         fileContents,
       }
     } catch (err) {
-      await rm(tmpDir, { recursive: true, force: true }).catch(() => {})
+      await rm(cloneDir, { recursive: true, force: true }).catch(() => {})
       throw err
     }
   }