mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-11-10 17:10:00 +08:00
fix: use paths in delete_files prompt example (#1702)
* fix: use paths param in delete_files prompt example The tag-mode prompt told the model to call delete_files with "files", but the MCP tool schema and handler expect "paths". Co-authored-by: Cursor <[email protected]> * test: prove delete_files prompt against the live MCP schema The old {files} payload is rejected by the same Zod shape the tool registers; the generated prompt example now parses cleanly. Co-authored-by: Cursor <[email protected]> * test: cover delete_files schema edges and the sibling commit_files tool Confirm the old files-only payload still fails, types and required fields are enforced, and commit_files was not inverted by the fix. Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: RESILIENCE Agentic Solutions <[email protected]> Co-authored-by: Cursor <[email protected]>
This commit is contained in:
RESILIENCE Agentic Solutions
co-authored by
Cursor
RESILIENCE Agentic Solutions
Cursor
RESILIENCE Agentic Solutions
parent
b58c16b325
commit
2d7a787fbd
@@ -824,7 +824,7 @@ ${
|
||||
? `- Use mcp__github_file_ops__commit_files for making commits (works for both new and existing files, single or multiple). Use mcp__github_file_ops__delete_files for deleting files (supports deleting single or multiple files atomically), or mcp__github__delete_file for deleting a single file. Edit files locally, and the tool will read the content from the same path on disk.
|
||||
Tool usage examples:
|
||||
- mcp__github_file_ops__commit_files: {"files": ["path/to/file1.js", "path/to/file2.py"], "message": "feat: add new feature"}
|
||||
- mcp__github_file_ops__delete_files: {"files": ["path/to/old.js"], "message": "chore: remove deprecated file"}`
|
||||
- mcp__github_file_ops__delete_files: {"paths": ["path/to/old.js"], "message": "chore: remove deprecated file"}`
|
||||
: `- Use git commands via the Bash tool for version control (remember that you have access to these git commands):
|
||||
- Stage files: Bash(git add <files>)
|
||||
- Commit changes: Bash(git commit -m "<message>")
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
import { z } from "zod";
|
||||
|
||||
/** Raw shape passed to `server.tool` for commit_files. */
|
||||
export const commitFilesInputSchema = {
|
||||
files: z
|
||||
.array(z.string())
|
||||
.describe(
|
||||
'Array of file paths relative to repository root (e.g. ["src/main.js", "README.md"]). All files must exist locally.',
|
||||
),
|
||||
message: z.string().describe("Commit message"),
|
||||
};
|
||||
|
||||
/** Raw shape passed to `server.tool` for delete_files. */
|
||||
export const deleteFilesInputSchema = {
|
||||
paths: z
|
||||
.array(z.string())
|
||||
.describe(
|
||||
'Array of file paths to delete relative to repository root (e.g. ["src/old-file.js", "docs/deprecated.md"])',
|
||||
),
|
||||
message: z.string().describe("Commit message"),
|
||||
};
|
||||
|
||||
export const commitFilesPayloadSchema = z.object(commitFilesInputSchema);
|
||||
export const deleteFilesPayloadSchema = z.object(deleteFilesInputSchema);
|
||||
@@ -2,7 +2,6 @@
|
||||
// GitHub File Operations MCP Server
|
||||
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
|
||||
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
|
||||
import { z } from "zod";
|
||||
import { readFile, stat } from "fs/promises";
|
||||
import { resolve } from "path";
|
||||
import { constants } from "fs";
|
||||
@@ -11,6 +10,10 @@ import { GITHUB_API_URL } from "../github/api/config";
|
||||
import { isBinaryContent } from "./binary-detection";
|
||||
import { validatePathWithinRepo } from "./path-validation";
|
||||
import { updateGitReference } from "./update-git-reference";
|
||||
import {
|
||||
commitFilesInputSchema,
|
||||
deleteFilesInputSchema,
|
||||
} from "./github-file-ops-schemas";
|
||||
|
||||
type GitHubRef = {
|
||||
object: {
|
||||
@@ -197,14 +200,7 @@ async function getFileMode(filePath: string): Promise<string> {
|
||||
server.tool(
|
||||
"commit_files",
|
||||
"Commit one or more files to a repository in a single commit (this will commit them atomically in the remote repository)",
|
||||
{
|
||||
files: z
|
||||
.array(z.string())
|
||||
.describe(
|
||||
'Array of file paths relative to repository root (e.g. ["src/main.js", "README.md"]). All files must exist locally.',
|
||||
),
|
||||
message: z.string().describe("Commit message"),
|
||||
},
|
||||
commitFilesInputSchema,
|
||||
async ({ files, message }) => {
|
||||
const owner = REPO_OWNER;
|
||||
const repo = REPO_NAME;
|
||||
@@ -414,14 +410,7 @@ server.tool(
|
||||
server.tool(
|
||||
"delete_files",
|
||||
"Delete one or more files from a repository in a single commit",
|
||||
{
|
||||
paths: z
|
||||
.array(z.string())
|
||||
.describe(
|
||||
'Array of file paths to delete relative to repository root (e.g. ["src/old-file.js", "docs/deprecated.md"])',
|
||||
),
|
||||
message: z.string().describe("Commit message"),
|
||||
},
|
||||
deleteFilesInputSchema,
|
||||
async ({ paths, message }) => {
|
||||
const owner = REPO_OWNER;
|
||||
const repo = REPO_NAME;
|
||||
|
||||
@@ -819,6 +819,12 @@ describe("generatePrompt", () => {
|
||||
// Should have commit signing tool instructions
|
||||
expect(prompt).toContain("mcp__github_file_ops__commit_files");
|
||||
expect(prompt).toContain("mcp__github_file_ops__delete_files");
|
||||
expect(prompt).toContain(
|
||||
'mcp__github_file_ops__delete_files: {"paths": ["path/to/old.js"]',
|
||||
);
|
||||
expect(prompt).not.toContain(
|
||||
'mcp__github_file_ops__delete_files: {"files":',
|
||||
);
|
||||
// Comment tool should always be from comment server, not file ops
|
||||
expect(prompt).toContain("mcp__github_comment__update_claude_comment");
|
||||
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
#!/usr/bin/env bun
|
||||
|
||||
import { describe, expect, test, beforeAll } from "bun:test";
|
||||
import { generatePrompt } from "../src/create-prompt";
|
||||
import type { PreparedContext } from "../src/create-prompt";
|
||||
import {
|
||||
commitFilesPayloadSchema,
|
||||
deleteFilesPayloadSchema,
|
||||
} from "../src/mcp/github-file-ops-schemas";
|
||||
|
||||
beforeAll(() => {
|
||||
process.env.GITHUB_ACTION_PATH = "/test/action/path";
|
||||
});
|
||||
|
||||
const mockGitHubData = {
|
||||
contextData: {
|
||||
title: "Test PR",
|
||||
body: "This is a test PR",
|
||||
author: { login: "testuser" },
|
||||
state: "OPEN",
|
||||
labels: { nodes: [] },
|
||||
createdAt: "2023-01-01T00:00:00Z",
|
||||
additions: 15,
|
||||
deletions: 5,
|
||||
baseRefName: "main",
|
||||
headRefName: "feature-branch",
|
||||
headRefOid: "abc123",
|
||||
isCrossRepository: false,
|
||||
headRepository: { owner: { login: "testowner" }, name: "testrepo" },
|
||||
commits: { totalCount: 0, nodes: [] },
|
||||
files: { nodes: [] },
|
||||
comments: { nodes: [] },
|
||||
reviews: { nodes: [] },
|
||||
},
|
||||
comments: [],
|
||||
changedFiles: [],
|
||||
changedFilesWithSHA: [],
|
||||
reviewData: null,
|
||||
imageUrlMap: new Map<string, string>(),
|
||||
};
|
||||
|
||||
const signingContext: PreparedContext = {
|
||||
repository: "owner/repo",
|
||||
claudeCommentId: "12345",
|
||||
triggerPhrase: "@claude",
|
||||
eventData: {
|
||||
eventName: "issue_comment",
|
||||
commentId: "67890",
|
||||
isPR: true,
|
||||
prNumber: "123",
|
||||
commentBody: "@claude delete the old file",
|
||||
},
|
||||
};
|
||||
|
||||
function extractToolExample(
|
||||
prompt: string,
|
||||
tool: string,
|
||||
): Record<string, unknown> {
|
||||
const match = prompt.match(
|
||||
new RegExp(
|
||||
`${tool.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}:\\s*(\\{[^}]+\\})`,
|
||||
),
|
||||
);
|
||||
if (!match) {
|
||||
throw new Error(`No JSON example for ${tool} in prompt`);
|
||||
}
|
||||
return JSON.parse(match[1] as string);
|
||||
}
|
||||
|
||||
describe("delete_files prompt vs live MCP schema (#1665)", () => {
|
||||
test("the payload the old prompt taught is rejected by the tool schema", () => {
|
||||
const taughtByOldPrompt = {
|
||||
files: ["path/to/old.js"],
|
||||
message: "chore: remove deprecated file",
|
||||
};
|
||||
const result = deleteFilesPayloadSchema.safeParse(taughtByOldPrompt);
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
const fields = result.error.issues.map((issue) => issue.path.join("."));
|
||||
expect(fields).toContain("paths");
|
||||
}
|
||||
});
|
||||
|
||||
test("the payload the new prompt teaches is accepted by the tool schema", () => {
|
||||
const taughtByNewPrompt = {
|
||||
paths: ["path/to/old.js"],
|
||||
message: "chore: remove deprecated file",
|
||||
};
|
||||
const result = deleteFilesPayloadSchema.safeParse(taughtByNewPrompt);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test("generated tag-mode prompt example parses against the live schema", async () => {
|
||||
const prompt = await generatePrompt(
|
||||
signingContext,
|
||||
mockGitHubData,
|
||||
true,
|
||||
"tag",
|
||||
);
|
||||
const example = extractToolExample(
|
||||
prompt,
|
||||
"mcp__github_file_ops__delete_files",
|
||||
);
|
||||
expect(example).toHaveProperty("paths");
|
||||
expect(example).not.toHaveProperty("files");
|
||||
const result = deleteFilesPayloadSchema.safeParse(example);
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test("rejects paths when the value is a string instead of an array", () => {
|
||||
const result = deleteFilesPayloadSchema.safeParse({
|
||||
paths: "path/to/old.js",
|
||||
message: "chore: remove deprecated file",
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
test("rejects a payload that has paths but omits message", () => {
|
||||
const result = deleteFilesPayloadSchema.safeParse({
|
||||
paths: ["path/to/old.js"],
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
if (!result.success) {
|
||||
expect(
|
||||
result.error.issues.map((issue) => issue.path.join(".")),
|
||||
).toContain("message");
|
||||
}
|
||||
});
|
||||
|
||||
test("accepts a payload that still includes the old files key beside paths", () => {
|
||||
const result = deleteFilesPayloadSchema.safeParse({
|
||||
files: ["path/to/old.js"],
|
||||
paths: ["path/to/old.js"],
|
||||
message: "chore: remove deprecated file",
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
test("does not change commit_files — that sibling tool still requires files", async () => {
|
||||
const prompt = await generatePrompt(
|
||||
signingContext,
|
||||
mockGitHubData,
|
||||
true,
|
||||
"tag",
|
||||
);
|
||||
const example = extractToolExample(
|
||||
prompt,
|
||||
"mcp__github_file_ops__commit_files",
|
||||
);
|
||||
expect(example).toHaveProperty("files");
|
||||
expect(example).not.toHaveProperty("paths");
|
||||
expect(commitFilesPayloadSchema.safeParse(example).success).toBe(true);
|
||||
expect(deleteFilesPayloadSchema.safeParse(example).success).toBe(false);
|
||||
});
|
||||
|
||||
test("generated delete_files example keys are exactly paths and message", async () => {
|
||||
const prompt = await generatePrompt(
|
||||
signingContext,
|
||||
mockGitHubData,
|
||||
true,
|
||||
"tag",
|
||||
);
|
||||
const example = extractToolExample(
|
||||
prompt,
|
||||
"mcp__github_file_ops__delete_files",
|
||||
);
|
||||
expect(Object.keys(example).sort()).toEqual(["message", "paths"]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user