diff --git a/entry b/entry index 4de7038..8cb45fb 100755 --- a/entry +++ b/entry @@ -141017,6 +141017,11 @@ async function checkoutPrBranch(pullNumber, params) { if (isFork) { toolState.pushUrl = `https://github.com/${headRepo.full_name}.git`; } + toolState.pushDest = { + remoteName: isFork ? `pr-${pullNumber}` : "origin", + remoteBranch: headBranch, + localBranch + }; await executeLifecycleHook({ event: "post-checkout", script: params.postCheckoutScript @@ -141417,6 +141422,9 @@ async function reportProgress(ctx, { body }) { action: "updated" }; } + if (existingCommentId === null) { + return { body, action: "skipped" }; + } if (issueNumber === void 0) { return { body, action: "skipped" }; } @@ -141823,7 +141831,10 @@ function resolveInstructions(ctx) { Call \`delegate\` with a mode, effort level, and optional instructions: - \`mode\`: The workflow to run (see available modes below) -- \`effort\`: \`"auto"\` (default, most capable), \`"mini"\` (fast, for simple tasks), or \`"max"\` (maximum capability) +- \`effort\`: + - \`"mini"\`: low-effort and fast, for simple tasks + - \`"auto"\`: medium-effort, good for typical tasks that don't require significant reasoning + - \`"max"\`: high-effort, good for PR reviews and complex coding tasks. - \`instructions\`: Optional additional context for the subagent. Use this to pass results from earlier delegations or narrow the subagent's focus. ### Single vs. multi-phase delegation @@ -141921,7 +141932,7 @@ var DelegateParams = type({ "the name of the mode to delegate to (e.g., 'Build', 'Plan', 'Review', 'Fix', 'AddressReviews')" ), "effort?": Effort.describe( - 'effort level for the subagent: "mini" (fast), "auto" (default, highly capable), or "max" (maximum capability)' + `effort level for the subagent: "mini" (low-effort and fast, only for simple tasks), "auto" (medium-effort, good for typical tasks that don't require significant reasoning), or "max" (high-effort, good for PR reviews and complex coding tasks)` ), "instructions?": type.string.describe( "optional additional context or instructions for the subagent \u2014 use this to pass results from earlier delegations or narrow the subagent's focus" @@ -142941,7 +142952,14 @@ function ListDirectoryTool(_ctx) { } // mcp/git.ts -function getPushDestination(branch) { +function getPushDestination(branch, storedDest) { + if (storedDest && storedDest.localBranch === branch) { + log.debug(`using stored push destination: ${storedDest.remoteName}/${storedDest.remoteBranch}`); + const url4 = $("git", ["remote", "get-url", "--push", storedDest.remoteName], { + log: false + }).trim(); + return { remoteName: storedDest.remoteName, remoteBranch: storedDest.remoteBranch, url: url4 }; + } try { const pushRemote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim(); const merge4 = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim(); @@ -142958,7 +142976,7 @@ function normalizeUrl(url4) { return url4.replace(/\.git$/, "").toLowerCase(); } function validatePushDestination(params) { - const dest = getPushDestination(params.branch); + const dest = getPushDestination(params.branch, params.storedDest); if (normalizeUrl(dest.url) !== normalizeUrl(params.pushUrl)) { throw new Error( `Push blocked: destination does not match expected repository. @@ -142989,7 +143007,11 @@ function PushBranchTool(ctx) { if (!pushUrl) { throw new Error("pushUrl not set - setupGit must run before push_branch"); } - const pushDest = validatePushDestination({ branch, pushUrl }); + const pushDest = validatePushDestination({ + branch, + pushUrl, + storedDest: ctx.toolState.pushDest + }); if (pushPermission === "restricted" && pushDest.remoteBranch === defaultBranch) { throw new Error( `Push blocked: cannot push directly to default branch '${pushDest.remoteBranch}'. Create a feature branch and open a PR instead.` @@ -144069,10 +144091,10 @@ function UploadFileTool(ctx) { // mcp/server.ts function initToolState(params) { - const progressCommentId = params.progressCommentId ? parseInt(params.progressCommentId, 10) : null; - const resolvedId = Number.isNaN(progressCommentId) ? null : progressCommentId; - if (progressCommentId) { - log.info(`\xBB using pre-created progress comment: ${progressCommentId}`); + const parsed2 = params.progressCommentId ? parseInt(params.progressCommentId, 10) : NaN; + const resolvedId = Number.isNaN(parsed2) ? void 0 : parsed2; + if (resolvedId) { + log.info(`\xBB using pre-created progress comment: ${resolvedId}`); } return { progressCommentId: resolvedId, diff --git a/mcp/checkout.ts b/mcp/checkout.ts index abdac32..08c89a4 100644 --- a/mcp/checkout.ts +++ b/mcp/checkout.ts @@ -290,6 +290,15 @@ export async function checkoutPrBranch( toolState.pushUrl = `https://github.com/${headRepo.full_name}.git`; } + // store push destination so push_branch can use it directly + // git config is the primary mechanism, but toolState serves as a reliable fallback + // in case git config reads fail in certain environments + toolState.pushDest = { + remoteName: isFork ? `pr-${pullNumber}` : "origin", + remoteBranch: headBranch, + localBranch, + }; + // execute post-checkout lifecycle hook await executeLifecycleHook({ event: "post-checkout", diff --git a/mcp/comment.ts b/mcp/comment.ts index 7d81e1d..6095255 100644 --- a/mcp/comment.ts +++ b/mcp/comment.ts @@ -148,10 +148,14 @@ export const ReportProgress = type({ }); /** - * Standalone function to report progress to GitHub comment. - * Can be called directly without going through the MCP tool interface. - * Returns result data if successful. - * When there's no comment target (no progressCommentId and no issueNumber), returns a "skipped" result. + * Report progress to a GitHub comment. + * + * progressCommentId has three states: + * - undefined: no comment yet — will create one if an issue/PR target exists + * - number: active comment — will update it in place + * - null: deliberately deleted (e.g. after submitting a PR review) — skips silently + * + * The body is always tracked in lastProgressBody for the job summary regardless of comment state. */ export async function reportProgress( ctx: ToolContext, @@ -201,6 +205,11 @@ export async function reportProgress( }; } + // null = progress comment was deliberately deleted (e.g. by create_pull_request_review) + if (existingCommentId === null) { + return { body, action: "skipped" }; + } + // no existing comment - need an issue/PR to create one on // use fallback chain: dynamically set context > event payload if (issueNumber === undefined) { @@ -288,6 +297,8 @@ export function ReportProgressTool(ctx: ToolContext) { /** * Delete the progress comment if it exists. * Used after submitting a PR review since the review body contains all necessary info. + * Sets progressCommentId to null, which prevents future report_progress calls from + * creating a new comment (the agent may call report_progress again after this). */ export async function deleteProgressComment(ctx: ToolContext): Promise { const existingCommentId = ctx.toolState.progressCommentId; @@ -310,7 +321,7 @@ export async function deleteProgressComment(ctx: ToolContext): Promise } } - // reset state and mark as updated so post script doesn't try to handle it + // set to null (not undefined) so report_progress skips instead of creating a new comment ctx.toolState.progressCommentId = null; ctx.toolState.wasUpdated = true; diff --git a/mcp/delegate.ts b/mcp/delegate.ts index b67f91b..233d752 100644 --- a/mcp/delegate.ts +++ b/mcp/delegate.ts @@ -12,7 +12,7 @@ export const DelegateParams = type({ "the name of the mode to delegate to (e.g., 'Build', 'Plan', 'Review', 'Fix', 'AddressReviews')" ), "effort?": Effort.describe( - 'effort level for the subagent: "mini" (fast), "auto" (default, highly capable), or "max" (maximum capability)' + 'effort level for the subagent: "mini" (low-effort and fast, only for simple tasks), "auto" (medium-effort, good for typical tasks that don\'t require significant reasoning), or "max" (high-effort, good for PR reviews and complex coding tasks)' ), "instructions?": type.string.describe( "optional additional context or instructions for the subagent — use this to pass results from earlier delegations or narrow the subagent's focus" diff --git a/mcp/git.ts b/mcp/git.ts index 4a02058..fd9bc6c 100644 --- a/mcp/git.ts +++ b/mcp/git.ts @@ -3,7 +3,7 @@ import { type } from "arktype"; import { log } from "../utils/cli.ts"; import { $git } from "../utils/gitAuth.ts"; import { $ } from "../utils/shell.ts"; -import type { ToolContext } from "./server.ts"; +import type { StoredPushDest, ToolContext } from "./server.ts"; import { execute, tool } from "./shared.ts"; type PushDestination = { @@ -14,17 +14,27 @@ type PushDestination = { /** * get where git would actually push this branch. - * reads branch.X.pushRemote and branch.X.merge set by checkout_pr. + * prefers the stored destination from toolState (set by checkout_pr) when it + * matches the current branch, because git config reads can silently fail in + * certain environments causing pushes to the wrong remote branch. * - * NOTE: we read git config directly instead of using @{push} because - * push.default=simple (git's default) requires local/remote branch names - * to match. since checkout_pr uses pr-N as the local name, @{push} resolves - * to origin/pr-N instead of the actual remote branch. - * - * for branches created via checkout_pr: uses configured pushRemote/merge - * for new branches (git checkout -b): falls back to origin/ + * falls back to reading branch.X.pushRemote and branch.X.merge from git config, + * and finally to origin/ for branches created without checkout_pr. */ -function getPushDestination(branch: string): PushDestination { +function getPushDestination( + branch: string, + storedDest: StoredPushDest | undefined +): PushDestination { + // prefer stored destination from checkout_pr when it matches the current branch + if (storedDest && storedDest.localBranch === branch) { + log.debug(`using stored push destination: ${storedDest.remoteName}/${storedDest.remoteBranch}`); + const url = $("git", ["remote", "get-url", "--push", storedDest.remoteName], { + log: false, + }).trim(); + return { remoteName: storedDest.remoteName, remoteBranch: storedDest.remoteBranch, url }; + } + + // fall back to git config (for branches not created by checkout_pr) try { const pushRemote = $("git", ["config", `branch.${branch}.pushRemote`], { log: false }).trim(); const merge = $("git", ["config", `branch.${branch}.merge`], { log: false }).trim(); @@ -49,6 +59,7 @@ function normalizeUrl(url: string): string { type ValidatePushParams = { branch: string; pushUrl: string; + storedDest: StoredPushDest | undefined; }; /** @@ -56,7 +67,7 @@ type ValidatePushParams = { * pushUrl is set by setupGit (base repo) and updated by checkout_pr (fork repo). */ function validatePushDestination(params: ValidatePushParams): PushDestination { - const dest = getPushDestination(params.branch); + const dest = getPushDestination(params.branch, params.storedDest); if (normalizeUrl(dest.url) !== normalizeUrl(params.pushUrl)) { throw new Error( @@ -102,7 +113,11 @@ export function PushBranchTool(ctx: ToolContext) { if (!pushUrl) { throw new Error("pushUrl not set - setupGit must run before push_branch"); } - const pushDest = validatePushDestination({ branch, pushUrl }); + const pushDest = validatePushDestination({ + branch, + pushUrl, + storedDest: ctx.toolState.pushDest, + }); // block pushes to default branch in restricted mode if (pushPermission === "restricted" && pushDest.remoteBranch === defaultBranch) { diff --git a/mcp/server.ts b/mcp/server.ts index 095a866..1004066 100644 --- a/mcp/server.ts +++ b/mcp/server.ts @@ -15,10 +15,19 @@ export type BackgroundProcess = { pidPath: string; }; +export type StoredPushDest = { + remoteName: string; + remoteBranch: string; + localBranch: string; +}; + export interface ToolState { // where we're allowed to push - base repo initially, fork URL for fork PRs // set by setupGit, updated by checkout_pr. always set before push validation. pushUrl?: string; + // push destination set by checkout_pr - used as primary source in push_branch + // because git config reads can fail in certain environments + pushDest?: StoredPushDest; // issue or PR number (same number space in GitHub) issueNumber?: number; selectedMode?: string; @@ -34,7 +43,8 @@ export interface ToolState { promise: Promise | undefined; results: PrepResult[] | undefined; }; - progressCommentId: number | null; + // undefined = no comment yet, number = active comment, null = deliberately deleted + progressCommentId: number | null | undefined; lastProgressBody?: string; wasUpdated?: boolean; output?: string; @@ -45,13 +55,11 @@ interface InitToolStateParams { } export function initToolState(params: InitToolStateParams): ToolState { - const progressCommentId = params.progressCommentId - ? parseInt(params.progressCommentId, 10) - : null; - const resolvedId = Number.isNaN(progressCommentId) ? null : progressCommentId; + const parsed = params.progressCommentId ? parseInt(params.progressCommentId, 10) : NaN; + const resolvedId = Number.isNaN(parsed) ? undefined : parsed; - if (progressCommentId) { - log.info(`» using pre-created progress comment: ${progressCommentId}`); + if (resolvedId) { + log.info(`» using pre-created progress comment: ${resolvedId}`); } return { diff --git a/utils/instructions.ts b/utils/instructions.ts index 655db96..c65d794 100644 --- a/utils/instructions.ts +++ b/utils/instructions.ts @@ -346,7 +346,10 @@ export function resolveInstructions(ctx: InstructionsContext): ResolvedInstructi Call \`delegate\` with a mode, effort level, and optional instructions: - \`mode\`: The workflow to run (see available modes below) -- \`effort\`: \`"auto"\` (default, most capable), \`"mini"\` (fast, for simple tasks), or \`"max"\` (maximum capability) +- \`effort\`: + - \`"mini"\`: low-effort and fast, for simple tasks + - \`"auto"\`: medium-effort, good for typical tasks that don't require significant reasoning + - \`"max"\`: high-effort, good for PR reviews and complex coding tasks. - \`instructions\`: Optional additional context for the subagent. Use this to pass results from earlier delegations or narrow the subagent's focus. ### Single vs. multi-phase delegation