Files
shockbot/mcp/comment.ts
T
Colin McDonnell f3d18401ac eager signup credit + free-OpenCode fallback when BYOK has no key (#789)
* eager signup credit + free-OpenCode fallback when BYOK has no key

addresses the silent-churn pattern that took out 15 first-run-failure
accounts post-launch: GH Actions secret references resolved to empty
strings (because the secrets didn't exist on the repo), the action
launched Claude Code with no key, the LLM provider 401'd, and the run
died in seconds with a synthetic "Invalid API key" message. those
accounts had no Router credits to fall back to because the lazy claim
required a dashboard visit they never made.

three changes, one PR:

1. Eager $10 signup credit at account creation. Both account-creation
   sites (`upsertAccountByClerkId` for dashboard signin, `fetchOrCreateRepo`
   for CLI / GH-App-only) now insert the `CreditGrant { reason: "signup" }`
   in the same transaction as the `accounts` row. CLI installers who
   never sign in get the credit. The dashboard `/signup-credit/claim`
   POST stays as an idempotent backstop for accounts created before
   this shipped.

2. Free-OpenCode fallback in the action. When the configured BYOK slug
   needs a provider key the runner doesn't have, swap to
   `opencode/minimax-m2.5-free` before agent selection so the run still
   succeeds. Surfaced via a `» fell back from <slug> to <free>` warning
   in the action log. Skipped on Router runs (Pullfrog mints the key)
   and when no model is configured (auto-select-with-throw still fires
   for the genuinely-misconfigured case).

3. New action-test fixture `byok-no-keys-fallback` that empty-strings
   every known provider key (matching how GH Actions handles missing
   secrets) and asserts the run succeeds with the fallback log line
   present. plus a unit test for the helper covering each skip case.

skipping the schema flip from `byok` to `router` — that's coming via
the onboarding-stepper PR (#762).

* fallback: skip Bedrock + surface in PR-comment footer

addresses copilot review on #789 (real bug — parseModel throws on
Bedrock raw IDs that have no slash, would crash before
validateBedrockSetup could surface its own error) and the user-side
ask to make the fallback visible in PR comments.

- selectFallbackModelIfNeeded skips when resolvedModel has no '/' so
  Bedrock routing IDs (e.g. us.anthropic.claude-opus-4-7) don't crash
  inside hasProviderKey -> parseModel. unit test covers it.
- toolState.modelFallback records the configured slug we fell back
  from. set in main.ts when fallback engages.
- buildPullfrogFooter accepts fallbackFrom and renders
  "Using `MiniMax M2.5` (free) (credentials for Claude Opus not
  configured)" so the substitution is visible in PR comments,
  reviews, PR bodies, and error reports.
- threaded through all four action-side footer call sites
  (mcp/comment, mcp/pr, mcp/review, utils/errorReport). server-side
  call sites in triggerWorkflow.ts / handleWorkflowRunWebhook.ts
  fire pre-action and don't have toolState — left as-is.

* fallback footer: use provider display name + document email asymmetry

addresses pullfrog reviewer findings on #789:

- footer now shows 'credentials for Anthropic not configured' (provider
  display name from `providers.anthropic.displayName`) instead of the
  per-model name. credentials are provider-scoped (ANTHROPIC_API_KEY
  covers all Anthropic models), so this matches what the user actually
  needs to fix.

- document the intentional asymmetry between eager and lazy signup
  credit paths: eager skips both the signupCreditClaimedEmail and the
  per-grant team@ alert. comment explains why (the 'new account
  created' alert already covers it on the eager path; the user-facing
  email assumes a user-initiated action that hasn't happened yet for
  CLI/GH-App-only signups).

- skipping the backfill for the 15 historical accounts per user's
  earlier decision — they all uninstalled, so the cohort self-selected
  out of being reachable.

* fallback: gate on resolvedModel + skip resolveModel re-resolve post-swap

local agnostic fixture run surfaced two real bugs the unit tests didn't
catch:

1. fallback gate was on configuredSlug (=payload.model) but the test
   uses PULLFROG_MODEL to set the model, which is read by resolveModel
   AFTER its slug arg. configuredSlug stayed undefined → fallback never
   fired. drop configuredSlug from the helper signature; gate purely on
   resolvedModel since that's the same value regardless of how the
   model was specified (DB config vs PULLFROG_MODEL env).

2. when fallback engaged, the post-swap resolveModel({slug: fallback.to})
   call was ALSO honoring PULLFROG_MODEL, re-overriding the fallback
   target back to the unkeyed model. validateAgentApiKey then threw
   "no API key found" against the original model. fix: skip the
   re-resolve. fallback.to is already a CLI-ready specifier.

unit tests updated for the new helper signature (8 tests, all pass).
fallback log line confirmed emitted in the local run pre-second-fix;
the second fix unblocks the validation that previously threw.
2026-05-20 02:17:22 +00:00

508 lines
18 KiB
TypeScript

import { type } from "arktype";
import { getApiUrl } from "../utils/apiUrl.ts";
import { buildPullfrogFooter, stripExistingFooter } from "../utils/buildPullfrogFooter.ts";
import { log } from "../utils/cli.ts";
import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
import { patchWorkflowRunFields } from "../utils/patchWorkflowRunFields.ts";
import {
createLeapingProgressComment,
deleteProgressCommentApi,
updateProgressComment,
} from "../utils/progressComment.ts";
import type { ToolContext } from "./server.ts";
import { execute, tool } from "./shared.ts";
// re-export for backward compat with anything importing the leaping helpers from mcp/comment
export {
isLeapingIntoActionCommentBody,
LEAPING_INTO_ACTION_PREFIX,
} from "../utils/leapingComment.ts";
function buildCommentFooter(ctx: ToolContext, customParts?: string[]): string {
const runId = ctx.runId;
return buildPullfrogFooter({
triggeredBy: true,
workflowRun:
runId !== undefined
? {
owner: ctx.repo.owner,
repo: ctx.repo.name,
runId,
jobId: ctx.jobId,
}
: undefined,
customParts,
model: ctx.toolState.model,
fallbackFrom: ctx.toolState.modelFallback?.from,
});
}
function buildImplementPlanLink(ctx: ToolContext, issueNumber: number, commentId: number): string {
const apiUrl = getApiUrl();
return `[Implement plan ➔](${apiUrl}/trigger/${ctx.repo.owner}/${ctx.repo.name}/${issueNumber}?action=implement&comment_id=${commentId})`;
}
export function addFooter(ctx: ToolContext, body: string): string {
if (/<br\s*\/?>[ \t]*\n(?!\s*\n)/i.test(body)) {
throw new Error(
"body contains <br/> followed by a non-blank line, which breaks GitHub markdown rendering. always add a blank line after <br/> tags."
);
}
const bodyWithoutFooter = stripExistingFooter(fixDoubleEscapedString(body));
const footer = buildCommentFooter(ctx);
return `${bodyWithoutFooter}${footer}`;
}
export const Comment = type({
issueNumber: type.number.describe("the issue number to comment on"),
body: type.string.describe("the comment body content"),
type: type
.enumerated("Plan", "Comment")
.describe("Plan: record as the plan for this run. Comment: regular comment (default).")
.optional(),
});
export function CreateCommentTool(ctx: ToolContext) {
return tool({
name: "create_issue_comment",
description:
"Create a comment on a GitHub issue or PR. " +
'Example: `create_issue_comment({ issueNumber: 1234, body: "Thanks for the report." })`. ' +
"For progress/plan updates on the current run use report_progress instead. Use type: 'Plan' for plan comments.",
parameters: Comment,
execute: execute(async ({ issueNumber, body, type: commentType }) => {
const bodyWithFooter = addFooter(ctx, body);
const result = await ctx.octokit.rest.issues.createComment({
owner: ctx.repo.owner,
repo: ctx.repo.name,
issue_number: issueNumber,
body: bodyWithFooter,
});
ctx.toolState.wasUpdated = true;
log.info(`» created comment ${result.data.id}`);
if (commentType === "Plan") {
if (result.data.node_id) {
await patchWorkflowRunFields(ctx, { planCommentNodeId: result.data.node_id });
}
// add "Implement plan" link (needs comment ID, so create-then-update)
const customParts = [buildImplementPlanLink(ctx, issueNumber, result.data.id)];
const footer = buildCommentFooter(ctx, customParts);
const bodyWithPlanLink = `${stripExistingFooter(body)}${footer}`;
const updateResult = await ctx.octokit.rest.issues.updateComment({
owner: ctx.repo.owner,
repo: ctx.repo.name,
comment_id: result.data.id,
body: bodyWithPlanLink,
});
log.info(`» updated comment ${updateResult.data.id}`);
return {
success: true,
commentId: updateResult.data.id,
url: updateResult.data.html_url,
body: updateResult.data.body,
};
}
return {
success: true,
commentId: result.data.id,
url: result.data.html_url,
body: result.data.body,
};
}),
});
}
export const EditComment = type({
commentId: type.number.describe("the ID of the comment to edit"),
body: type.string.describe("the new comment body content"),
});
export function EditCommentTool(ctx: ToolContext) {
return tool({
name: "edit_issue_comment",
description: "Edit a GitHub issue comment by its ID",
parameters: EditComment,
execute: execute(async ({ commentId, body }) => {
const bodyWithFooter = addFooter(ctx, body);
const result = await ctx.octokit.rest.issues.updateComment({
owner: ctx.repo.owner,
repo: ctx.repo.name,
comment_id: commentId,
body: bodyWithFooter,
});
log.info(`» updated comment ${result.data.id}`);
return {
success: true,
commentId: result.data.id,
url: result.data.html_url,
body: result.data.body,
updatedAt: result.data.updated_at,
};
}),
});
}
export const ReportProgress = type({
body: type.string.describe("the progress update content to share"),
"target_plan_comment?": type("boolean").describe(
"when true, update the existing plan comment (from select_mode lookup) instead of the progress comment; use when editing an existing plan"
),
});
/**
* Report progress to a GitHub comment.
*
* progressComment has three states:
* - undefined: no comment yet — will create one if an issue/PR target exists
* - object: active comment — will update it in place via the right REST endpoint for its type
* - 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.
*
* The "existing plan comment" path always targets a top-level issue comment (plan comments are
* created by create_issue_comment with type:"Plan", never as review-thread replies).
*/
export async function reportProgress(
ctx: ToolContext,
params: { body: string; target_plan_comment?: boolean }
): Promise<{
commentId?: number;
url?: string;
body: string;
action: "created" | "updated" | "skipped";
}> {
const { body, target_plan_comment } = params;
// always track the body for job summary
ctx.toolState.lastProgressBody = body;
// silent events (e.g., auto-label, pr-summary Task) should never create or update progress comments.
// the body is still tracked above for the GitHub Actions job summary.
if (ctx.payload.event.silent) {
return { body, action: "skipped" };
}
const issueNumber = ctx.payload.event.issue_number ?? ctx.toolState.issueNumber;
const isPlanMode = ctx.toolState.selectedMode === "Plan";
const apiCtx = { octokit: ctx.octokit, owner: ctx.repo.owner, repo: ctx.repo.name };
// when editing existing plan: update the plan comment from tool state (set by select_mode)
if (target_plan_comment === true && ctx.toolState.existingPlanCommentId === undefined) {
log.warning("target_plan_comment requested but no existingPlanCommentId in tool state");
}
if (target_plan_comment === true && ctx.toolState.existingPlanCommentId !== undefined) {
const commentId = ctx.toolState.existingPlanCommentId;
const customParts =
issueNumber !== undefined ? [buildImplementPlanLink(ctx, issueNumber, commentId)] : undefined;
const bodyWithoutFooter = stripExistingFooter(body);
const footer = buildCommentFooter(ctx, customParts);
const bodyWithFooter = `${bodyWithoutFooter}${footer}`;
const result = await updateProgressComment(
apiCtx,
{ id: commentId, type: "issue" },
bodyWithFooter
);
ctx.toolState.wasUpdated = true;
if (isPlanMode && result.node_id) {
await patchWorkflowRunFields(ctx, { planCommentNodeId: result.node_id });
}
return {
commentId: result.id,
url: result.html_url,
body: result.body || "",
action: "updated",
};
}
const existingComment = ctx.toolState.progressComment;
// if we already have a progress comment, update it
if (existingComment) {
const customParts =
isPlanMode && issueNumber !== undefined
? [buildImplementPlanLink(ctx, issueNumber, existingComment.id)]
: undefined;
const bodyWithoutFooter = stripExistingFooter(body);
const footer = buildCommentFooter(ctx, customParts);
const bodyWithFooter = `${bodyWithoutFooter}${footer}`;
const result = await updateProgressComment(apiCtx, existingComment, bodyWithFooter);
ctx.toolState.wasUpdated = true;
if (isPlanMode && result.node_id) {
await patchWorkflowRunFields(ctx, { planCommentNodeId: result.node_id });
}
return {
commentId: result.id,
url: result.html_url,
body: result.body || "",
action: "updated",
};
}
// null = progress comment was deleted by stranded-comment cleanup in main.ts
if (existingComment === 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) {
// no-op: no comment target (e.g., workflow_dispatch events)
// body is already tracked for job summary
return { body, action: "skipped" };
}
// for new comments, we need to create first, then update with Plan link if in Plan mode
// self-created progress comments are always top-level issue comments — review-reply
// progress comments only originate from the dispatch path and arrive pre-created.
const initialBody = addFooter(ctx, body);
const created = await createLeapingProgressComment(
apiCtx,
{ kind: "issue", issueNumber },
initialBody
);
ctx.toolState.progressComment = created.comment;
ctx.toolState.wasUpdated = true;
// if Plan mode, update the comment to add the "Implement plan" link
if (isPlanMode) {
const customParts = [buildImplementPlanLink(ctx, issueNumber, created.comment.id)];
const bodyWithoutFooter = stripExistingFooter(body);
const footer = buildCommentFooter(ctx, customParts);
const bodyWithPlanLink = `${bodyWithoutFooter}${footer}`;
const updateResult = await updateProgressComment(apiCtx, created.comment, bodyWithPlanLink);
if (updateResult.node_id) {
await patchWorkflowRunFields(ctx, { planCommentNodeId: updateResult.node_id });
}
return {
commentId: updateResult.id,
url: updateResult.html_url,
body: updateResult.body || "",
action: "created",
};
}
return {
commentId: created.comment.id,
url: created.html_url,
body: created.body || "",
action: "created",
};
}
export function ReportProgressTool(ctx: ToolContext) {
return tool({
name: "report_progress",
description:
"Share progress on the associated GitHub issue/PR. The first call creates a comment; subsequent calls update it in place. " +
'Example: `report_progress({ body: "Implemented the auth check and added tests." })`. ' +
"Call this at the end of every run with a brief final summary (1-3 sentences) unless the mode guidance instructs otherwise. The current task list is automatically appended in a collapsible section — do not restate individual steps.",
parameters: ReportProgress,
execute: execute(async (params) => {
let body = params.body;
// for non-plan calls: stop auto-updates, wait for in-flight writes to settle,
// then append completed task list collapsible
if (!params.target_plan_comment && ctx.toolState.todoTracker) {
ctx.toolState.todoTracker.cancel();
await ctx.toolState.todoTracker.settled();
const collapsible = ctx.toolState.todoTracker.renderCollapsible({
completeInProgress: true,
});
if (collapsible) {
body = `${body}\n\n${collapsible}`;
}
}
const reportParams: { body: string; target_plan_comment?: boolean } = { body };
if (params.target_plan_comment !== undefined) {
reportParams.target_plan_comment = params.target_plan_comment;
}
const result = await reportProgress(ctx, reportParams);
if (result.action === "skipped") {
return {
success: true,
message:
"progress recorded (no GitHub comment created - this may occur for workflow_dispatch events or when there is no associated issue/PR)",
};
}
if (result.commentId !== undefined) {
log.info(`» ${result.action} comment ${result.commentId}`);
}
if (!params.target_plan_comment) {
ctx.toolState.finalSummaryWritten = true;
}
return {
success: true,
...result,
};
}),
});
}
/**
* Delete the progress comment if it exists.
* Used by main.ts for stranded-comment cleanup (orphaned "Leaping into action" or
* checklist left by the todo tracker when the agent didn't call report_progress).
* Sets progressComment to null so subsequent report_progress calls are no-ops.
*/
export async function deleteProgressComment(ctx: ToolContext): Promise<boolean> {
const existing = ctx.toolState.progressComment;
if (!existing) {
return false;
}
try {
await deleteProgressCommentApi(
{ octokit: ctx.octokit, owner: ctx.repo.owner, repo: ctx.repo.name },
existing
);
} catch (error) {
// ignore 404 - comment already deleted
if (error instanceof Error && error.message.includes("Not Found")) {
// comment already deleted, continue
} else {
throw error;
}
}
// set to null (not undefined) so report_progress skips instead of creating a new comment
ctx.toolState.progressComment = null;
return true;
}
export const ReplyToReviewComment = type({
pull_number: type.number.describe("the pull request number"),
comment_id: type.number.describe("the ID of the review comment to reply to"),
body: type.string.describe(
"extremely brief reply (1 sentence max) explaining what was fixed, e.g. 'Fixed by renaming to X' or 'Added null check'"
),
});
/**
* decision returned by `duplicateReplyDecision` when a session has already
* posted an identical reply to the same parent review comment.
*/
export interface DuplicateReplyDecision {
kind: "already-replied";
commentId: number;
url: string | undefined;
reason: string;
}
/**
* decide whether a second reply_to_review_comment call in the same session
* is a duplicate of an earlier reply to the same parent comment.
*
* the agent is instructed to call reply_to_review_comment exactly once per
* parent comment per AddressReviews session, but in practice it sometimes
* emits the same call twice. PR #610 reproduced this with Kimi K2:
* identical body posted 3 seconds apart, only one tool_use event in the
* agent log. the second post is always redundant and clutters the PR thread.
*
* we key on (comment_id, bodyWithFooter) so a legitimate follow-up reply
* with different content still goes through. within a single run the
* footer is constant (workflow run + model + jobId), so byte-equal bodies
* catch the stutter without blocking real follow-ups.
*
* mirrors the shape of `duplicateReviewDecision` in mcp/review.ts.
*/
export function duplicateReplyDecision(params: {
existing: { commentId: number; url: string | undefined; bodyWithFooter: string } | undefined;
bodyWithFooter: string;
}): DuplicateReplyDecision | null {
const existing = params.existing;
if (!existing) return null;
if (existing.bodyWithFooter !== params.bodyWithFooter) return null;
return {
kind: "already-replied",
commentId: existing.commentId,
url: existing.url,
reason: `reply ${existing.commentId} with identical body was already posted in this session; ignoring duplicate call`,
};
}
export function ReplyToReviewCommentTool(ctx: ToolContext) {
return tool({
name: "reply_to_review_comment",
description:
"Reply to a PR review comment thread (NOT issue comments — this only works for inline review comments on PR diffs). " +
'Example: `reply_to_review_comment({ pull_number: 1234, comment_id: 567890, body: "Fixed by adding a null check." })`. ' +
"Call exactly ONCE per parent comment you address in AddressReviews mode — duplicate calls with the same body are a no-op. Keep replies extremely brief (1 sentence max).",
parameters: ReplyToReviewComment,
execute: execute(async ({ pull_number, comment_id, body }) => {
const bodyWithFooter = addFooter(ctx, body);
// guard against duplicate reply submissions in the same session.
// see duplicateReplyDecision for the rationale.
const dup = duplicateReplyDecision({
existing: ctx.toolState.reviewReplies?.get(comment_id),
bodyWithFooter,
});
if (dup) {
log.info(`skipping duplicate review reply: ${dup.reason}`);
return {
success: true,
skipped: true,
reason: dup.reason,
commentId: dup.commentId,
url: dup.url,
};
}
const result = await ctx.octokit.rest.pulls.createReplyForReviewComment({
owner: ctx.repo.owner,
repo: ctx.repo.name,
pull_number,
comment_id,
body: bodyWithFooter,
});
log.info(`» created review comment ${result.data.id} (in reply to ${comment_id})`);
// mark progress as updated so error reporting + run-result handling know
// a substantive write happened (used by reportErrorToComment / handleAgentResult)
ctx.toolState.wasUpdated = true;
// record this reply for in-session dedupe of subsequent identical calls.
ctx.toolState.reviewReplies ??= new Map();
ctx.toolState.reviewReplies.set(comment_id, {
commentId: result.data.id,
url: result.data.html_url,
bodyWithFooter,
});
return {
success: true,
commentId: result.data.id,
url: result.data.html_url,
body: result.data.body,
in_reply_to_id: result.data.in_reply_to_id,
};
}, "reply_to_review_comment"),
});
}