From a643c13111cdbb33690da5cade6963d62ede453d Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:21:26 -0400
Subject: [PATCH 1/8] Add GitHub Actions CI workflow
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Add GitHub Actions CI workflow

Runs lint, tests, and build on push/PR to master across Node 18/20/22.
The repo currently has no CI — the test-results badge is updated
manually, so regressions can land without any automated check.

* Drop Node 18 from CI matrix — dev toolchain requires Node >=20

PR CI was failing on the Node 18.x job: vitest 4's transitive dependency
rolldown imports `styleText` from node:util, which doesn't exist until
Node 20. npm's EBADENGINE warnings during install confirm several
devDependencies (commander@14, vite@8, vitest@4, mute-stream@3,
@oxc-project/runtime) now require Node >=20.

This isn't a flake — Node 18 genuinely cannot run `npm test`/`npm run
build` with the current toolchain. Matrix now covers 20.x/22.x/24.x,
which is what's actually being verified as working.

Separately (not addressed here): the published CLI's dist bundle also
pulls in @inquirer/select, whose dependency chain calls the same
Node-20+-only `styleText` API eagerly at module-load time — so running
the built CLI on Node 18 likely fails too, despite package.json's
"engines": ">=18". Worth a follow-up to either bump the documented
minimum or address the dependency.
---
 .github/workflows/ci.yml | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 .github/workflows/ci.yml

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..dd3c930
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,40 @@
+name: CI
+
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+
+jobs:
+  test:
+    name: Lint & Test (Node ${{ matrix.node-version }})
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        # Node 18 is dropped from this matrix: the dev/test toolchain (vitest 4,
+        # tsup/rolldown, commander 14, @inquirer/select 5) requires Node >=20 and
+        # cannot run `npm test`/`npm run build` on Node 18. This is separate from
+        # (and doesn't verify) the package's documented "engines": ">=18" for the
+        # published CLI itself.
+        node-version: [20.x, 22.x, 24.x]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+          cache: npm
+
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Lint (type-check)
+        run: npm run lint
+
+      - name: Run unit & integration tests
+        run: npm test
+
+      - name: Build
+        run: npm run build

From a5236e315176c9bc5564ed8f558a99039b130f8d Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:23:25 -0400
Subject: [PATCH 2/8] Fix temp-dir leak when asset validation fails (#2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

resolveGit/resolveHttpFile clone or download sources into os.tmpdir()
before validateAsset runs. When validation failed, runInstaller called
process.exit(2) directly — which does not run pending finally blocks —
so the cleanup loop at the end of the function never executed and the
temp dir was left on disk.

Track resolved temp dirs in a Set as sources are resolved and clean
them up explicitly before every process.exit(2) call, not just on the
success path.
---
 src/installer.ts             | 28 +++++++++------
 tests/unit/installer.test.ts | 68 ++++++++++++++++++++++++++++++++++++
 2 files changed, 86 insertions(+), 10 deletions(-)
 create mode 100644 tests/unit/installer.test.ts

diff --git a/src/installer.ts b/src/installer.ts
index 1d35ced..b3b7127 100644
--- a/src/installer.ts
+++ b/src/installer.ts
@@ -27,6 +27,16 @@ export interface CliInput {
   host?: string;
 }
 
+async function cleanupTempDirs(tempDirs: Set<string>): Promise<void> {
+  for (const dir of tempDirs) {
+    try {
+      await fs.promises.rm(dir, { recursive: true, force: true });
+    } catch {
+      // ignore cleanup errors
+    }
+  }
+}
+
 function getHandler(assetType: AssetType) {
   switch (assetType) {
     case 'mcp': return mcpHandler;
@@ -147,12 +157,17 @@ export async function runInstaller(
     fromExplicitFlag: boolean;
   }> = [];
 
+  const tempDirsToClean = new Set<string>();
+
   let explicitCount = explicitDescriptors.length;
   let idx = 0;
   for (const item of expandedItems) {
     const isGlob = item.source.endsWith('/*');
     const cleanSource = isGlob ? item.source.slice(0, -2) : item.source;
     const resolved = await resolveSource(cleanSource, cwd);
+    if (resolved.tempDir) {
+      tempDirsToClean.add(resolved.tempDir);
+    }
     const fromExplicitFlag = idx < explicitCount;
 
     if (isGlob) {
@@ -162,6 +177,7 @@ export async function runInstaller(
           `agent-add error: No matching files found in directory for ${item.assetType}\n` +
           `  Source: ${item.source}\n`,
         );
+        await cleanupTempDirs(tempDirsToClean);
         process.exit(2);
       }
       const dirName = path.basename(resolved.localPath);
@@ -190,6 +206,7 @@ export async function runInstaller(
     if (validationError) {
       process.stderr.write(`agent-add error: ${validationError}\n`);
       process.stderr.write(`  Source: ${item.resolved.originalSource}\n`);
+      await cleanupTempDirs(tempDirsToClean);
       process.exit(2);
     }
   }
@@ -228,16 +245,7 @@ export async function runInstaller(
     results.push(result);
   }
 
-  for (const item of resolvedItems) {
-    if (item.resolved.tempDir) {
-      try {
-        const fs2 = await import('fs');
-        await fs2.promises.rm(item.resolved.tempDir, { recursive: true, force: true });
-      } catch {
-        // ignore cleanup errors
-      }
-    }
-  }
+  await cleanupTempDirs(tempDirsToClean);
 
   return { host, results };
 }
diff --git a/tests/unit/installer.test.ts b/tests/unit/installer.test.ts
new file mode 100644
index 0000000..f9b3d92
--- /dev/null
+++ b/tests/unit/installer.test.ts
@@ -0,0 +1,68 @@
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { runInstaller } from '../../src/installer.js';
+import { getHost } from '../../src/hosts/index.js';
+
+function listAgentAddTempDirs(): string[] {
+  return fs
+    .readdirSync(os.tmpdir())
+    .filter((name) => name.startsWith('agent-add-http-'))
+    .map((name) => path.join(os.tmpdir(), name));
+}
+
+describe('runInstaller temp-dir cleanup', () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+    vi.unstubAllGlobals();
+  });
+
+  it('removes downloaded temp dirs even when validation fails', async () => {
+    const before = new Set(listAgentAddTempDirs());
+
+    vi.stubGlobal(
+      'fetch',
+      vi.fn().mockResolvedValue(
+        new Response('not json', {
+          status: 200,
+          headers: { 'content-type': 'text/plain' },
+        }),
+      ),
+    );
+
+    const exitSpy = vi
+      .spyOn(process, 'exit')
+      .mockImplementation(((code?: number) => {
+        throw new Error(`process.exit(${code})`);
+      }) as never);
+    const stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true);
+
+    const host = getHost('claude-code')!;
+
+    await expect(
+      runInstaller(
+        {
+          // .txt extension fails MCP asset validation (must be .json)
+          mcp: ['https://example.com/not-an-mcp-config.txt'],
+          skill: [],
+          prompt: [],
+          command: [],
+          subAgent: [],
+          pack: [],
+          host: 'claude-code',
+        },
+        host,
+        process.cwd(),
+      ),
+    ).rejects.toThrow('process.exit(2)');
+
+    expect(exitSpy).toHaveBeenCalledWith(2);
+
+    const after = new Set(listAgentAddTempDirs());
+    const leaked = [...after].filter((dir) => !before.has(dir));
+    expect(leaked).toEqual([]);
+
+    stderrSpy.mockRestore();
+  });
+});

From 0f5db9fa8c3c4c23bd1785e755267982d698e29f Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:24:44 -0400
Subject: [PATCH 3/8] Fix git source ref/subPath parsing (#3)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Extracts the ref/subPath splitting logic from resolveGit into a pure,
unit-testable parseGitSource function and fixes two bugs found in it:

1. The @ref separator was found via the first "@" after an optional
   leading "git@" prefix. For HTTPS URLs with embedded credentials
   (https://token@github.com/org/repo.git@v1.0.0), this matched the
   credentials separator instead of the ref separator, truncating the
   repo URL to "https://token". The search now starts at the path
   segment (after the authority) for non-SSH URLs, so any @ in
   userinfo is skipped.

2. normalizeGitUrl (used for GitHub/GitLab /tree/<ref>/<path> web URLs)
   emits sources in "repo.git#path@ref" order, but resolveGit only ever
   looked for @ref in the part *before* "#" — so the ref was silently
   dropped (falling back to HEAD) and the literal string "path@ref" was
   used as the subPath, which doesn't exist in the checked-out repo.
   parseGitSource now also recognizes a trailing @ref after the
   subPath.

README's documented "repo.git@ref#path" ordering continues to work as
before.
---
 src/source/git.ts                          | 59 ++++++++++++--
 tests/unit/source/parse-git-source.test.ts | 90 ++++++++++++++++++++++
 2 files changed, 144 insertions(+), 5 deletions(-)
 create mode 100644 tests/unit/source/parse-git-source.test.ts

diff --git a/src/source/git.ts b/src/source/git.ts
index d975085..26dafa7 100644
--- a/src/source/git.ts
+++ b/src/source/git.ts
@@ -8,18 +8,67 @@ import type { SourceType } from './index.js';
 
 const execFileAsync = promisify(execFile);
 
-export async function resolveGit(source: string, type: SourceType): Promise<ResolvedSource> {
+export interface ParsedGitSource {
+  repoUrl: string;
+  ref?: string;
+  subPath?: string;
+}
+
+/**
+ * Split a git source string into repo URL, optional @ref, and optional #subPath.
+ *
+ * Two ref orderings are supported:
+ *   - `repo.git@ref#path` (the documented convention for user-supplied sources)
+ *   - `repo.git#path@ref` (emitted by normalizeGitUrl for GitHub/GitLab
+ *     `/tree/<ref>/<path>` web URLs)
+ *
+ * For non-SSH URLs, the @ref separator before `#` is only searched for within
+ * the path segment (after the authority), so credentials embedded in the URL
+ * (e.g. https://token@github.com/org/repo.git) are not mistaken for a ref
+ * separator.
+ */
+export function parseGitSource(source: string): ParsedGitSource {
   // Step 1: split off #subPath
   const hashIdx = source.indexOf('#');
   const withoutPath = hashIdx !== -1 ? source.slice(0, hashIdx) : source;
-  const subPath = hashIdx !== -1 ? source.slice(hashIdx + 1) || undefined : undefined;
+  const rawSubPath = hashIdx !== -1 ? source.slice(hashIdx + 1) || undefined : undefined;
 
-  // Step 2: split off @ref — for SSH URLs (git@host:...), skip the leading "git@" prefix
+  // Step 2: split off @ref — for SSH URLs (git@host:...), skip the leading "git@" prefix.
+  // For other URLs, only look for @ within the path segment, so userinfo
+  // credentials (https://user:pass@host/... or https://token@host/...) aren't
+  // mistaken for the ref separator.
   const isSSH = withoutPath.startsWith('git@');
-  const searchFrom = isSSH ? 4 : 0;
+  let searchFrom = 0;
+  if (isSSH) {
+    searchFrom = 4;
+  } else {
+    const schemeIdx = withoutPath.indexOf('://');
+    if (schemeIdx !== -1) {
+      const authorityStart = schemeIdx + 3;
+      const pathStart = withoutPath.indexOf('/', authorityStart);
+      searchFrom = pathStart !== -1 ? pathStart : withoutPath.length;
+    }
+  }
   const atIdx = withoutPath.indexOf('@', searchFrom);
   const repoUrl = atIdx !== -1 ? withoutPath.slice(0, atIdx) : withoutPath;
-  const ref = atIdx !== -1 ? withoutPath.slice(atIdx + 1) || undefined : undefined;
+  let ref = atIdx !== -1 ? withoutPath.slice(atIdx + 1) || undefined : undefined;
+
+  // Step 3: if no ref was found before `#`, check for a trailing @ref after
+  // the subPath (the `#path@ref` ordering).
+  let subPath = rawSubPath;
+  if (!ref && rawSubPath) {
+    const subAtIdx = rawSubPath.lastIndexOf('@');
+    if (subAtIdx !== -1) {
+      ref = rawSubPath.slice(subAtIdx + 1) || undefined;
+      subPath = rawSubPath.slice(0, subAtIdx) || undefined;
+    }
+  }
+
+  return { repoUrl, ref, subPath };
+}
+
+export async function resolveGit(source: string, type: SourceType): Promise<ResolvedSource> {
+  const { repoUrl, ref, subPath } = parseGitSource(source);
 
   const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-git-'));
 
diff --git a/tests/unit/source/parse-git-source.test.ts b/tests/unit/source/parse-git-source.test.ts
new file mode 100644
index 0000000..a459dff
--- /dev/null
+++ b/tests/unit/source/parse-git-source.test.ts
@@ -0,0 +1,90 @@
+import { describe, it, expect } from 'vitest';
+import { parseGitSource } from '../../../src/source/git.js';
+
+describe('parseGitSource', () => {
+  describe('HTTPS URLs', () => {
+    it('parses a plain repo URL with no ref or subpath', () => {
+      expect(parseGitSource('https://github.com/org/repo.git')).toEqual({
+        repoUrl: 'https://github.com/org/repo.git',
+        ref: undefined,
+        subPath: undefined,
+      });
+    });
+
+    it('parses @ref', () => {
+      expect(parseGitSource('https://github.com/org/repo.git@v1.0.0')).toEqual({
+        repoUrl: 'https://github.com/org/repo.git',
+        ref: 'v1.0.0',
+        subPath: undefined,
+      });
+    });
+
+    it('parses #subPath', () => {
+      expect(parseGitSource('https://github.com/org/repo.git#skills/pdf')).toEqual({
+        repoUrl: 'https://github.com/org/repo.git',
+        ref: undefined,
+        subPath: 'skills/pdf',
+      });
+    });
+
+    it('parses #subPath@ref', () => {
+      expect(parseGitSource('https://github.com/org/repo.git#skills/pdf@main')).toEqual({
+        repoUrl: 'https://github.com/org/repo.git',
+        ref: 'main',
+        subPath: 'skills/pdf',
+      });
+    });
+
+    it('does not mistake a token embedded as userinfo for the ref separator', () => {
+      expect(parseGitSource('https://ghp_token123@github.com/org/repo.git@v1.0.0')).toEqual({
+        repoUrl: 'https://ghp_token123@github.com/org/repo.git',
+        ref: 'v1.0.0',
+        subPath: undefined,
+      });
+    });
+
+    it('does not mistake user:pass credentials for the ref separator', () => {
+      expect(parseGitSource('https://user:pass@github.com/org/repo.git')).toEqual({
+        repoUrl: 'https://user:pass@github.com/org/repo.git',
+        ref: undefined,
+        subPath: undefined,
+      });
+    });
+
+    it('parses the documented @ref#path ordering', () => {
+      expect(parseGitSource('https://github.com/org/repo.git@v1.0#path/to/skill')).toEqual({
+        repoUrl: 'https://github.com/org/repo.git',
+        ref: 'v1.0',
+        subPath: 'path/to/skill',
+      });
+    });
+
+    it('preserves credentials and still parses ref + subpath together', () => {
+      expect(
+        parseGitSource('https://user:pass@github.com/org/repo.git#skills/pdf@main'),
+      ).toEqual({
+        repoUrl: 'https://user:pass@github.com/org/repo.git',
+        ref: 'main',
+        subPath: 'skills/pdf',
+      });
+    });
+  });
+
+  describe('SSH URLs', () => {
+    it('parses a plain repo URL with no ref', () => {
+      expect(parseGitSource('git@github.com:org/repo.git')).toEqual({
+        repoUrl: 'git@github.com:org/repo.git',
+        ref: undefined,
+        subPath: undefined,
+      });
+    });
+
+    it('parses @ref, skipping the leading git@ prefix', () => {
+      expect(parseGitSource('git@github.com:org/repo.git@v1.0.0')).toEqual({
+        repoUrl: 'git@github.com:org/repo.git',
+        ref: 'v1.0.0',
+        subPath: undefined,
+      });
+    });
+  });
+});

From bbb96aace17d56aa6a630b36d4ddad57aeaa4356 Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:25:50 -0400
Subject: [PATCH 4/8] Fix source-type detection misclassifying non-git HTTP
 file URLs (#4)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

detectSourceType treated any http(s) URL containing "#" as a git
repository, so a plain file URL with a fragment (e.g.
https://example.com/prompt.md#usage) would attempt a git clone against
a non-git HTTP endpoint instead of being fetched directly. This
double-purposes "#" both as a subPath marker for git sources and as a
literal URL fragment for direct file downloads.

Only treat "#" as a git subPath marker when the URL portion before it
doesn't already look like a direct file this tool knows how to fetch
(.json/.md) — the only asset types resolved via plain HTTP download.
---
 src/source/index.ts                  | 14 +++++++++++++-
 tests/unit/source/uri-detect.test.ts | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/source/index.ts b/src/source/index.ts
index cc7102c..faacc6c 100644
--- a/src/source/index.ts
+++ b/src/source/index.ts
@@ -10,6 +10,11 @@ export type SourceType = 'local' | 'git-ssh' | 'git-https' | 'http-file' | 'inli
 // .git suffix: appears as .git at end, before #, @, or /
 const GIT_REPO_SUFFIX_RE = /\.git(\/|@|#|$)/;
 
+// Extensions this tool fetches directly as a single file (mcp/prompt/command/subAgent).
+// If the part of the URL before "#" already looks like one of these, the "#"
+// is treated as a URL fragment rather than a git subPath marker.
+const DIRECT_FILE_EXT_RE = /\.(json|md)$/i;
+
 // GitHub web URL: https://github.com/owner/repo[/tree|blob/ref/path]
 const GITHUB_WEB_RE = /^https?:\/\/(www\.)?github\.com\/([^/#]+\/[^/#]+?)(?:\.git)?\/?(?:\/(tree|blob)\/([^/]+)\/?(.*?))?$/;
 
@@ -73,9 +78,16 @@ export function detectSourceType(source: string): SourceType {
     return 'git-ssh';
   }
   if (source.startsWith('https://') || source.startsWith('http://')) {
-    if (GIT_REPO_SUFFIX_RE.test(source) || source.includes('#')) {
+    if (GIT_REPO_SUFFIX_RE.test(source)) {
       return 'git-https';
     }
+    const hashIdx = source.indexOf('#');
+    if (hashIdx !== -1) {
+      const beforeHash = (source.slice(0, hashIdx).split('?')[0] ?? '');
+      if (!DIRECT_FILE_EXT_RE.test(beforeHash)) {
+        return 'git-https';
+      }
+    }
     return 'http-file';
   }
   // Fallback: try JSON.parse to handle edge cases where MSYS inserts a non-whitespace
diff --git a/tests/unit/source/uri-detect.test.ts b/tests/unit/source/uri-detect.test.ts
index 95387c5..503c09c 100644
--- a/tests/unit/source/uri-detect.test.ts
+++ b/tests/unit/source/uri-detect.test.ts
@@ -105,5 +105,21 @@ describe('detectSourceType', () => {
     it('https URL with # but no .git is git-https', () => {
       expect(detectSourceType('https://github.com/org/repo#main/src')).toBe('git-https');
     });
+
+    it('plain .md file URL with a # fragment is http-file, not git-https (regression)', () => {
+      expect(detectSourceType('https://example.com/prompt.md#usage')).toBe('http-file');
+    });
+
+    it('plain .json file URL with a # fragment is http-file, not git-https (regression)', () => {
+      expect(detectSourceType('https://example.com/config.json#section')).toBe('http-file');
+    });
+
+    it('plain .md file URL with a query string and # fragment is http-file', () => {
+      expect(detectSourceType('https://example.com/prompt.md?raw=1#usage')).toBe('http-file');
+    });
+
+    it('non-.git repo URL with # subPath ending in .json is still git-https when the repo itself has no extension', () => {
+      expect(detectSourceType('https://git.example.com/org/repo#configs/settings.json')).toBe('git-https');
+    });
   });
 });

From ccb1eba370d49e3d1d7f6f6c17028a6fd9b5d7ec Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:27:21 -0400
Subject: [PATCH 5/8] Fix stripExtension mangling directory-based skill asset
 names (#5)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

inferName() always stripped everything after the last "." when
deriving an asset name from a local basename or a git #path last
segment. This is correct for file-based assets (mcp/.json,
prompt|command|subAgent/.md), but skill sources always resolve to a
directory — a dotted directory name like "pdf.js-tools" was being
truncated to "pdf".

Added an isDirectorySource option to inferName, passed from
installer.ts based on the asset type being installed. The git-repo-name
branch (repo.git -> repo) is unaffected, since stripping the ".git"
URL suffix is always correct there regardless of asset type.
---
 src/installer.ts                     |  2 +-
 src/source/infer-name.ts             | 22 ++++++++++++++++------
 tests/unit/source/infer-name.test.ts | 21 +++++++++++++++++++++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/src/installer.ts b/src/installer.ts
index b3b7127..a83237d 100644
--- a/src/installer.ts
+++ b/src/installer.ts
@@ -190,7 +190,7 @@ export async function runInstaller(
         });
       }
     } else {
-      const assetName = inferName(cleanSource);
+      const assetName = inferName(cleanSource, { isDirectorySource: item.assetType === 'skill' });
       resolvedItems.push({
         assetType: item.assetType,
         assetName,
diff --git a/src/source/infer-name.ts b/src/source/infer-name.ts
index 993974d..48cae75 100644
--- a/src/source/infer-name.ts
+++ b/src/source/infer-name.ts
@@ -1,17 +1,27 @@
 import path from 'path';
 import { unwrapMcpServers } from '../utils/unwrap-mcp-servers.js';
 
+export interface InferNameOptions {
+  /**
+   * Set for sources that resolve to a directory (e.g. skill assets), where
+   * the last path segment / basename IS the asset name and must not have a
+   * trailing ".something" stripped as if it were a file extension.
+   */
+  isDirectorySource?: boolean;
+}
+
 /**
  * Infer asset name from source string.
  *
  * Rules:
  * 0. If inline JSON (starts with `{`): extract the single top-level key
  * 0. If inline Markdown (contains `\n`): extract first `# Heading` and kebab-case it
- * 1. If source contains `#path`, use last segment of path (minus extension)
+ * 1. If source contains `#path`, use last segment of path (minus extension, unless isDirectorySource)
  * 2. If git URL without #path (e.g. git@...repo.git), use repo name (strip .git)
- * 3. If local path or http-file URL, use filename without extension
+ * 3. If local path or http-file URL, use filename without extension (unless isDirectorySource)
  */
-export function inferName(source: string): string {
+export function inferName(source: string, options: InferNameOptions = {}): string {
+  const { isDirectorySource = false } = options;
   const s = source.trim(); // normalize: remove leading/trailing whitespace or BOM
 
   // Inline JSON: extract single top-level key as name
@@ -64,8 +74,8 @@ export function inferName(source: string): string {
     const subPath = s.slice(hashIdx + 1);
     const segments = subPath.split('/').filter(Boolean);
     if (segments.length > 0) {
-      const last = segments[segments.length - 1];
-      return stripExtension(last);
+      const last = segments[segments.length - 1]!;
+      return isDirectorySource ? last : stripExtension(last);
     }
   }
 
@@ -80,7 +90,7 @@ export function inferName(source: string): string {
 
   // Local path or HTTP file: use filename without extension
   const basename = path.basename(s.split('?')[0] ?? s);
-  return stripExtension(basename);
+  return isDirectorySource ? basename : stripExtension(basename);
 }
 
 function stripExtension(filename: string): string {
diff --git a/tests/unit/source/infer-name.test.ts b/tests/unit/source/infer-name.test.ts
index 58a23ff..38b1d29 100644
--- a/tests/unit/source/infer-name.test.ts
+++ b/tests/unit/source/infer-name.test.ts
@@ -103,4 +103,25 @@ describe('inferName', () => {
       expect(inferName('https://github.com/org/repo.git@abc123f')).toBe('repo');
     });
   });
+
+  describe('isDirectorySource (skill assets)', () => {
+    it('should not strip a dotted directory name from a local path (regression)', () => {
+      expect(inferName('./skills/pdf.js-tools', { isDirectorySource: true })).toBe('pdf.js-tools');
+    });
+
+    it('should not strip a dotted directory name from a git #path last segment (regression)', () => {
+      expect(inferName('git@github.com:demo/skills.git#pdf.js-tools', { isDirectorySource: true })).toBe(
+        'pdf.js-tools',
+      );
+    });
+
+    it('should still strip .git suffix for a bare repo (whole repo is the skill directory)', () => {
+      expect(inferName('git@github.com:org/repo.git', { isDirectorySource: true })).toBe('repo');
+    });
+
+    it('should behave identically to the default when the name has no dot', () => {
+      expect(inferName('./skills/e2e-guide', { isDirectorySource: true })).toBe('e2e-guide');
+      expect(inferName('./skills/e2e-guide')).toBe('e2e-guide');
+    });
+  });
 });

From 0cfb2146e5f2ab1c5a7541eead440d2cfe351648 Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:28:18 -0400
Subject: [PATCH 6/8] Fix TTY detection to require both stdin and stdout be
 interactive (#6)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The check for whether to show the interactive host-selection prompt
only looked at process.stdout.isTTY. An interactive prompt (via
@inquirer/select) needs to both read from an interactive stdin and
render to an interactive stdout — checking only stdout misses the case
where stdin is piped/redirected but stdout is a terminal (e.g. `agent-add
--mcp ... < /dev/null`), which would proceed into the prompt and hang
waiting on input that will never arrive interactively.

Now requires both process.stdin.isTTY and process.stdout.isTTY before
attempting the interactive prompt.
---
 src/cli.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cli.ts b/src/cli.ts
index b3a4283..86ea092 100644
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -89,7 +89,7 @@ Examples:
 
       if (cliInput.host) {
         hostId = cliInput.host;
-      } else if (!process.stdout.isTTY) {
+      } else if (!process.stdin.isTTY || !process.stdout.isTTY) {
         const validIds = getValidHostIds().join(', ');
         process.stderr.write(
           `agent-add error: Non-interactive environment detected. Please specify a host with --host <host>.\n`,

From 0649adc9c24f08a5bf2a355ce7f50d6d7db7bcdc Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:29:37 -0400
Subject: [PATCH 7/8] Unify error messages to English (#7)

Several validation and parsing error messages (installer.ts's
validateAsset, source/inline.ts, source/infer-name.ts) were written in
Chinese while the rest of the CLI's user-facing output (help text,
CLI errors, host capability messages) is English. Translated for
consistency; no behavior change.
---
 src/installer.ts         | 12 ++++++------
 src/source/infer-name.ts |  8 ++++----
 src/source/inline.ts     |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/installer.ts b/src/installer.ts
index a83237d..afb5ad4 100644
--- a/src/installer.ts
+++ b/src/installer.ts
@@ -53,13 +53,13 @@ async function validateAsset(
 ): Promise<string | null> {
   if (assetType === 'skill') {
     if (resolved.type === 'http-file' || resolved.type === 'inline-json' || resolved.type === 'inline-md') {
-      return 'Skill 资产必须指向目录来源（本地路径或 Git URL），不支持内联内容或直接 HTTP(S) URL';
+      return 'Skill assets must point to a directory source (local path or Git URL); inline content and direct HTTP(S) URLs are not supported';
     }
     const skillMdPath = path.join(resolved.localPath, 'SKILL.md');
     try {
       await fs.promises.access(skillMdPath);
     } catch {
-      return `Skill 目录内缺少 SKILL.md 文件（期望路径：${skillMdPath}）`;
+      return `Skill directory is missing a SKILL.md file (expected path: ${skillMdPath})`;
     }
     return null;
   }
@@ -68,10 +68,10 @@ async function validateAsset(
     try {
       await fs.promises.access(resolved.localPath);
     } catch {
-      return `MCP 来源文件不存在：${resolved.localPath}`;
+      return `MCP source file does not exist: ${resolved.localPath}`;
     }
     if (!resolved.localPath.endsWith('.json')) {
-      return `MCP 来源文件扩展名必须为 .json（得到：${resolved.localPath}）`;
+      return `MCP source file extension must be .json (got: ${resolved.localPath})`;
     }
     return null;
   }
@@ -80,10 +80,10 @@ async function validateAsset(
     try {
       await fs.promises.access(resolved.localPath);
     } catch {
-      return `来源文件不存在：${resolved.localPath}`;
+      return `Source file does not exist: ${resolved.localPath}`;
     }
     if (!resolved.localPath.endsWith('.md')) {
-      return `${assetType} 来源文件扩展名必须为 .md（得到：${resolved.localPath}）`;
+      return `${assetType} source file extension must be .md (got: ${resolved.localPath})`;
     }
     return null;
   }
diff --git a/src/source/infer-name.ts b/src/source/infer-name.ts
index 48cae75..489297f 100644
--- a/src/source/infer-name.ts
+++ b/src/source/infer-name.ts
@@ -31,11 +31,11 @@ export function inferName(source: string, options: InferNameOptions = {}): strin
       parsed = JSON.parse(s);
     } catch {
       throw new Error(
-        `内联 JSON 解析失败。格式应为 {"<name>":{...}}，例如：{"playwright":{"command":"npx","args":["-y","@playwright/mcp"]}}`,
+        `Failed to parse inline JSON. Expected format: {"<name>":{...}}, e.g.: {"playwright":{"command":"npx","args":["-y","@playwright/mcp"]}}`,
       );
     }
     if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
-      throw new Error(`内联 JSON 必须为对象类型`);
+      throw new Error(`Inline JSON must be an object`);
     }
     const obj = parsed as Record<string, unknown>;
     const unwrapped = unwrapMcpServers(obj);
@@ -45,7 +45,7 @@ export function inferName(source: string, options: InferNameOptions = {}): strin
     const keys = Object.keys(obj);
     if (keys.length !== 1) {
       throw new Error(
-        `内联 JSON 必须包含恰好一个 key（作为资产名称），当前有 ${keys.length} 个 key`,
+        `Inline JSON must contain exactly one key (used as the asset name), got ${keys.length} keys`,
       );
     }
     return keys[0]!;
@@ -64,7 +64,7 @@ export function inferName(source: string, options: InferNameOptions = {}): strin
       }
     }
     throw new Error(
-      `内联 Markdown 必须包含一级标题（如 # My Prompt）以推断资产名称`,
+      `Inline Markdown must contain a level-1 heading (e.g. # My Prompt) to infer the asset name`,
     );
   }
 
diff --git a/src/source/inline.ts b/src/source/inline.ts
index 9868bf8..b8041a2 100644
--- a/src/source/inline.ts
+++ b/src/source/inline.ts
@@ -10,12 +10,12 @@ export async function resolveInlineJson(source: string, assetName: string): Prom
     parsed = JSON.parse(source);
   } catch {
     throw new Error(
-      `内联 JSON 解析失败。格式应为 {"<name>":{...}}，例如：{"playwright":{"command":"npx","args":["-y","@playwright/mcp"]}}`,
+      `Failed to parse inline JSON. Expected format: {"<name>":{...}}, e.g.: {"playwright":{"command":"npx","args":["-y","@playwright/mcp"]}}`,
     );
   }
 
   if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
-    throw new Error(`内联 JSON 必须为对象类型，得到：${JSON.stringify(parsed)}`);
+    throw new Error(`Inline JSON must be an object, got: ${JSON.stringify(parsed)}`);
   }
 
   const obj = parsed as Record<string, unknown>;
@@ -28,7 +28,7 @@ export async function resolveInlineJson(source: string, assetName: string): Prom
     const entries = Object.entries(obj);
     if (entries.length !== 1) {
       throw new Error(
-        `内联 JSON 必须包含恰好一个 key（作为资产名称），当前有 ${entries.length} 个 key`,
+        `Inline JSON must contain exactly one key (used as the asset name), got ${entries.length} keys`,
       );
     }
     [, value] = entries[0]!;

From ff3228f3c66efe29f191fbfd5bfded46a11adfcd Mon Sep 17 00:00:00 2001
From: Griffen Fargo <GhFargo@gmail.com>
Date: Fri, 10 Jul 2026 11:31:13 -0400
Subject: [PATCH 8/8] Fix TOML MCP name/conflict bugs, config data-loss, and
 skill exists check (#8)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three related correctness bugs found while auditing the asset handlers:

1. src/assets/mcp.ts: when dispatching to the TOML config path (used by
   Codex/Vibe), the handler passed the original `job` object instead of
   the locally-reassigned `assetName`. mcpHandler.handle() unwraps the
   {"mcpServers":{"name":{...}}} convenience format and reassigns its
   local `assetName` variable, but handleTomlMcp re-derived assetName
   from `job.assetName`, which is the *pre-unwrap* name (typically
   derived from the source filename). Installing a wrapped-format MCP
   source for a TOML host silently wrote the entry under the wrong key.
   Existing scenario tests didn't catch this because their fixtures
   aren't in wrapped format, so the reassignment path is never
   exercised.

2. Both TOML branches (Vibe's [[mcp_servers]] array and the default
   [mcp_servers.<name>] table format) treated any existing same-name
   entry as `status: 'exists'` without comparing content, unlike the
   JSON write path. Re-running an install with updated MCP config for a
   TOML host silently dropped the update instead of reporting a
   conflict.

3. src/utils/fs.ts: readJSONOrNull() caught *all* errors — including
   JSON.parse failures on an existing file — and returned null the same
   as a missing file. src/assets/mcp.ts defaults a null result to `{}`,
   so an existing-but-malformed config file was silently treated as
   empty and then overwritten via atomicWriteJSON, destroying every
   other entry it contained. Now only ENOENT (file not found) returns
   null; other errors propagate, and mcpHandler turns that into an
   'error' result instead of writing over the file.

4. src/assets/skill.ts: the "already installed" check only tested
   whether the target directory exists, not whether SKILL.md is present
   inside it. A directory left behind by an interrupted install (e.g.
   process killed mid-copyDirRecursive) would be permanently reported as
   'exists' on every future run, with no way to repair it short of
   manually deleting the directory. Now checks for the entry file
   itself, so an incomplete install is retried.
---
 src/assets/mcp.ts                          |  48 ++++--
 src/assets/skill.ts                        |   8 +-
 src/utils/fs.ts                            |  18 ++-
 tests/unit/assets/mcp.test.ts              | 179 +++++++++++++++++++++
 tests/unit/assets/skill.test.ts            |  82 ++++++++++
 tests/unit/utils/read-json-or-null.test.ts |  35 ++++
 6 files changed, 354 insertions(+), 16 deletions(-)
 create mode 100644 tests/unit/assets/mcp.test.ts
 create mode 100644 tests/unit/assets/skill.test.ts
 create mode 100644 tests/unit/utils/read-json-or-null.test.ts

diff --git a/src/assets/mcp.ts b/src/assets/mcp.ts
index 8473e2c..52b8826 100644
--- a/src/assets/mcp.ts
+++ b/src/assets/mcp.ts
@@ -37,10 +37,11 @@ function resolveConfigFilePath(
 async function handleTomlMcp(
   job: InstallJob,
   configFilePath: string,
+  assetName: string,
   newServerConfig: Record<string, unknown>,
 ): Promise<InstallResult> {
   const { parse, stringify } = await import('smol-toml');
-  const { host, assetName } = job;
+  const { host } = job;
 
   await ensureDir(path.dirname(configFilePath));
 
@@ -65,16 +66,25 @@ async function handleTomlMcp(
   if (host.id === 'vibe') {
     // Vibe uses [[mcp_servers]] array format
     const mcpServers = (parsed['mcp_servers'] as Array<Record<string, unknown>>) ?? [];
-    const existing = mcpServers.find((s) => s['name'] === assetName);
-    if (existing) {
-      return { job, status: 'exists', targetPath: configFilePath };
-    }
     const entry: Record<string, unknown> = {
       name: assetName,
       transport: 'stdio',
       command: (newServerConfig['command'] as string) ?? '',
     };
     if (newServerConfig['args']) entry['args'] = newServerConfig['args'];
+
+    const existingIdx = mcpServers.findIndex((s) => s['name'] === assetName);
+    if (existingIdx !== -1) {
+      if (JSON.stringify(mcpServers[existingIdx]) === JSON.stringify(entry)) {
+        return { job, status: 'exists', targetPath: configFilePath };
+      }
+      return {
+        job,
+        status: 'conflict',
+        targetPath: configFilePath,
+        reason: `${configFilePath} already has an mcp_servers entry named '${assetName}' with different content`,
+      };
+    }
     mcpServers.push(entry);
     parsed['mcp_servers'] = mcpServers;
   } else {
@@ -83,14 +93,23 @@ async function handleTomlMcp(
       parsed['mcp_servers'] = {};
     }
     const mcpServers = parsed['mcp_servers'] as Record<string, unknown>;
-    if (assetName in mcpServers) {
-      return { job, status: 'exists', targetPath: configFilePath };
-    }
     const entry: Record<string, unknown> = {
       command: (newServerConfig['command'] as string) ?? '',
     };
     if (newServerConfig['args']) entry['args'] = newServerConfig['args'];
     if (newServerConfig['env']) entry['env'] = newServerConfig['env'];
+
+    if (assetName in mcpServers) {
+      if (JSON.stringify(mcpServers[assetName]) === JSON.stringify(entry)) {
+        return { job, status: 'exists', targetPath: configFilePath };
+      }
+      return {
+        job,
+        status: 'conflict',
+        targetPath: configFilePath,
+        reason: `${configFilePath} already has a '${assetName}' key with different content`,
+      };
+    }
     mcpServers[assetName] = entry;
   }
 
@@ -139,12 +158,21 @@ export const mcpHandler: AssetHandler = {
 
     // Dispatch to TOML handler when config file has .toml extension
     if (configFilePath.endsWith('.toml')) {
-      return handleTomlMcp(job, configFilePath, newServerConfig);
+      return handleTomlMcp(job, configFilePath, assetName, newServerConfig);
     }
 
     const configKey = mcpCapability.configKey ?? 'mcpServers';
 
-    const existingConfig = (await readJSONOrNull<Record<string, unknown>>(configFilePath)) ?? {};
+    let existingConfig: Record<string, unknown>;
+    try {
+      existingConfig = (await readJSONOrNull<Record<string, unknown>>(configFilePath)) ?? {};
+    } catch (err) {
+      return {
+        job,
+        status: 'error',
+        reason: `Failed to parse existing config file as JSON: ${configFilePath}\n  Cause: ${(err as Error).message}`,
+      };
+    }
     const mcpServers = (existingConfig[configKey] as Record<string, unknown>) ?? {};
 
     if (assetName in mcpServers) {
diff --git a/src/assets/skill.ts b/src/assets/skill.ts
index 205663b..d079404 100644
--- a/src/assets/skill.ts
+++ b/src/assets/skill.ts
@@ -26,10 +26,14 @@ export const skillHandler: AssetHandler = {
     const entryFile = path.join(targetDir, 'SKILL.md');
 
     try {
-      await fs.promises.access(targetDir);
+      // Check the entry file, not just the directory: a directory that
+      // exists without SKILL.md means a previous install was interrupted
+      // partway through copyDirRecursive, and should be retried rather
+      // than permanently reported as already installed.
+      await fs.promises.access(entryFile);
       return { job, status: 'exists', targetPath: entryFile };
     } catch {
-      // target doesn't exist, proceed with install
+      // target doesn't exist or is an incomplete install, proceed with install
     }
 
     await copyDirRecursive(resolvedSource.localPath, targetDir);
diff --git a/src/utils/fs.ts b/src/utils/fs.ts
index a021221..a9e0b21 100644
--- a/src/utils/fs.ts
+++ b/src/utils/fs.ts
@@ -5,13 +5,23 @@ export async function ensureDir(dirPath: string): Promise<void> {
   await fs.promises.mkdir(dirPath, { recursive: true });
 }
 
+/**
+ * Read and parse a JSON file. Returns null only if the file doesn't exist —
+ * any other error (permission denied, invalid JSON) is thrown, so callers
+ * that default a missing file to `{}` don't also silently discard an
+ * existing-but-malformed config and overwrite it.
+ */
 export async function readJSONOrNull<T = unknown>(filePath: string): Promise<T | null> {
+  let content: string;
   try {
-    const content = await fs.promises.readFile(filePath, 'utf-8');
-    return JSON.parse(content) as T;
-  } catch {
-    return null;
+    content = await fs.promises.readFile(filePath, 'utf-8');
+  } catch (err) {
+    if ((err as NodeJS.ErrnoException).code === 'ENOENT') {
+      return null;
+    }
+    throw err;
   }
+  return JSON.parse(content) as T;
 }
 
 export async function atomicWriteJSON(filePath: string, data: unknown): Promise<void> {
diff --git a/tests/unit/assets/mcp.test.ts b/tests/unit/assets/mcp.test.ts
new file mode 100644
index 0000000..7dab639
--- /dev/null
+++ b/tests/unit/assets/mcp.test.ts
@@ -0,0 +1,179 @@
+import { describe, it, expect } from 'vitest';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { mcpHandler } from '../../../src/assets/mcp.js';
+import type { InstallJob } from '../../../src/assets/types.js';
+import type { HostConfig } from '../../../src/hosts/types.js';
+
+async function makeTmpDir(): Promise<string> {
+  return fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-mcp-test-'));
+}
+
+function makeHost(configFilePath: string, hostId = 'codex'): HostConfig {
+  return {
+    id: hostId,
+    displayName: hostId,
+    docs: '',
+    detection: { paths: [] },
+    assets: {
+      mcp: {
+        supported: true,
+        configFile: configFilePath,
+        configKey: 'mcpServers',
+        writeStrategy: 'inject-json-key',
+      },
+      skill: { supported: false },
+      prompt: { supported: false },
+      command: { supported: false },
+      subAgent: { supported: false },
+    },
+  } as HostConfig;
+}
+
+async function cleanup(tmpDir: string): Promise<void> {
+  await fs.promises.rm(tmpDir, { recursive: true, force: true });
+}
+
+describe('mcpHandler - TOML hosts (regression)', () => {
+  it('writes the unwrapped mcpServers name, not the stale pre-unwrap assetName', async () => {
+    const tmpDir = await makeTmpDir();
+    const sourcePath = path.join(tmpDir, 'wrapped-source.json');
+    await fs.promises.writeFile(
+      sourcePath,
+      JSON.stringify({ mcpServers: { playwright: { command: 'npx', args: ['-y', '@playwright/mcp'] } } }),
+    );
+    const configFilePath = path.join(tmpDir, 'config.toml');
+    const host = makeHost(configFilePath);
+
+    const job: InstallJob = {
+      assetType: 'mcp',
+      assetName: 'wrapped-source', // the (wrong) name inferred from the source filename
+      resolvedSource: { type: 'local', localPath: sourcePath, originalSource: sourcePath },
+      host,
+    };
+
+    const result = await mcpHandler.handle(job);
+    expect(result.status).toBe('written');
+
+    const toml = await fs.promises.readFile(configFilePath, 'utf-8');
+    expect(toml).toContain('playwright');
+    expect(toml).not.toContain('wrapped-source');
+
+    await cleanup(tmpDir);
+  });
+
+  it('reports conflict (not exists) when a differently-configured entry already exists at the same name', async () => {
+    const tmpDir = await makeTmpDir();
+    const sourcePath = path.join(tmpDir, 'playwright.json');
+    await fs.promises.writeFile(sourcePath, JSON.stringify({ command: 'npx', args: ['-y', '@playwright/mcp'] }));
+    const configFilePath = path.join(tmpDir, 'config.toml');
+    await fs.promises.writeFile(configFilePath, '[mcp_servers.playwright]\ncommand = "different-command"\n');
+    const host = makeHost(configFilePath);
+
+    const job: InstallJob = {
+      assetType: 'mcp',
+      assetName: 'playwright',
+      resolvedSource: { type: 'local', localPath: sourcePath, originalSource: sourcePath },
+      host,
+    };
+
+    const result = await mcpHandler.handle(job);
+    expect(result.status).toBe('conflict');
+
+    await cleanup(tmpDir);
+  });
+
+  it('reports exists (not conflict) once the identical entry has already been written', async () => {
+    const tmpDir = await makeTmpDir();
+    const sourcePath = path.join(tmpDir, 'playwright.json');
+    await fs.promises.writeFile(sourcePath, JSON.stringify({ command: 'npx' }));
+    const configFilePath = path.join(tmpDir, 'config.toml');
+    const host = makeHost(configFilePath);
+    const job: InstallJob = {
+      assetType: 'mcp',
+      assetName: 'playwright',
+      resolvedSource: { type: 'local', localPath: sourcePath, originalSource: sourcePath },
+      host,
+    };
+
+    const first = await mcpHandler.handle(job);
+    expect(first.status).toBe('written');
+
+    const second = await mcpHandler.handle(job);
+    expect(second.status).toBe('exists');
+
+    await cleanup(tmpDir);
+  });
+
+  it('vibe array format: reports conflict for a same-name entry with different content', async () => {
+    const tmpDir = await makeTmpDir();
+    const sourcePath = path.join(tmpDir, 'playwright.json');
+    await fs.promises.writeFile(sourcePath, JSON.stringify({ command: 'npx' }));
+    const configFilePath = path.join(tmpDir, 'config.toml');
+    await fs.promises.writeFile(
+      configFilePath,
+      '[[mcp_servers]]\nname = "playwright"\ntransport = "stdio"\ncommand = "different"\n',
+    );
+    const host = makeHost(configFilePath, 'vibe');
+    const job: InstallJob = {
+      assetType: 'mcp',
+      assetName: 'playwright',
+      resolvedSource: { type: 'local', localPath: sourcePath, originalSource: sourcePath },
+      host,
+    };
+
+    const result = await mcpHandler.handle(job);
+    expect(result.status).toBe('conflict');
+
+    await cleanup(tmpDir);
+  });
+
+  it('vibe array format: reports exists once the identical entry has already been written', async () => {
+    const tmpDir = await makeTmpDir();
+    const sourcePath = path.join(tmpDir, 'playwright.json');
+    await fs.promises.writeFile(sourcePath, JSON.stringify({ command: 'npx' }));
+    const configFilePath = path.join(tmpDir, 'config.toml');
+    const host = makeHost(configFilePath, 'vibe');
+    const job: InstallJob = {
+      assetType: 'mcp',
+      assetName: 'playwright',
+      resolvedSource: { type: 'local', localPath: sourcePath, originalSource: sourcePath },
+      host,
+    };
+
+    const first = await mcpHandler.handle(job);
+    expect(first.status).toBe('written');
+    const second = await mcpHandler.handle(job);
+    expect(second.status).toBe('exists');
+
+    await cleanup(tmpDir);
+  });
+});
+
+describe('mcpHandler - JSON hosts (regression)', () => {
+  it('reports error instead of silently discarding a malformed existing config', async () => {
+    const tmpDir = await makeTmpDir();
+    const sourcePath = path.join(tmpDir, 'playwright.json');
+    await fs.promises.writeFile(sourcePath, JSON.stringify({ command: 'npx' }));
+    const configFilePath = path.join(tmpDir, 'mcp.json');
+    // malformed JSON (trailing comma) — this file has real, pre-existing content
+    await fs.promises.writeFile(configFilePath, '{"mcpServers":{"existing-tool":{"command":"echo"},}}');
+    const host = makeHost(configFilePath);
+    const job: InstallJob = {
+      assetType: 'mcp',
+      assetName: 'playwright',
+      resolvedSource: { type: 'local', localPath: sourcePath, originalSource: sourcePath },
+      host,
+    };
+
+    const result = await mcpHandler.handle(job);
+    expect(result.status).toBe('error');
+
+    // The malformed file must be left untouched, not silently overwritten
+    const stillThere = await fs.promises.readFile(configFilePath, 'utf-8');
+    expect(stillThere).toContain('existing-tool');
+
+    await cleanup(tmpDir);
+  });
+});
diff --git a/tests/unit/assets/skill.test.ts b/tests/unit/assets/skill.test.ts
new file mode 100644
index 0000000..31e5d87
--- /dev/null
+++ b/tests/unit/assets/skill.test.ts
@@ -0,0 +1,82 @@
+import { describe, it, expect } from 'vitest';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { skillHandler } from '../../../src/assets/skill.js';
+import type { InstallJob } from '../../../src/assets/types.js';
+import type { HostConfig } from '../../../src/hosts/types.js';
+
+function makeHost(installDir: string): HostConfig {
+  return {
+    id: 'test-host',
+    displayName: 'Test Host',
+    docs: '',
+    detection: { paths: [] },
+    assets: {
+      mcp: { supported: false },
+      skill: { supported: true, installDir, entryFile: 'SKILL.md', writeStrategy: 'copy-file' },
+      prompt: { supported: false },
+      command: { supported: false },
+      subAgent: { supported: false },
+    },
+  } as HostConfig;
+}
+
+describe('skillHandler (regression)', () => {
+  it('retries an install left incomplete by a previous interrupted run', async () => {
+    const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-skill-test-'));
+    const sourceDir = path.join(tmpDir, 'source-skill');
+    await fs.promises.mkdir(sourceDir, { recursive: true });
+    await fs.promises.writeFile(path.join(sourceDir, 'SKILL.md'), '# My Skill\n');
+    await fs.promises.writeFile(path.join(sourceDir, 'extra.txt'), 'extra content');
+
+    const installRoot = path.join(tmpDir, 'installed');
+    // Simulate a previous install interrupted mid-copy: the target directory
+    // exists, but SKILL.md was never written into it.
+    const targetDir = path.join(installRoot, 'my-skill');
+    await fs.promises.mkdir(targetDir, { recursive: true });
+
+    const host = makeHost(installRoot);
+    const job: InstallJob = {
+      assetType: 'skill',
+      assetName: 'my-skill',
+      resolvedSource: { type: 'local', localPath: sourceDir, originalSource: sourceDir },
+      host,
+    };
+
+    const result = await skillHandler.handle(job);
+    expect(result.status).toBe('written');
+
+    const installedSkillMd = await fs.promises.readFile(path.join(targetDir, 'SKILL.md'), 'utf-8');
+    expect(installedSkillMd).toBe('# My Skill\n');
+
+    await fs.promises.rm(tmpDir, { recursive: true, force: true });
+  });
+
+  it('reports exists when SKILL.md is already present in the target directory', async () => {
+    const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-skill-test-'));
+    const sourceDir = path.join(tmpDir, 'source-skill');
+    await fs.promises.mkdir(sourceDir, { recursive: true });
+    await fs.promises.writeFile(path.join(sourceDir, 'SKILL.md'), '# My Skill\n');
+
+    const installRoot = path.join(tmpDir, 'installed');
+    const targetDir = path.join(installRoot, 'my-skill');
+    await fs.promises.mkdir(targetDir, { recursive: true });
+    await fs.promises.writeFile(path.join(targetDir, 'SKILL.md'), '# Already installed\n');
+
+    const host = makeHost(installRoot);
+    const job: InstallJob = {
+      assetType: 'skill',
+      assetName: 'my-skill',
+      resolvedSource: { type: 'local', localPath: sourceDir, originalSource: sourceDir },
+      host,
+    };
+
+    const result = await skillHandler.handle(job);
+    expect(result.status).toBe('exists');
+    const content = await fs.promises.readFile(path.join(targetDir, 'SKILL.md'), 'utf-8');
+    expect(content).toBe('# Already installed\n');
+
+    await fs.promises.rm(tmpDir, { recursive: true, force: true });
+  });
+});
diff --git a/tests/unit/utils/read-json-or-null.test.ts b/tests/unit/utils/read-json-or-null.test.ts
new file mode 100644
index 0000000..a96cbca
--- /dev/null
+++ b/tests/unit/utils/read-json-or-null.test.ts
@@ -0,0 +1,35 @@
+import { describe, it, expect } from 'vitest';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { readJSONOrNull } from '../../../src/utils/fs.js';
+
+describe('readJSONOrNull', () => {
+  it('returns null when the file does not exist', async () => {
+    const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-test-'));
+    const result = await readJSONOrNull(path.join(tmpDir, 'missing.json'));
+    expect(result).toBeNull();
+    await fs.promises.rm(tmpDir, { recursive: true, force: true });
+  });
+
+  it('returns the parsed object for valid JSON', async () => {
+    const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-test-'));
+    const filePath = path.join(tmpDir, 'valid.json');
+    await fs.promises.writeFile(filePath, JSON.stringify({ key: 'value' }));
+    const result = await readJSONOrNull(filePath);
+    expect(result).toEqual({ key: 'value' });
+    await fs.promises.rm(tmpDir, { recursive: true, force: true });
+  });
+
+  it('throws (does not silently return null) for a file with malformed JSON (regression)', async () => {
+    // Regression: previously any error (including a JSON.parse failure on an
+    // existing-but-malformed file) was swallowed and treated the same as a
+    // missing file, which let callers default to {} and silently overwrite/
+    // destroy the malformed file's actual content.
+    const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'agent-add-test-'));
+    const filePath = path.join(tmpDir, 'malformed.json');
+    await fs.promises.writeFile(filePath, '{"mcpServers":{"existing-tool":{"command":"echo"},}}');
+    await expect(readJSONOrNull(filePath)).rejects.toThrow();
+    await fs.promises.rm(tmpDir, { recursive: true, force: true });
+  });
+});