enable track_progress for comments, fix mcp config (#558)

* enable track_progress for comments

* refactor: pass mode explicitly to prepareMcpConfig

Update prepareMcpConfig to receive the mode parameter from its callers
instead of detecting agent mode by checking context.inputs.prompt.
This makes mode determination explicit and controlled by the caller.

Also update all test cases to include the required mode parameter
and fix agent mode test expectations to match new behavior where
MCP config is only included when tools are explicitly allowed.

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

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

* fix test

---------

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Ashwin Bhat
2025-09-09 09:19:14 -07:00
committed by GitHub
parent 1b7eb924f1
commit a3ff61d47a
7 changed files with 34 additions and 9 deletions

View File

@@ -13,6 +13,7 @@ type PrepareConfigParams = {
claudeCommentId?: string; claudeCommentId?: string;
allowedTools: string[]; allowedTools: string[];
context: GitHubContext; context: GitHubContext;
mode: "tag" | "agent";
}; };
async function checkActionsReadPermission( async function checkActionsReadPermission(
@@ -59,12 +60,12 @@ export async function prepareMcpConfig(
claudeCommentId, claudeCommentId,
allowedTools, allowedTools,
context, context,
mode,
} = params; } = params;
try { try {
const allowedToolsList = allowedTools || []; const allowedToolsList = allowedTools || [];
// Detect if we're in agent mode (explicit prompt provided) const isAgentMode = mode === "agent";
const isAgentMode = !!context.inputs?.prompt;
const hasGitHubMcpTools = allowedToolsList.some((tool) => const hasGitHubMcpTools = allowedToolsList.some((tool) =>
tool.startsWith("mcp__github__"), tool.startsWith("mcp__github__"),

View File

@@ -136,6 +136,7 @@ export const agentMode: Mode = {
claudeCommentId: undefined, // No tracking comment in agent mode claudeCommentId: undefined, // No tracking comment in agent mode
allowedTools, allowedTools,
context, context,
mode: "agent",
}); });
// Build final claude_args with multiple --mcp-config flags // Build final claude_args with multiple --mcp-config flags

View File

@@ -19,7 +19,13 @@ export function detectMode(context: GitHubContext): AutoDetectedMode {
// If track_progress is set for PR/issue events, force tag mode // If track_progress is set for PR/issue events, force tag mode
if (context.inputs.trackProgress && isEntityContext(context)) { if (context.inputs.trackProgress && isEntityContext(context)) {
if (isPullRequestEvent(context) || isIssuesEvent(context)) { if (
isPullRequestEvent(context) ||
isIssuesEvent(context) ||
isIssueCommentEvent(context) ||
isPullRequestReviewCommentEvent(context) ||
isPullRequestReviewEvent(context)
) {
return "tag"; return "tag";
} }
} }
@@ -87,10 +93,16 @@ export function getModeDescription(mode: AutoDetectedMode): string {
function validateTrackProgressEvent(context: GitHubContext): void { function validateTrackProgressEvent(context: GitHubContext): void {
// track_progress is only valid for pull_request and issue events // track_progress is only valid for pull_request and issue events
const validEvents = ["pull_request", "issues"]; const validEvents = [
"pull_request",
"issues",
"issue_comment",
"pull_request_review_comment",
"pull_request_review",
];
if (!validEvents.includes(context.eventName)) { if (!validEvents.includes(context.eventName)) {
throw new Error( throw new Error(
`track_progress is only supported for pull_request and issue events. ` + `track_progress is only supported for events: ${validEvents.join(", ")}. ` +
`Current event: ${context.eventName}`, `Current event: ${context.eventName}`,
); );
} }

View File

@@ -122,6 +122,7 @@ export const tagMode: Mode = {
claudeCommentId: commentId.toString(), claudeCommentId: commentId.toString(),
allowedTools: [], allowedTools: [],
context, context,
mode: "tag",
}); });
// Don't output mcp_config separately anymore - include in claude_args // Don't output mcp_config separately anymore - include in claude_args

View File

@@ -85,6 +85,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockContext, context: mockContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -106,6 +107,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockContextWithSigning, context: mockContextWithSigning,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -129,6 +131,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: ["mcp__github__create_issue", "mcp__github__create_pr"], allowedTools: ["mcp__github__create_issue", "mcp__github__create_pr"],
context: mockContext, context: mockContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -149,6 +152,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: ["mcp__github_inline_comment__create_inline_comment"], allowedTools: ["mcp__github_inline_comment__create_inline_comment"],
context: mockPRContext, context: mockPRContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -169,6 +173,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockContext, context: mockContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -189,6 +194,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockContextWithSigning, context: mockContextWithSigning,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -208,6 +214,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockContextWithSigning, context: mockContextWithSigning,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -225,6 +232,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockPRContext, context: mockPRContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -244,6 +252,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockContext, context: mockContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);
@@ -261,6 +270,7 @@ describe("prepareMcpConfig", () => {
baseBranch: "main", baseBranch: "main",
allowedTools: [], allowedTools: [],
context: mockPRContext, context: mockPRContext,
mode: "tag",
}); });
const parsed = JSON.parse(result); const parsed = JSON.parse(result);

View File

@@ -162,11 +162,11 @@ describe("Agent Mode", () => {
githubToken: "test-token", githubToken: "test-token",
}); });
// Verify claude_args includes MCP config and user args // Verify claude_args includes user args (no MCP config in agent mode without allowed tools)
const callArgs = setOutputSpy.mock.calls[0]; const callArgs = setOutputSpy.mock.calls[0];
expect(callArgs[0]).toBe("claude_args"); expect(callArgs[0]).toBe("claude_args");
expect(callArgs[1]).toContain("--mcp-config"); expect(callArgs[1]).toBe("--model claude-sonnet-4 --max-turns 10");
expect(callArgs[1]).toContain("--model claude-sonnet-4 --max-turns 10"); expect(callArgs[1]).not.toContain("--mcp-config");
// Verify return structure - should use "main" as fallback when no env vars set // Verify return structure - should use "main" as fallback when no env vars set
expect(result).toEqual({ expect(result).toEqual({

View File

@@ -200,7 +200,7 @@ describe("detectMode with enhanced routing", () => {
}; };
expect(() => detectMode(context)).toThrow( expect(() => detectMode(context)).toThrow(
/track_progress is only supported for pull_request and issue events/, /track_progress is only supported /,
); );
}); });