From 821c45b2ecf063432c0d40271e1be368706fb48e Mon Sep 17 00:00:00 2001 From: Michal Date: Fri, 14 Aug 2026 23:57:46 +0100 Subject: [PATCH] fix(cli): stop claiming that accept merges anything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mcpctl tasks accept --help` said "fires the merge". Nothing merges. There is no createPullRequest or mergePullRequest anywhere in the codebase — the git-provider client is task 148 and is not built. Accept records human approval and moves the task to `merged`. Until 148/149 land, `merged` means "a human approved this", not "this is in main", and the help text and service comments now say so. A user who accepted a task expecting a PR to land would have been silently wrong. Co-Authored-By: Claude Opus 5 (1M context) --- completions/mcpctl.fish | 2 +- src/cli/src/commands/tasks.ts | 2 +- src/mcpd/src/services/task.service.ts | 9 ++++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index 9e5d169..9cac1cb 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -371,7 +371,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active repos import" -l ssh -d 'Use ssh c set -l tasks_cmds advance verdict accept reject history complete -c mcpctl -n "__fish_seen_subcommand_from tasks; and not __fish_seen_subcommand_from $tasks_cmds" -a advance -d 'Move a task to the next state' complete -c mcpctl -n "__fish_seen_subcommand_from tasks; and not __fish_seen_subcommand_from $tasks_cmds" -a verdict -d 'Record a QA / Security / SRE verdict, pinned to the commit it judged' -complete -c mcpctl -n "__fish_seen_subcommand_from tasks; and not __fish_seen_subcommand_from $tasks_cmds" -a accept -d 'Accept a task awaiting review — fires the merge' +complete -c mcpctl -n "__fish_seen_subcommand_from tasks; and not __fish_seen_subcommand_from $tasks_cmds" -a accept -d 'Accept a task awaiting review (records the decision; does NOT merge the PR yet)' complete -c mcpctl -n "__fish_seen_subcommand_from tasks; and not __fish_seen_subcommand_from $tasks_cmds" -a reject -d 'Send a task back with direction (the comment is required)' complete -c mcpctl -n "__fish_seen_subcommand_from tasks; and not __fish_seen_subcommand_from $tasks_cmds" -a history -d 'Show the transition log and recorded verdicts' diff --git a/src/cli/src/commands/tasks.ts b/src/cli/src/commands/tasks.ts index ffee69e..469a646 100644 --- a/src/cli/src/commands/tasks.ts +++ b/src/cli/src/commands/tasks.ts @@ -85,7 +85,7 @@ export function createTasksCommand(deps: TasksCommandDeps): Command { }); cmd.command('accept') - .description('Accept a task awaiting review — fires the merge') + .description('Accept a task awaiting review (records the decision; does NOT merge the PR yet)') .argument('', 'Task name or id') .option('--note ', 'Comment stored with the decision') .action(async (task: string, opts) => { diff --git a/src/mcpd/src/services/task.service.ts b/src/mcpd/src/services/task.service.ts index 8fa35f9..7131d75 100644 --- a/src/mcpd/src/services/task.service.ts +++ b/src/mcpd/src/services/task.service.ts @@ -31,7 +31,10 @@ const TRANSITIONS: Record = { security_review: ['sre_review', 'done', 'in_progress', 'cancelled'], sre_review: ['done', 'in_progress', 'cancelled'], done: ['awaiting_review', 'in_progress', 'cancelled'], - // The human gate: accept fires the merge, reject sends it back with a comment. + // The human gate: accept records the decision, reject sends it back with a + // comment. NOTE: accept does not yet merge anything — the git-provider merge + // call is task 148/149. Until then `merged` means "a human approved it", not + // "it is in main". awaiting_review: ['merged', 'in_progress', 'cancelled'], // Post-merge failure reopens; the same agent picks it back up. merged: ['reopened'], @@ -263,6 +266,10 @@ export class TaskService { * Accept: awaiting_review → merged. The caller's comment is stored on the * task, so the decision and its reason live together. * + * Does NOT merge a pull request. No git-provider client is wired yet (148), + * so this records human approval and nothing else. Anything that reads + * `merged` as "landed in main" is currently wrong. + * * This is a task-level yes/no on a resolved outcome, NOT a per-tool approval * — the agent has already acted autonomously up to this point. */