mirror of
https://github.com/anthropics/claude-code-action.git
synced 2026-09-17 15:00:46 +08:00
fix: bound download_job_log against a stalled log fetch (#1719)
The download_job_log MCP tool called client.actions.downloadJobLogsForWorkflowRun() with no timeout and no AbortController. @octokit/rest@21 runs on Node's native fetch, which has no default timeout, and Octokit only cancels a request when the caller passes request.signal. If the log blob fetch stalls, that await never resolves and never rejects. This tool is always enabled in tag mode (src/modes/tag/index.ts), so a "fix the failing CI" run that calls get_ci_status -> get_workflow_run_details -> download_job_log can hang on this one await with nothing to recover it. It's headless, so the run only ends when the Actions job-level timeout-minutes kills it, burning the whole job budget with the tracking comment stuck at "Claude Code is working...". Sibling fetch in src/github/utils/image-downloader.ts (fetchImage) already got this treatment in #1625 via a timeout-driven AbortController. Same shape of call: fetch a GitHub-hosted resource by ID from untrusted PR/CI content. This mirrors that fix for github-actions-server.ts. Extracted the download+write logic into an exported downloadJobLog() function (with an injectable timeoutMs) so the timeout path is directly testable, and guarded the module's entrypoint side effects with import.meta.main, matching the pattern already used by the other entrypoints in src/entrypoints/.
This commit is contained in:
@@ -17,11 +17,18 @@ const PR_NUMBER = process.env.PR_NUMBER;
|
||||
const GITHUB_TOKEN = process.env.GITHUB_TOKEN;
|
||||
const RUNNER_TEMP = process.env.RUNNER_TEMP || "/tmp";
|
||||
|
||||
if (!REPO_OWNER || !REPO_NAME || !PR_NUMBER || !GITHUB_TOKEN) {
|
||||
console.error(
|
||||
"[GitHub CI Server] Error: REPO_OWNER, REPO_NAME, PR_NUMBER, and GITHUB_TOKEN environment variables are required",
|
||||
);
|
||||
process.exit(1);
|
||||
// Job logs are fetched by ID from GitHub-hosted storage; bound the request so a
|
||||
// stalled fetch can't hang this MCP call forever. Mirrors the timeout added to
|
||||
// fetchImage() in src/github/utils/image-downloader.ts (#1625).
|
||||
const DOWNLOAD_JOB_LOG_TIMEOUT_MS = 30_000;
|
||||
|
||||
if (import.meta.main) {
|
||||
if (!REPO_OWNER || !REPO_NAME || !PR_NUMBER || !GITHUB_TOKEN) {
|
||||
console.error(
|
||||
"[GitHub CI Server] Error: REPO_OWNER, REPO_NAME, PR_NUMBER, and GITHUB_TOKEN environment variables are required",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
const server = new McpServer({
|
||||
@@ -205,6 +212,40 @@ server.tool(
|
||||
},
|
||||
);
|
||||
|
||||
export async function downloadJobLog(
|
||||
client: Octokit,
|
||||
params: { owner: string; repo: string; job_id: number },
|
||||
runnerTemp: string,
|
||||
timeoutMs: number = DOWNLOAD_JOB_LOG_TIMEOUT_MS,
|
||||
): Promise<{ path: string; size_bytes: number }> {
|
||||
const controller = new AbortController();
|
||||
const timeoutHandle = setTimeout(() => controller.abort(), timeoutMs);
|
||||
|
||||
try {
|
||||
const response = await client.actions.downloadJobLogsForWorkflowRun({
|
||||
owner: params.owner,
|
||||
repo: params.repo,
|
||||
job_id: params.job_id,
|
||||
request: { signal: controller.signal },
|
||||
});
|
||||
|
||||
const logsText = response.data as unknown as string;
|
||||
|
||||
const logsDir = `${runnerTemp}/github-ci-logs`;
|
||||
await mkdir(logsDir, { recursive: true });
|
||||
|
||||
const logPath = `${logsDir}/job-${params.job_id}.log`;
|
||||
await writeFile(logPath, logsText, "utf-8");
|
||||
|
||||
return {
|
||||
path: logPath,
|
||||
size_bytes: Buffer.byteLength(logsText, "utf-8"),
|
||||
};
|
||||
} finally {
|
||||
clearTimeout(timeoutHandle);
|
||||
}
|
||||
}
|
||||
|
||||
server.tool(
|
||||
"download_job_log",
|
||||
"Download job logs to disk",
|
||||
@@ -218,24 +259,11 @@ server.tool(
|
||||
baseUrl: GITHUB_API_URL,
|
||||
});
|
||||
|
||||
const response = await client.actions.downloadJobLogsForWorkflowRun({
|
||||
owner: REPO_OWNER!,
|
||||
repo: REPO_NAME!,
|
||||
job_id,
|
||||
});
|
||||
|
||||
const logsText = response.data as unknown as string;
|
||||
|
||||
const logsDir = `${RUNNER_TEMP}/github-ci-logs`;
|
||||
await mkdir(logsDir, { recursive: true });
|
||||
|
||||
const logPath = `${logsDir}/job-${job_id}.log`;
|
||||
await writeFile(logPath, logsText, "utf-8");
|
||||
|
||||
const result = {
|
||||
path: logPath,
|
||||
size_bytes: Buffer.byteLength(logsText, "utf-8"),
|
||||
};
|
||||
const result = await downloadJobLog(
|
||||
client,
|
||||
{ owner: REPO_OWNER!, repo: REPO_NAME!, job_id },
|
||||
RUNNER_TEMP,
|
||||
);
|
||||
|
||||
return {
|
||||
content: [
|
||||
@@ -277,6 +305,8 @@ async function runServer() {
|
||||
}
|
||||
}
|
||||
|
||||
runServer().catch(() => {
|
||||
process.exit(1);
|
||||
});
|
||||
if (import.meta.main) {
|
||||
runServer().catch(() => {
|
||||
process.exit(1);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,94 @@
|
||||
import { describe, test, expect, afterEach } from "bun:test";
|
||||
import { readFile, rm } from "fs/promises";
|
||||
import os from "os";
|
||||
import path from "path";
|
||||
import { downloadJobLog } from "../src/mcp/github-actions-server";
|
||||
import type { Octokit } from "@octokit/rest";
|
||||
|
||||
describe("downloadJobLog", () => {
|
||||
const tmpDirs: string[] = [];
|
||||
|
||||
const makeRunnerTemp = () => {
|
||||
const dir = path.join(
|
||||
os.tmpdir(),
|
||||
`download-job-log-test-${Date.now()}-${Math.random().toString(36).slice(2)}`,
|
||||
);
|
||||
tmpDirs.push(dir);
|
||||
return dir;
|
||||
};
|
||||
|
||||
afterEach(async () => {
|
||||
while (tmpDirs.length) {
|
||||
const dir = tmpDirs.pop()!;
|
||||
await rm(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
const createStallingClient = (): {
|
||||
client: Octokit;
|
||||
getSignal: () => AbortSignal | undefined;
|
||||
} => {
|
||||
let signal: AbortSignal | undefined;
|
||||
const client = {
|
||||
actions: {
|
||||
downloadJobLogsForWorkflowRun: (params: {
|
||||
request?: { signal?: AbortSignal };
|
||||
}) => {
|
||||
signal = params.request?.signal;
|
||||
return new Promise((_resolve, reject) => {
|
||||
signal?.addEventListener("abort", () => {
|
||||
reject(new Error("This operation was aborted"));
|
||||
});
|
||||
// Otherwise never settles, simulating a stalled fetch.
|
||||
});
|
||||
},
|
||||
},
|
||||
} as unknown as Octokit;
|
||||
return { client, getSignal: () => signal };
|
||||
};
|
||||
|
||||
test("rejects with a timeout instead of hanging when the download stalls", async () => {
|
||||
const { client, getSignal } = createStallingClient();
|
||||
const runnerTemp = makeRunnerTemp();
|
||||
|
||||
await expect(
|
||||
downloadJobLog(
|
||||
client,
|
||||
{ owner: "owner", repo: "repo", job_id: 123 },
|
||||
runnerTemp,
|
||||
5,
|
||||
),
|
||||
).rejects.toThrow();
|
||||
|
||||
expect(getSignal()?.aborted).toBe(true);
|
||||
});
|
||||
|
||||
test("writes the log to disk and clears the timeout when the download succeeds", async () => {
|
||||
const runnerTemp = makeRunnerTemp();
|
||||
const client = {
|
||||
actions: {
|
||||
downloadJobLogsForWorkflowRun: async (params: {
|
||||
request?: { signal?: AbortSignal };
|
||||
}) => {
|
||||
expect(params.request?.signal?.aborted).toBe(false);
|
||||
return { data: "log line 1\nlog line 2\n" };
|
||||
},
|
||||
},
|
||||
} as unknown as Octokit;
|
||||
|
||||
const result = await downloadJobLog(
|
||||
client,
|
||||
{ owner: "owner", repo: "repo", job_id: 456 },
|
||||
runnerTemp,
|
||||
30_000,
|
||||
);
|
||||
|
||||
expect(result.path).toBe(`${runnerTemp}/github-ci-logs/job-456.log`);
|
||||
expect(result.size_bytes).toBe(
|
||||
Buffer.byteLength("log line 1\nlog line 2\n", "utf-8"),
|
||||
);
|
||||
|
||||
const written = await readFile(result.path, "utf-8");
|
||||
expect(written).toBe("log line 1\nlog line 2\n");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user