feat: change plugins input from comma-separated to newline-separated (#644)

* feat: change plugins input from comma-separated to newline-separated

Changes:
- Update parsePlugins() to split by newline instead of comma for consistency with marketplaces input
- Update action.yml and base-action/action.yml with newline-separated format and realistic plugin examples
- Add plugin_marketplaces documentation to docs/usage.md
- Update all unit tests to match new installPlugins() signature (marketplaces, plugins, executable)
- Improve JSDoc comments for parsePlugins() and installPlugin() functions
- All 25 install-plugins tests passing

Breaking change: Users must update their workflows to use newline-separated format:
  Before: plugins: "plugin1,plugin2"
  After: plugins: "plugin1\nplugin2"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* test: add comprehensive marketplace functionality tests

Critical fix: All previous tests passed undefined as marketplacesInput parameter,
leaving the entire marketplace functionality completely untested.

Added 13 new tests covering:
- Single marketplace installation
- Multiple marketplaces with newline separation
- Marketplace + plugin installation order verification
- Marketplace URL validation (format, protocol, .git extension)
- Whitespace and empty entry handling
- Error handling for marketplace operations
- Custom executable path for marketplace operations

Test coverage: 38 tests (was 25), 81 expect calls (was 50)
All tests passing 

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Wanghong Yuan
2025-10-27 09:01:34 -07:00
committed by GitHub
parent 8ad13bd20b
commit 29fe50368c
6 changed files with 319 additions and 61 deletions

View File

@@ -39,25 +39,25 @@ describe("installPlugins", () => {
test("should not call spawn when no plugins are specified", async () => {
const spy = createMockSpawn();
await installPlugins("");
await installPlugins(undefined, "");
expect(spy).not.toHaveBeenCalled();
});
test("should not call spawn when plugins is undefined", async () => {
const spy = createMockSpawn();
await installPlugins(undefined);
await installPlugins(undefined, undefined);
expect(spy).not.toHaveBeenCalled();
});
test("should not call spawn when plugins is only whitespace", async () => {
const spy = createMockSpawn();
await installPlugins(" ");
await installPlugins(undefined, " ");
expect(spy).not.toHaveBeenCalled();
});
test("should install a single plugin with default executable", async () => {
const spy = createMockSpawn();
await installPlugins("test-plugin");
await installPlugins(undefined, "test-plugin");
expect(spy).toHaveBeenCalledTimes(1);
// Only call: install plugin (no marketplace without explicit marketplace input)
@@ -71,7 +71,7 @@ describe("installPlugins", () => {
test("should install multiple plugins sequentially", async () => {
const spy = createMockSpawn();
await installPlugins("plugin1,plugin2,plugin3");
await installPlugins(undefined, "plugin1\nplugin2\nplugin3");
expect(spy).toHaveBeenCalledTimes(3);
// Install plugins (no marketplace without explicit marketplace input)
@@ -97,7 +97,7 @@ describe("installPlugins", () => {
test("should use custom claude executable path when provided", async () => {
const spy = createMockSpawn();
await installPlugins("test-plugin", "/custom/path/to/claude");
await installPlugins(undefined, "test-plugin", "/custom/path/to/claude");
expect(spy).toHaveBeenCalledTimes(1);
// Only call: install plugin (no marketplace without explicit marketplace input)
@@ -111,7 +111,7 @@ describe("installPlugins", () => {
test("should trim whitespace from plugin names before installation", async () => {
const spy = createMockSpawn();
await installPlugins(" plugin1 , plugin2 ");
await installPlugins(undefined, " plugin1 \n plugin2 ");
expect(spy).toHaveBeenCalledTimes(2);
// Install plugins (no marketplace without explicit marketplace input)
@@ -131,7 +131,7 @@ describe("installPlugins", () => {
test("should skip empty entries in plugin list", async () => {
const spy = createMockSpawn();
await installPlugins("plugin1,,plugin2");
await installPlugins(undefined, "plugin1\n\nplugin2");
expect(spy).toHaveBeenCalledTimes(2);
// Install plugins (no marketplace without explicit marketplace input)
@@ -152,7 +152,7 @@ describe("installPlugins", () => {
test("should handle plugin installation error and throw", async () => {
createMockSpawn(1, false); // Exit code 1
await expect(installPlugins("failing-plugin")).rejects.toThrow(
await expect(installPlugins(undefined, "failing-plugin")).rejects.toThrow(
"Failed to install plugin 'failing-plugin' (exit code: 1)",
);
});
@@ -160,7 +160,9 @@ describe("installPlugins", () => {
test("should handle null exit code (process terminated by signal)", async () => {
createMockSpawn(null, false); // Exit code null (terminated by signal)
await expect(installPlugins("terminated-plugin")).rejects.toThrow(
await expect(
installPlugins(undefined, "terminated-plugin"),
).rejects.toThrow(
"Failed to install plugin 'terminated-plugin': process terminated by signal",
);
});
@@ -168,9 +170,9 @@ describe("installPlugins", () => {
test("should stop installation on first error", async () => {
const spy = createMockSpawn(1, false); // Exit code 1
await expect(installPlugins("plugin1,plugin2,plugin3")).rejects.toThrow(
"Failed to install plugin 'plugin1' (exit code: 1)",
);
await expect(
installPlugins(undefined, "plugin1\nplugin2\nplugin3"),
).rejects.toThrow("Failed to install plugin 'plugin1' (exit code: 1)");
// Should only try to install first plugin before failing
expect(spy).toHaveBeenCalledTimes(1);
@@ -178,7 +180,7 @@ describe("installPlugins", () => {
test("should handle plugins with special characters in names", async () => {
const spy = createMockSpawn();
await installPlugins("org/plugin-name,@scope/plugin");
await installPlugins(undefined, "org/plugin-name\n@scope/plugin");
expect(spy).toHaveBeenCalledTimes(2);
// Install plugins (no marketplace without explicit marketplace input)
@@ -199,14 +201,18 @@ describe("installPlugins", () => {
test("should handle spawn errors", async () => {
createMockSpawn(0, true); // Trigger error event
await expect(installPlugins("test-plugin")).rejects.toThrow(
await expect(installPlugins(undefined, "test-plugin")).rejects.toThrow(
"Failed to install plugin 'test-plugin': spawn error",
);
});
test("should install plugins with custom executable and multiple plugins", async () => {
const spy = createMockSpawn();
await installPlugins("plugin-a,plugin-b", "/usr/local/bin/claude-custom");
await installPlugins(
undefined,
"plugin-a\nplugin-b",
"/usr/local/bin/claude-custom",
);
expect(spy).toHaveBeenCalledTimes(2);
// Install plugins (no marketplace without explicit marketplace input)
@@ -228,9 +234,9 @@ describe("installPlugins", () => {
const spy = createMockSpawn();
// Should throw due to invalid characters (semicolon and spaces)
await expect(installPlugins("plugin-name; rm -rf /")).rejects.toThrow(
"Invalid plugin name format",
);
await expect(
installPlugins(undefined, "plugin-name; rm -rf /"),
).rejects.toThrow("Invalid plugin name format");
// Mock should never be called because validation fails first
expect(spy).not.toHaveBeenCalled();
@@ -239,9 +245,9 @@ describe("installPlugins", () => {
test("should reject plugin names with path traversal using ../", async () => {
const spy = createMockSpawn();
await expect(installPlugins("../../../malicious-plugin")).rejects.toThrow(
"Invalid plugin name format",
);
await expect(
installPlugins(undefined, "../../../malicious-plugin"),
).rejects.toThrow("Invalid plugin name format");
expect(spy).not.toHaveBeenCalled();
});
@@ -249,9 +255,9 @@ describe("installPlugins", () => {
test("should reject plugin names with path traversal using ./", async () => {
const spy = createMockSpawn();
await expect(installPlugins("./../../@scope/package")).rejects.toThrow(
"Invalid plugin name format",
);
await expect(
installPlugins(undefined, "./../../@scope/package"),
).rejects.toThrow("Invalid plugin name format");
expect(spy).not.toHaveBeenCalled();
});
@@ -259,7 +265,7 @@ describe("installPlugins", () => {
test("should reject plugin names with consecutive dots", async () => {
const spy = createMockSpawn();
await expect(installPlugins(".../.../package")).rejects.toThrow(
await expect(installPlugins(undefined, ".../.../package")).rejects.toThrow(
"Invalid plugin name format",
);
@@ -269,7 +275,7 @@ describe("installPlugins", () => {
test("should reject plugin names with hidden path traversal", async () => {
const spy = createMockSpawn();
await expect(installPlugins("package/../other")).rejects.toThrow(
await expect(installPlugins(undefined, "package/../other")).rejects.toThrow(
"Invalid plugin name format",
);
@@ -278,7 +284,7 @@ describe("installPlugins", () => {
test("should accept plugin names with single dots in version numbers", async () => {
const spy = createMockSpawn();
await installPlugins("plugin-v1.0.2");
await installPlugins(undefined, "plugin-v1.0.2");
expect(spy).toHaveBeenCalledTimes(1);
// Only call: install plugin (no marketplace without explicit marketplace input)
@@ -292,7 +298,7 @@ describe("installPlugins", () => {
test("should accept plugin names with multiple dots in semantic versions", async () => {
const spy = createMockSpawn();
await installPlugins("@scope/plugin-v1.0.0-beta.1");
await installPlugins(undefined, "@scope/plugin-v1.0.0-beta.1");
expect(spy).toHaveBeenCalledTimes(1);
// Only call: install plugin (no marketplace without explicit marketplace input)
@@ -308,7 +314,7 @@ describe("installPlugins", () => {
const spy = createMockSpawn();
// Using fullwidth dots (U+FF0E) and fullwidth solidus (U+FF0F)
await expect(installPlugins("malicious")).rejects.toThrow(
await expect(installPlugins(undefined, "malicious")).rejects.toThrow(
"Invalid plugin name format",
);
@@ -318,7 +324,7 @@ describe("installPlugins", () => {
test("should reject path traversal at end of path", async () => {
const spy = createMockSpawn();
await expect(installPlugins("package/..")).rejects.toThrow(
await expect(installPlugins(undefined, "package/..")).rejects.toThrow(
"Invalid plugin name format",
);
@@ -328,7 +334,7 @@ describe("installPlugins", () => {
test("should reject single dot directory reference", async () => {
const spy = createMockSpawn();
await expect(installPlugins("package/.")).rejects.toThrow(
await expect(installPlugins(undefined, "package/.")).rejects.toThrow(
"Invalid plugin name format",
);
@@ -338,10 +344,256 @@ describe("installPlugins", () => {
test("should reject path traversal in middle of path", async () => {
const spy = createMockSpawn();
await expect(installPlugins("package/../other")).rejects.toThrow(
await expect(installPlugins(undefined, "package/../other")).rejects.toThrow(
"Invalid plugin name format",
);
expect(spy).not.toHaveBeenCalled();
});
// Marketplace functionality tests
test("should add a single marketplace before installing plugins", async () => {
const spy = createMockSpawn();
await installPlugins(
"https://github.com/user/marketplace.git",
"test-plugin",
);
expect(spy).toHaveBeenCalledTimes(2);
// First call: add marketplace
expect(spy).toHaveBeenNthCalledWith(
1,
"claude",
[
"plugin",
"marketplace",
"add",
"https://github.com/user/marketplace.git",
],
{ stdio: "inherit" },
);
// Second call: install plugin
expect(spy).toHaveBeenNthCalledWith(
2,
"claude",
["plugin", "install", "test-plugin"],
{ stdio: "inherit" },
);
});
test("should add multiple marketplaces with newline separation", async () => {
const spy = createMockSpawn();
await installPlugins(
"https://github.com/user/m1.git\nhttps://github.com/user/m2.git",
"test-plugin",
);
expect(spy).toHaveBeenCalledTimes(3); // 2 marketplaces + 1 plugin
// First two calls: add marketplaces
expect(spy).toHaveBeenNthCalledWith(
1,
"claude",
["plugin", "marketplace", "add", "https://github.com/user/m1.git"],
{ stdio: "inherit" },
);
expect(spy).toHaveBeenNthCalledWith(
2,
"claude",
["plugin", "marketplace", "add", "https://github.com/user/m2.git"],
{ stdio: "inherit" },
);
// Third call: install plugin
expect(spy).toHaveBeenNthCalledWith(
3,
"claude",
["plugin", "install", "test-plugin"],
{ stdio: "inherit" },
);
});
test("should add marketplaces before installing multiple plugins", async () => {
const spy = createMockSpawn();
await installPlugins(
"https://github.com/user/marketplace.git",
"plugin1\nplugin2",
);
expect(spy).toHaveBeenCalledTimes(3); // 1 marketplace + 2 plugins
// First call: add marketplace
expect(spy).toHaveBeenNthCalledWith(
1,
"claude",
[
"plugin",
"marketplace",
"add",
"https://github.com/user/marketplace.git",
],
{ stdio: "inherit" },
);
// Next calls: install plugins
expect(spy).toHaveBeenNthCalledWith(
2,
"claude",
["plugin", "install", "plugin1"],
{ stdio: "inherit" },
);
expect(spy).toHaveBeenNthCalledWith(
3,
"claude",
["plugin", "install", "plugin2"],
{ stdio: "inherit" },
);
});
test("should handle only marketplaces without plugins", async () => {
const spy = createMockSpawn();
await installPlugins("https://github.com/user/marketplace.git", undefined);
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenNthCalledWith(
1,
"claude",
[
"plugin",
"marketplace",
"add",
"https://github.com/user/marketplace.git",
],
{ stdio: "inherit" },
);
});
test("should skip empty marketplace entries", async () => {
const spy = createMockSpawn();
await installPlugins(
"https://github.com/user/m1.git\n\nhttps://github.com/user/m2.git",
"test-plugin",
);
expect(spy).toHaveBeenCalledTimes(3); // 2 marketplaces (skip empty) + 1 plugin
});
test("should trim whitespace from marketplace URLs", async () => {
const spy = createMockSpawn();
await installPlugins(
" https://github.com/user/marketplace.git \n https://github.com/user/m2.git ",
"test-plugin",
);
expect(spy).toHaveBeenCalledTimes(3);
expect(spy).toHaveBeenNthCalledWith(
1,
"claude",
[
"plugin",
"marketplace",
"add",
"https://github.com/user/marketplace.git",
],
{ stdio: "inherit" },
);
expect(spy).toHaveBeenNthCalledWith(
2,
"claude",
["plugin", "marketplace", "add", "https://github.com/user/m2.git"],
{ stdio: "inherit" },
);
});
test("should reject invalid marketplace URL format", async () => {
const spy = createMockSpawn();
await expect(
installPlugins("not-a-valid-url", "test-plugin"),
).rejects.toThrow("Invalid marketplace URL format");
expect(spy).not.toHaveBeenCalled();
});
test("should reject marketplace URL without .git extension", async () => {
const spy = createMockSpawn();
await expect(
installPlugins("https://github.com/user/marketplace", "test-plugin"),
).rejects.toThrow("Invalid marketplace URL format");
expect(spy).not.toHaveBeenCalled();
});
test("should reject marketplace URL with non-https protocol", async () => {
const spy = createMockSpawn();
await expect(
installPlugins("http://github.com/user/marketplace.git", "test-plugin"),
).rejects.toThrow("Invalid marketplace URL format");
expect(spy).not.toHaveBeenCalled();
});
test("should skip whitespace-only marketplace input", async () => {
const spy = createMockSpawn();
await installPlugins(" ", "test-plugin");
// Should skip marketplaces and only install plugin
expect(spy).toHaveBeenCalledTimes(1);
expect(spy).toHaveBeenNthCalledWith(
1,
"claude",
["plugin", "install", "test-plugin"],
{ stdio: "inherit" },
);
});
test("should handle marketplace addition error", async () => {
createMockSpawn(1, false); // Exit code 1
await expect(
installPlugins("https://github.com/user/marketplace.git", "test-plugin"),
).rejects.toThrow(
"Failed to add marketplace 'https://github.com/user/marketplace.git' (exit code: 1)",
);
});
test("should stop if marketplace addition fails before installing plugins", async () => {
const spy = createMockSpawn(1, false); // Exit code 1
await expect(
installPlugins(
"https://github.com/user/marketplace.git",
"plugin1\nplugin2",
),
).rejects.toThrow("Failed to add marketplace");
// Should only try to add marketplace, not install any plugins
expect(spy).toHaveBeenCalledTimes(1);
});
test("should use custom executable for marketplace operations", async () => {
const spy = createMockSpawn();
await installPlugins(
"https://github.com/user/marketplace.git",
"test-plugin",
"/custom/path/to/claude",
);
expect(spy).toHaveBeenCalledTimes(2);
expect(spy).toHaveBeenNthCalledWith(
1,
"/custom/path/to/claude",
[
"plugin",
"marketplace",
"add",
"https://github.com/user/marketplace.git",
],
{ stdio: "inherit" },
);
expect(spy).toHaveBeenNthCalledWith(
2,
"/custom/path/to/claude",
["plugin", "install", "test-plugin"],
{ stdio: "inherit" },
);
});
});