Fix MCP file discovery errors (#9)

* fix tool errors

*QA
This commit is contained in:
Shawn Morreau
2025-12-17 11:29:27 -05:00
committed by GitHub
parent a88b3d18ce
commit 6716183068
7 changed files with 6909 additions and 7559 deletions
+96 -55
View File
@@ -20,6 +20,97 @@ export type CheckoutPrResult = {
headRepo: string;
};
/**
* Shared helper to checkout a PR branch and configure fork remotes.
* Assumes origin remote is already configured with authentication.
*/
export async function checkoutPrBranch(ctx: Context, pull_number: number): Promise<void> {
log.info(`🔀 checking out PR #${pull_number}...`);
// fetch PR metadata
const pr = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
pull_number,
});
const headRepo = pr.data.head.repo;
if (!headRepo) {
throw new Error(`PR #${pull_number} source repository was deleted`);
}
const isFork = headRepo.full_name !== pr.data.base.repo.full_name;
const baseBranch = pr.data.base.ref;
const headBranch = pr.data.head.ref;
// check if we're already on the correct branch
const currentBranch = $("git", ["rev-parse", "--abbrev-ref", "HEAD"], { log: false }).trim();
const alreadyOnBranch = currentBranch === headBranch;
if (alreadyOnBranch) {
log.info(`already on PR branch ${headBranch}, skipping checkout`);
} else {
// fetch base branch so origin/<base> exists for diff operations
log.info(`📥 fetching base branch (${baseBranch})...`);
$("git", ["fetch", "--no-tags", "origin", baseBranch]);
// checkout base branch first to avoid "refusing to fetch into current branch" error
// -B creates or resets the branch to match origin/baseBranch
$("git", ["checkout", "-B", baseBranch, `origin/${baseBranch}`]);
// fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs)
log.info(`🌿 fetching PR #${pull_number} (${headBranch})...`);
$("git", ["fetch", "--no-tags", "origin", `pull/${pull_number}/head:${headBranch}`]);
// checkout the branch
$("git", ["checkout", headBranch]);
log.info(`✓ checked out PR #${pull_number}`);
}
// ensure base branch is fetched (needed for diff operations)
// fetch if we skipped checkout (already on branch) - otherwise already fetched above
if (alreadyOnBranch) {
log.info(`📥 fetching base branch (${baseBranch})...`);
$("git", ["fetch", "--no-tags", "origin", baseBranch]);
}
// configure push remote for this branch
// NOTE: This always runs regardless of alreadyOnBranch, because setupGit doesn't configure
// fork remotes. This ensures fork PRs can push even when checkout_pr is called after setupGit.
if (isFork) {
const remoteName = `pr-${pull_number}`;
const forkUrl = `https://x-access-token:${ctx.githubInstallationToken}@github.com/${headRepo.full_name}.git`;
// add fork as a named remote (ignore error if already exists)
try {
$("git", ["remote", "add", remoteName, forkUrl]);
log.info(`📌 added remote '${remoteName}' for fork ${headRepo.full_name}`);
} catch {
// remote already exists, update its URL
$("git", ["remote", "set-url", remoteName, forkUrl]);
log.info(`📌 updated remote '${remoteName}' for fork ${headRepo.full_name}`);
}
// set branch push config so `git push` knows where to push
$("git", ["config", `branch.${headBranch}.pushRemote`, remoteName]);
log.info(`📌 configured branch '${headBranch}' to push to '${remoteName}'`);
// warn if maintainer can't modify (push will likely fail)
if (!pr.data.maintainer_can_modify) {
log.warning(
`⚠️ fork PR has maintainer_can_modify=false - push operations will fail. ` +
`ask the PR author to enable "Allow edits from maintainers" or the fork may be owned by an organization.`
);
}
} else {
// for same-repo PRs, push to origin
$("git", ["config", `branch.${headBranch}.pushRemote`, "origin"]);
}
// set PR context
ctx.toolState.prNumber = pull_number;
}
export function CheckoutPrTool(ctx: Context) {
return tool({
name: "checkout_pr",
@@ -27,9 +118,9 @@ export function CheckoutPrTool(ctx: Context) {
"Checkout a pull request branch locally. This fetches the PR branch and sets up push configuration for fork PRs. Use this when you need to work on an existing PR.",
parameters: CheckoutPr,
execute: execute(ctx, async ({ pull_number }) => {
log.info(`🔀 checking out PR #${pull_number}...`);
await checkoutPrBranch(ctx, pull_number);
// fetch PR metadata
// fetch PR metadata to return result
const pr = await ctx.octokit.rest.pulls.get({
owner: ctx.owner,
repo: ctx.name,
@@ -41,63 +132,13 @@ export function CheckoutPrTool(ctx: Context) {
throw new Error(`PR #${pull_number} source repository was deleted`);
}
const isFork = headRepo.full_name !== pr.data.base.repo.full_name;
const baseBranch = pr.data.base.ref;
const headBranch = pr.data.head.ref;
// fetch base branch so origin/<base> exists for diff operations
log.info(`📥 fetching base branch (${baseBranch})...`);
$("git", ["fetch", "--no-tags", "origin", baseBranch]);
// fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs)
log.info(`🌿 fetching PR #${pull_number} (${headBranch})...`);
$("git", ["fetch", "--no-tags", "origin", `pull/${pull_number}/head:${headBranch}`]);
// checkout the branch
$("git", ["checkout", headBranch]);
log.info(`✓ checked out PR #${pull_number}`);
// configure push remote for this branch
if (isFork) {
const remoteName = `pr-${pull_number}`;
const forkUrl = `https://x-access-token:${ctx.githubInstallationToken}@github.com/${headRepo.full_name}.git`;
// add fork as a named remote (ignore error if already exists)
try {
$("git", ["remote", "add", remoteName, forkUrl]);
log.info(`📌 added remote '${remoteName}' for fork ${headRepo.full_name}`);
} catch {
// remote already exists, update its URL
$("git", ["remote", "set-url", remoteName, forkUrl]);
log.info(`📌 updated remote '${remoteName}' for fork ${headRepo.full_name}`);
}
// set branch push config so `git push` knows where to push
$("git", ["config", `branch.${headBranch}.pushRemote`, remoteName]);
log.info(`📌 configured branch '${headBranch}' to push to '${remoteName}'`);
// warn if maintainer can't modify (push will likely fail)
if (!pr.data.maintainer_can_modify) {
log.warning(
`⚠️ fork PR has maintainer_can_modify=false - push operations will fail. ` +
`ask the PR author to enable "Allow edits from maintainers" or the fork may be owned by an organization.`
);
}
} else {
// for same-repo PRs, push to origin
$("git", ["config", `branch.${headBranch}.pushRemote`, "origin"]);
}
// set PR context
ctx.toolState.prNumber = pull_number;
return {
success: true,
number: pr.data.number,
title: pr.data.title,
base: baseBranch,
head: headBranch,
isFork,
base: pr.data.base.ref,
head: pr.data.head.ref,
isFork: headRepo.full_name !== pr.data.base.repo.full_name,
maintainerCanModify: pr.data.maintainer_can_modify,
url: pr.data.html_url,
headRepo: headRepo.full_name,
+65 -15
View File
@@ -1,6 +1,7 @@
import { relative, resolve } from "node:path";
import { type } from "arktype";
import { $ } from "../utils/shell.ts";
import { handleToolError, handleToolSuccess, tool, type ToolResult } from "./shared.ts";
import { handleToolError, handleToolSuccess, type ToolResult, tool } from "./shared.ts";
export const ListFiles = type({
path: type.string
@@ -12,32 +13,81 @@ export const ListFiles = type({
export const ListFilesTool = tool({
name: "list_files",
description:
"List files in the repository using git ls-files. Useful for discovering the file structure and locating files.",
"List files in the repository, including both git-tracked and untracked files. Useful for discovering the file structure and locating files, including newly created files that haven't been committed yet.",
parameters: ListFiles,
execute: async ({ path }: { path?: string }): Promise<ToolResult> => {
try {
// Use git ls-files to list tracked files
// This respects .gitignore and gives a clean list of source files
const pathStr = path ?? ".";
const output = $("git", pathStr === "." ? ["ls-files"] : ["ls-files", pathStr], {
log: false,
});
const files = output.split("\n").filter((f) => f.trim() !== "");
const cwd = process.cwd();
if (files.length === 0) {
// Fallback for non-git environments or untracked files
// Get git-tracked files
let gitFiles: string[] = [];
let gitFailed = false;
try {
const gitArgs = pathStr === "." ? ["ls-files"] : ["ls-files", pathStr];
const gitOutput = $("git", gitArgs, { log: false });
gitFiles = gitOutput
.split("\n")
.filter((f) => f.trim() !== "")
.map((f) => f.trim());
} catch {
// git might fail, that's ok - we'll use find instead
gitFailed = true;
}
// Always also check filesystem for untracked files
// This is important because newly created files won't be in git yet
let filesystemFiles: string[] = [];
let findFailed = false;
try {
const findOutput = $(
"find",
[pathStr, "-maxdepth", "3", "-not", "-path", "*/.*", "-type", "f"],
{ log: false }
);
return handleToolSuccess({
files: findOutput.split("\n").filter((f) => f.trim() !== ""),
method: "find",
});
filesystemFiles = findOutput
.split("\n")
.filter((f) => f.trim() !== "")
.map((f) => {
const trimmed = f.trim();
// normalize to relative paths for comparison
try {
return relative(cwd, resolve(cwd, trimmed));
} catch {
return trimmed;
}
});
} catch {
// find might fail, that's ok - we'll just use git files
findFailed = true;
}
return handleToolSuccess({ files, method: "git" });
// if both methods failed, return an error
if (gitFailed && findFailed) {
return handleToolError(
new Error(
`Failed to list files: both git ls-files and find commands failed. ` +
`Path: ${pathStr}, working directory: ${cwd}`
)
);
}
// Create a Set of git files for efficient lookup
const gitFilesSet = new Set(gitFiles);
// Combine both lists, removing duplicates
const allFiles = [...new Set([...gitFiles, ...filesystemFiles])].sort();
// Calculate actual untracked count (files in filesystem but not in git)
const untrackedFiles = filesystemFiles.filter((f) => !gitFilesSet.has(f));
const untrackedCount = untrackedFiles.length;
return handleToolSuccess({
files: allFiles,
method: "combined",
trackedCount: gitFiles.length,
untrackedCount,
});
} catch (error) {
return handleToolError(error);
}
+2
View File
@@ -13,6 +13,7 @@ import {
ReportProgressTool,
} from "./comment.ts";
import { DebugShellCommandTool } from "./debug.ts";
import { ListFilesTool } from "./files.ts";
import { CommitFilesTool, CreateBranchTool, PushBranchTool } from "./git.ts";
import { IssueTool } from "./issue.ts";
import { GetIssueCommentsTool } from "./issueComments.ts";
@@ -91,6 +92,7 @@ export async function startMcpHttpServer(
CreateBranchTool(ctx),
CommitFilesTool(ctx),
PushBranchTool(ctx),
ListFilesTool,
];
// only include ReportProgressTool if progress comment is not disabled