From a3ff61d47aa5118a43b33ae44c4087d9eb51111a Mon Sep 17 00:00:00 2001 From: Ashwin Bhat Date: Tue, 9 Sep 2025 09:19:14 -0700 Subject: [PATCH] enable track_progress for comments, fix mcp config (#558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * fix test --------- Co-authored-by: Claude --- src/mcp/install-mcp-server.ts | 5 +++-- src/modes/agent/index.ts | 1 + src/modes/detector.ts | 18 +++++++++++++++--- src/modes/tag/index.ts | 1 + test/install-mcp-server.test.ts | 10 ++++++++++ test/modes/agent.test.ts | 6 +++--- tests/modes/detector.test.ts | 2 +- 7 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/mcp/install-mcp-server.ts b/src/mcp/install-mcp-server.ts index aae6a02..c107fc1 100644 --- a/src/mcp/install-mcp-server.ts +++ b/src/mcp/install-mcp-server.ts @@ -13,6 +13,7 @@ type PrepareConfigParams = { claudeCommentId?: string; allowedTools: string[]; context: GitHubContext; + mode: "tag" | "agent"; }; async function checkActionsReadPermission( @@ -59,12 +60,12 @@ export async function prepareMcpConfig( claudeCommentId, allowedTools, context, + mode, } = params; try { const allowedToolsList = allowedTools || []; - // Detect if we're in agent mode (explicit prompt provided) - const isAgentMode = !!context.inputs?.prompt; + const isAgentMode = mode === "agent"; const hasGitHubMcpTools = allowedToolsList.some((tool) => tool.startsWith("mcp__github__"), diff --git a/src/modes/agent/index.ts b/src/modes/agent/index.ts index ce526ba..9c3a7b2 100644 --- a/src/modes/agent/index.ts +++ b/src/modes/agent/index.ts @@ -136,6 +136,7 @@ export const agentMode: Mode = { claudeCommentId: undefined, // No tracking comment in agent mode allowedTools, context, + mode: "agent", }); // Build final claude_args with multiple --mcp-config flags diff --git a/src/modes/detector.ts b/src/modes/detector.ts index 5b9b7cd..8e30aff 100644 --- a/src/modes/detector.ts +++ b/src/modes/detector.ts @@ -19,7 +19,13 @@ export function detectMode(context: GitHubContext): AutoDetectedMode { // If track_progress is set for PR/issue events, force tag mode if (context.inputs.trackProgress && isEntityContext(context)) { - if (isPullRequestEvent(context) || isIssuesEvent(context)) { + if ( + isPullRequestEvent(context) || + isIssuesEvent(context) || + isIssueCommentEvent(context) || + isPullRequestReviewCommentEvent(context) || + isPullRequestReviewEvent(context) + ) { return "tag"; } } @@ -87,10 +93,16 @@ export function getModeDescription(mode: AutoDetectedMode): string { function validateTrackProgressEvent(context: GitHubContext): void { // 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)) { 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}`, ); } diff --git a/src/modes/tag/index.ts b/src/modes/tag/index.ts index 4d997f2..adcf6e8 100644 --- a/src/modes/tag/index.ts +++ b/src/modes/tag/index.ts @@ -122,6 +122,7 @@ export const tagMode: Mode = { claudeCommentId: commentId.toString(), allowedTools: [], context, + mode: "tag", }); // Don't output mcp_config separately anymore - include in claude_args diff --git a/test/install-mcp-server.test.ts b/test/install-mcp-server.test.ts index 41879d6..987dd7c 100644 --- a/test/install-mcp-server.test.ts +++ b/test/install-mcp-server.test.ts @@ -85,6 +85,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockContext, + mode: "tag", }); const parsed = JSON.parse(result); @@ -106,6 +107,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockContextWithSigning, + mode: "tag", }); const parsed = JSON.parse(result); @@ -129,6 +131,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: ["mcp__github__create_issue", "mcp__github__create_pr"], context: mockContext, + mode: "tag", }); const parsed = JSON.parse(result); @@ -149,6 +152,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: ["mcp__github_inline_comment__create_inline_comment"], context: mockPRContext, + mode: "tag", }); const parsed = JSON.parse(result); @@ -169,6 +173,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockContext, + mode: "tag", }); const parsed = JSON.parse(result); @@ -189,6 +194,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockContextWithSigning, + mode: "tag", }); const parsed = JSON.parse(result); @@ -208,6 +214,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockContextWithSigning, + mode: "tag", }); const parsed = JSON.parse(result); @@ -225,6 +232,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockPRContext, + mode: "tag", }); const parsed = JSON.parse(result); @@ -244,6 +252,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockContext, + mode: "tag", }); const parsed = JSON.parse(result); @@ -261,6 +270,7 @@ describe("prepareMcpConfig", () => { baseBranch: "main", allowedTools: [], context: mockPRContext, + mode: "tag", }); const parsed = JSON.parse(result); diff --git a/test/modes/agent.test.ts b/test/modes/agent.test.ts index 9811707..16e3796 100644 --- a/test/modes/agent.test.ts +++ b/test/modes/agent.test.ts @@ -162,11 +162,11 @@ describe("Agent Mode", () => { 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]; expect(callArgs[0]).toBe("claude_args"); - expect(callArgs[1]).toContain("--mcp-config"); - expect(callArgs[1]).toContain("--model claude-sonnet-4 --max-turns 10"); + expect(callArgs[1]).toBe("--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 expect(result).toEqual({ diff --git a/tests/modes/detector.test.ts b/tests/modes/detector.test.ts index 39f5d14..5f1b812 100644 --- a/tests/modes/detector.test.ts +++ b/tests/modes/detector.test.ts @@ -200,7 +200,7 @@ describe("detectMode with enhanced routing", () => { }; expect(() => detectMode(context)).toThrow( - /track_progress is only supported for pull_request and issue events/, + /track_progress is only supported /, ); });