diff --git a/action.yml b/action.yml index 2a36dc85..3cbacca9 100644 --- a/action.yml +++ b/action.yml @@ -259,6 +259,7 @@ runs: OPENROUTER_API_KEY: ${{ inputs.openrouter_api_key }} GOOGLE_API_KEY: ${{ inputs.google_api_key }} AUTO_COLLECT_FEEDBACK: ${{ inputs.auto_collect_feedback }} + JUNIE_VERSION: ${{ inputs.junie_version }} - name: Install Junie diff --git a/src/constants/environment.ts b/src/constants/environment.ts index 84a0d6c7..55e8b641 100644 --- a/src/constants/environment.ts +++ b/src/constants/environment.ts @@ -26,6 +26,7 @@ export const ENV_VARS = { RESOLVE_CONFLICTS: "RESOLVE_CONFLICTS", CREATE_NEW_BRANCH_FOR_PR: "CREATE_NEW_BRANCH_FOR_PR", SILENT_MODE: "SILENT_MODE", + JUNIE_VERSION: "JUNIE_VERSION", SKIP_REVIEW_SUMMARY: "SKIP_REVIEW_SUMMARY", SKIP_PR: "SKIP_PR", OUTPUT_BRANCH: "OUTPUT_BRANCH", diff --git a/src/github/context.ts b/src/github/context.ts index 42d7a392..8e7f3d73 100644 --- a/src/github/context.ts +++ b/src/github/context.ts @@ -139,6 +139,7 @@ type JunieWorkflowContext = { workingBranch?: string; allowedMcpServers?: string; autoCollectFeedback: boolean; + junieVersion: string; }; }; @@ -210,6 +211,7 @@ export function extractJunieWorkflowContext(tokenOwner: TokenOwner): JunieExecut targetBranch: process.env.TARGET_BRANCH, allowedMcpServers: process.env.ALLOWED_MCP_SERVERS, autoCollectFeedback: process.env.AUTO_COLLECT_FEEDBACK === "true", + junieVersion: process.env.JUNIE_VERSION ?? "", }, }; diff --git a/src/github/junie/junie-tasks.ts b/src/github/junie/junie-tasks.ts index e2c0fde7..09d7f722 100644 --- a/src/github/junie/junie-tasks.ts +++ b/src/github/junie/junie-tasks.ts @@ -16,11 +16,23 @@ import {Octokits} from "../api/client"; import {NewGitHubPromptFormatter} from "./new-prompt-formatter"; import {GraphQLGitHubDataFetcher} from "../api/graphql-data-fetcher"; import {FetchedData} from "../api/queries"; -import {CliInput} from "./types/junie"; -import {generateMcpToolsPrompt} from "../../mcp/mcp-prompts"; +import {CliInput, remoteRequestReviewTarget} from "./types/junie"; +import {generateMcpToolsPrompt, LEGACY_INLINE_COMMENT_TOOL_PROMPT} from "../../mcp/mcp-prompts"; import {junieArgsToString} from "../../utils/junie-args-parser"; import {buildDiffCommand} from "../../constants/github"; +/** + * First Junie CLI build accepting `codeReviewTask.reviewTarget`. Older builds parse the input + * strictly and abort the run with "Cannot parse input JSON" on an unknown field, so they get + * the payload they shipped with and post inline comments through the MCP server instead. + */ +const MIN_VERSION_WITH_REVIEW_TARGET = 3056.1; + +/** `latest` installs the nightly channel; an unparseable version is assumed to be old. */ +function supportsReviewTarget(version: string): boolean { + return version.trim() === "latest" || parseFloat(version) >= MIN_VERSION_WITH_REVIEW_TARGET; +} + function getTriggerTime(context: JunieExecutionContext): string | undefined { if (isIssueCommentEvent(context)) { return context.payload.comment.created_at; @@ -81,10 +93,17 @@ export async function prepareJunieTask( // Note: Attachments are already processed in fetchIssueData/fetchPullRequestData if (isCodeReviewEvent(context)) { const diffPoint = branchInfo.prBaseBranch || branchInfo.baseBranch; + const prNumber = context.entityNumber; + if (!prNumber) { + throw new Error("Code review requires a Pull Request number, but none was found in the event context."); + } const diffCommand = buildDiffCommand(diffPoint, branchInfo.mergeBaseSha); + const legacyCli = !supportsReviewTarget(context.inputs.junieVersion); + const legacyInlineComments = legacyCli && enabledMcpServers.includes("mcp_github_inline_comment_server"); junieCLITask.codeReviewTask = { - description: promptText, - diffCommand + description: legacyInlineComments ? `${promptText}\n\n${LEGACY_INLINE_COMMENT_TOOL_PROMPT}` : promptText, + diffCommand, + ...(legacyCli ? {} : {fetchVcsInfo: true, reviewTarget: remoteRequestReviewTarget(prNumber)}), } } else { junieCLITask.task = promptText; diff --git a/src/github/junie/types/junie.ts b/src/github/junie/types/junie.ts index eb023174..10ed67a5 100644 --- a/src/github/junie/types/junie.ts +++ b/src/github/junie/types/junie.ts @@ -13,9 +13,22 @@ export interface MergeTask { branch: string; } +export interface RemoteRequestReviewTarget { + type: "remoteRequest"; + number: number; +} + +export type ReviewTarget = RemoteRequestReviewTarget; + +export function remoteRequestReviewTarget(prNumber: number): RemoteRequestReviewTarget { + return {type: "remoteRequest", number: prNumber}; +} + export interface CodeReview { description?: string; diffCommand?: string; + fetchVcsInfo?: boolean; + reviewTarget?: ReviewTarget; } export interface CliOutput { diff --git a/src/mcp/mcp-prompts.ts b/src/mcp/mcp-prompts.ts index 2838e480..497e368c 100644 --- a/src/mcp/mcp-prompts.ts +++ b/src/mcp/mcp-prompts.ts @@ -9,11 +9,13 @@ export const MCP_TOOL_PROMPTS = { mcp_github_checks_server: 'Use get_pr_failed_checks_info to retrieve detailed information about failed CI/CD checks if needed.', - mcp_github_inline_comment_server: 'MANDATORY for code reviews: Use post_inline_review_comment to provide inline code review comments. IMPORTANT: If you are responding to a question in an existing review thread (user tagged you in in review thread), DO NOT use this tool - your summary will be automatically posted as a reply in that thread.', youtrack: 'IMPORTANT: Do not post any comments - your summary will be automatically posted by system. And DO NOT update issue status if user did not request it. ALSO: do not look for other issues or any other external information unless explicitly requested by the user.', jira: 'IMPORTANT: Do not post any comments - your summary will be automatically posted by system. And DO NOT update issue status if user did not request it. ALSO: do not look for other issues or any other external information unless explicitly requested by the user.', }; +/** Inline comment instructions for Junie CLI builds without `reviewTarget`: newer builds post comments themselves. */ +export const LEGACY_INLINE_COMMENT_TOOL_PROMPT = 'MANDATORY for code reviews: Use post_inline_review_comment to provide inline code review comments. IMPORTANT: If you are responding to a question in an existing review thread (user tagged you in in review thread), DO NOT use this tool - your summary will be automatically posted as a reply in that thread.'; + /** * Generates a combined prompt section describing all enabled MCP tools */ diff --git a/test/junie-tasks.test.ts b/test/junie-tasks.test.ts index dab3e57e..12db5357 100644 --- a/test/junie-tasks.test.ts +++ b/test/junie-tasks.test.ts @@ -15,6 +15,73 @@ mock.module("../src/github/junie/attachment-downloader", () => ({ downloadAttachmentsAndRewriteText: mock((text: string) => Promise.resolve(text)), })); +/** + * The input schema the Junie CLI accepts, mirrored from the CLI sources: `CliInput` and + * `CliCodeReviewTask` (api/Input.kt) and `ReviewTarget` (attachments/ReviewTarget.kt). + * The CLI decodes the input strictly, so a single unknown key or an unknown `reviewTarget.type` + * aborts the whole run with "Cannot parse input JSON" before any task starts. + */ +const CLI_INPUT_KEYS = [ + "codeReviewTask", + "debugTask", + "mergeTask", + "orchestratedTask", + "rebaseTask", + "sessionId", + "task" +]; + +const CODE_REVIEW_TASK_KEYS = [ + "description", + "diffCommand", + "fetchVcsInfo", + // Deprecated on the CLI side: superseded by reviewTarget, which also selects the comment + // channel. Sending it switches the review back to the external MCP comment tool. + "includeInlineCommentToolInstructions", + "reviewTarget" +]; + +const REVIEW_TARGET_TYPES = ["localChanges", "remoteRequest"]; + +/** The keys the action is expected to send for a code review, in the order `sort()` produces. */ +const EXPECTED_CODE_REVIEW_TASK_KEYS = ["description", "diffCommand", "fetchVcsInfo", "reviewTarget"]; + +/** The keys CLI builds without `reviewTarget` support accept. */ +const LEGACY_CODE_REVIEW_TASK_KEYS = ["description", "diffCommand"]; + +/** Asserts the payload carries nothing the CLI's strict parser would reject. */ +const expectParseableByJunieCli = (input: Record) => { + for (const key of Object.keys(input)) { + expect(CLI_INPUT_KEYS).toContain(key); + } + + if (input.mergeTask) { + expect(Object.keys(input.mergeTask)).toEqual(["branch"]); + expect(typeof input.mergeTask.branch).toBe("string"); + } + + const codeReviewTask = input.codeReviewTask; + if (codeReviewTask) { + for (const key of Object.keys(codeReviewTask)) { + expect(CODE_REVIEW_TASK_KEYS).toContain(key); + } + + const reviewTarget = codeReviewTask.reviewTarget; + if (reviewTarget) { + expect(REVIEW_TARGET_TYPES).toContain(reviewTarget.type); + if (reviewTarget.type === "remoteRequest") { + expect(Object.keys(reviewTarget).sort()).toEqual(["number", "type"]); + // Parsed into a Kotlin Int, so a float or a stringified number fails the parser + expect(Number.isInteger(reviewTarget.number)).toBe(true); + } + } + } +}; + +/** Reads back the file the CLI is actually fed, instead of the object `prepareJunieTask` returns. */ +const readJunieInputFile = (): Record => + JSON.parse(fs.readFileSync(`${process.env.WORKING_DIR}/junie_input.json`, "utf-8")); + describe("prepareJunieTask", () => { const createMockContext = (overrides: Partial = {}): JunieExecutionContext => { const defaultInputs = { @@ -29,7 +96,8 @@ describe("prepareJunieTask", () => { triggerPhrase: "@junie-agent", assigneeTrigger: "", labelTrigger: "", - allowedMcpServers: "" + allowedMcpServers: "", + junieVersion: "latest" }; const { inputs: _, ...restOverrides } = overrides; @@ -397,16 +465,19 @@ describe("prepareJunieTask", () => { }); test("should create codeReviewTask when code-review prompt is provided", async () => { + // Deliberately different from the default entity number: the review target + // must carry the number of the reviewed PR, not a hardcoded value + const prNumber = 4242; const context = createMockContext({ eventName: "pull_request", isPR: true, - entityNumber: 123, + entityNumber: prNumber, inputs: { prompt: "code-review" }, payload: { pull_request: { - number: 123, + number: prNumber, title: "Test PR", updated_at: "2024-01-01T00:00:00Z" }, @@ -418,11 +489,24 @@ describe("prepareJunieTask", () => { }); const octokit = createMockOctokit(); - const result = await prepareJunieTask(context, branchInfo, octokit); + // Inline comments are posted by the CLI itself, so the inline comment MCP server + // must not be advertised in the prompt anymore even when it is enabled + const result = await prepareJunieTask(context, branchInfo, octokit, ["mcp_github_inline_comment_server"]); expect(result).toBeDefined(); + expect(result.task).toBeUndefined(); + expect(result.mergeTask).toBeUndefined(); expect(result.codeReviewTask).toBeDefined(); + // The Junie CLI parses the whole input strictly and rejects it on unknown fields, + // so the exact set of keys is part of the contract, not an implementation detail + expect(Object.keys(result.codeReviewTask!).sort()).toEqual(EXPECTED_CODE_REVIEW_TASK_KEYS); + // reviewTarget alone must select the comment channel: the deprecated flag would + // pin the review to the external MCP comment tool instead of the CLI's own one + expect(result.codeReviewTask).not.toHaveProperty("includeInlineCommentToolInstructions"); + // The base ref the CLI derives from the command must stay resolvable expect(result.codeReviewTask?.diffCommand).toContain("git diff origin/main"); + expect(result.codeReviewTask?.fetchVcsInfo).toBe(true); + expect(result.codeReviewTask?.reviewTarget).toEqual({type: "remoteRequest", number: prNumber}); expect(result.codeReviewTask?.description).toContain(""); // Header should NOT contain "Your task is to:" expect(result.codeReviewTask?.description).toContain("You were triggered as a GitHub AI Assistant by pull_request action."); @@ -430,6 +514,46 @@ describe("prepareJunieTask", () => { // For code review, user_instruction should not be attached at all expect(result.codeReviewTask?.description).not.toContain(""); expect(result.codeReviewTask?.description).not.toContain("code-review"); + expect(result.codeReviewTask?.description).not.toContain("post_inline_review_comment"); + + // The CLI reads the task from the file, not from the returned object + expect(core.setOutput).toHaveBeenCalledWith("JUNIE_INPUT_FILE", `${process.env.WORKING_DIR}/junie_input.json`); + const writtenInput = readJunieInputFile(); + expect(writtenInput).toEqual(result); + expectParseableByJunieCli(writtenInput); + }); + + test("should create codeReviewTask without reviewTarget when the CLI version does not support it", async () => { + const context = createMockContext({ + eventName: "pull_request", + isPR: true, + entityNumber: 123, + inputs: { + prompt: "code-review", + // Released build without reviewTarget: sending it would abort the whole run + // with "Cannot parse input JSON" + junieVersion: "2929.5" + }, + payload: { + pull_request: { + number: 123, + title: "Test PR", + updated_at: "2024-01-01T00:00:00Z" + }, + repository: { + owner: {login: "owner"}, + name: "repo" + } + } as any + }); + const octokit = createMockOctokit(); + + const result = await prepareJunieTask(context, branchInfo, octokit, ["mcp_github_inline_comment_server"]); + + expect(Object.keys(result.codeReviewTask!).sort()).toEqual(LEGACY_CODE_REVIEW_TASK_KEYS); + // Such builds have no comment channel of their own, so the MCP tool has to be advertised + expect(result.codeReviewTask?.description).toContain("post_inline_review_comment"); + expectParseableByJunieCli(readJunieInputFile()); }); test("should trigger codeReviewTask from comment when inputs.prompt is empty and code-review keyword is used", async () => { @@ -465,15 +589,45 @@ describe("prepareJunieTask", () => { const result = await prepareJunieTask(context, branchInfo, octokit); expect(result).toBeDefined(); + expect(result.task).toBeUndefined(); expect(result.codeReviewTask).toBeDefined(); // Should detect code-review trigger from comment and create codeReviewTask + expect(Object.keys(result.codeReviewTask!).sort()).toEqual(EXPECTED_CODE_REVIEW_TASK_KEYS); expect(result.codeReviewTask?.diffCommand).toContain("git diff origin/main"); + expect(result.codeReviewTask?.fetchVcsInfo).toBe(true); + expect(result.codeReviewTask?.reviewTarget).toEqual({type: "remoteRequest", number: 123}); expect(result.codeReviewTask?.description).toContain(""); // Header should NOT contain "Your task is to:" expect(result.codeReviewTask?.description).toContain("You were triggered as a GitHub AI Assistant by pull_request_review action."); expect(result.codeReviewTask?.description).not.toContain("Your task is to:"); // For code review, user_instruction should not be attached expect(result.codeReviewTask?.description).not.toContain(""); + + const writtenInput = readJunieInputFile(); + expect(writtenInput).toEqual(result); + expectParseableByJunieCli(writtenInput); + }); + + test("should fail to create codeReviewTask when PR number is not available", async () => { + const context = createMockContext({ + eventName: "workflow_dispatch" as any, + isPR: false, + entityNumber: undefined, + inputs: { + prompt: "code-review" + }, + payload: { + repository: { + owner: {login: "owner"}, + name: "repo" + } + } as any + }); + const octokit = createMockOctokit(); + + await expect(prepareJunieTask(context, branchInfo, octokit)).rejects.toThrow( + "Code review requires a Pull Request number" + ); }); test("should not trigger fix CI prompt when workflow_run event has success conclusion", async () => { @@ -596,6 +750,7 @@ describe("prepareJunieTask", () => { expect(result.mergeTask).toBeDefined(); expect(result.task).toBeUndefined(); expect(result.mergeTask?.branch).toBe("main"); + expectParseableByJunieCli(readJunieInputFile()); }); test("should set merge task when comment has resolve trigger phrase", async () => { @@ -628,6 +783,7 @@ describe("prepareJunieTask", () => { expect(result.mergeTask).toBeDefined(); expect(result.task).toBeUndefined(); expect(result.mergeTask?.branch).toBe("main"); + expectParseableByJunieCli(readJunieInputFile()); }); }); @@ -653,6 +809,7 @@ describe("prepareJunieTask", () => { expect(result).toBeDefined(); expect(core.setOutput).toHaveBeenCalledWith("JUNIE_INPUT_FILE", expect.any(String)); expect(core.setOutput).toHaveBeenCalledWith("CUSTOM_JUNIE_ARGS", expect.any(String)); + expectParseableByJunieCli(readJunieInputFile()); }); });