Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions src/mcp/local-write-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,91 @@ export function buildFileIssueSpec(input: { repoFullName: string; title: string;
return spec("file_issue", "File a new issue.", { repoFullName: input.repoFullName, title: input.title, body: input.body, labels }, command);
}

export type DeferredReviewFinding = {
title: string;
detail: string;
path?: string | undefined;
action?: string | undefined;
};

const FOLLOW_UP_ISSUE_TITLE_MAX = 120;
const FOLLOW_UP_ISSUE_BODY_MAX = 4000;

function stripFollowUpMarkers(value: string): string {
return value.replace(/<!--[\s\S]*?-->/g, "").replace(/\r\n/g, "\n").trim();
}

function boundFollowUpLine(value: string, max: number): string {
const cleaned = stripFollowUpMarkers(value).replace(/\s+/g, " ").trim();
if (cleaned.length <= max) return cleaned;
return `${cleaned.slice(0, Math.max(0, max - 1)).trimEnd()}…`;
}

function boundFollowUpBody(value: string, max: number): string {
const cleaned = stripFollowUpMarkers(value).trim();
if (cleaned.length <= max) return cleaned;
return `${cleaned.slice(0, Math.max(0, max - 1)).trimEnd()}…`;
}

function composeFollowUpIssueTitle(finding: DeferredReviewFinding): string {
const cleaned = stripFollowUpMarkers(finding.title);
if (/^follow-up:/i.test(cleaned)) {
return boundFollowUpLine(cleaned, FOLLOW_UP_ISSUE_TITLE_MAX);
}
const prefix = "Follow-up: ";
return `${prefix}${boundFollowUpLine(cleaned, FOLLOW_UP_ISSUE_TITLE_MAX - prefix.length)}`;
}

function composeFollowUpIssueBody(input: { finding: DeferredReviewFinding; pullNumber?: number | undefined }): string {
const lines: string[] = [];
if (input.pullNumber !== undefined) lines.push(`Deferred from review on PR #${input.pullNumber}.`);
if (input.finding.path) lines.push(`File: \`${input.finding.path}\``);
lines.push("", boundFollowUpLine(input.finding.detail, FOLLOW_UP_ISSUE_BODY_MAX));
if (input.finding.action) {
lines.push("", "**Suggested next step**", boundFollowUpLine(input.finding.action, 500));
}
lines.push("", "_Filed locally from a deferred review finding — gittensory supplies content only._");
return boundFollowUpBody(lines.join("\n"), FOLLOW_UP_ISSUE_BODY_MAX);
}

function sanitizeFollowUpFinding(finding: DeferredReviewFinding): Record<string, string> {
const sanitized: Record<string, string> = {
title: stripFollowUpMarkers(finding.title),
detail: stripFollowUpMarkers(finding.detail),
};
if (finding.path) sanitized.path = finding.path;
if (finding.action) sanitized.action = stripFollowUpMarkers(finding.action);
return sanitized;
}

/** File a follow-up issue for a deferred review finding (#2177, #1962 slice). */
export function buildFollowUpIssueSpec(input: {
repoFullName: string;
finding: DeferredReviewFinding;
labels?: string[] | undefined;
pullNumber?: number | undefined;
}): LocalWriteActionSpec {
const sanitizedFinding = sanitizeFollowUpFinding(input.finding);
const title = composeFollowUpIssueTitle(input.finding);
const body = composeFollowUpIssueBody({ finding: input.finding, pullNumber: input.pullNumber });
const fileSpec = buildFileIssueSpec({
repoFullName: input.repoFullName,
title,
body,
labels: input.labels,
});
return {
...fileSpec,
action: "follow_up_issue",
description: `File a follow-up issue for a deferred review finding: ${title}`,
inputs: {
...fileSpec.inputs,
finding: sanitizedFinding,
...(input.pullNumber !== undefined ? { pullNumber: input.pullNumber } : {}),
},
};
}

/** Add labels to an issue or PR (gh issue edit also targets PRs). */
export function buildApplyLabelsSpec(input: { repoFullName: string; number: number; labels: string[] }): LocalWriteActionSpec {
const labelArgs = input.labels.map((label) => ` --add-label ${sq(label)}`).join("");
Expand Down
71 changes: 71 additions & 0 deletions test/unit/local-write-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
buildCreateBranchSpec,
buildDeleteBranchSpec,
buildFileIssueSpec,
buildFollowUpIssueSpec,
buildOpenPrSpec,
buildPostEligibilityCommentSpec,
buildTestGenSpec,
Expand Down Expand Up @@ -92,3 +93,73 @@ describe("buildTestGenSpec (#2188)", () => {
expect(s.command).toContain("it'\\''s edge case");
});
});

// #2177 (follow-up-issue action spec slice of #1962).
describe("buildFollowUpIssueSpec (#2177)", () => {
it("builds a follow_up_issue spec with composed title/body, labels, and the local-execution boundary", () => {
const s = buildFollowUpIssueSpec({
repoFullName: "o/r",
pullNumber: 42,
labels: ["gittensor:bug"],
finding: {
title: "Handle null branch in widget loader",
detail: "The loader never guards a null response.",
path: "src/widget.ts",
action: "Add a regression test for the null path.",
},
});
expect(s.action).toBe("follow_up_issue");
expect(s.boundary).toBe(LOCAL_WRITE_BOUNDARY);
expect(s.description).toContain("Follow-up: Handle null branch in widget loader");
expect(s.command).toBe(
"gh issue create --repo 'o/r' --title 'Follow-up: Handle null branch in widget loader' --body 'Deferred from review on PR #42.\nFile: `src/widget.ts`\n\nThe loader never guards a null response.\n\n**Suggested next step**\nAdd a regression test for the null path.\n\n_Filed locally from a deferred review finding — gittensory supplies content only._' --label 'gittensor:bug'",
);
expect(s.inputs).toMatchObject({ labels: ["gittensor:bug"], pullNumber: 42 });
expect(s.inputs.finding).toEqual({
title: "Handle null branch in widget loader",
detail: "The loader never guards a null response.",
path: "src/widget.ts",
action: "Add a regression test for the null path.",
});
});

it("omits labels and optional finding fields when absent, and strips HTML comment markers from the finding text", () => {
const s = buildFollowUpIssueSpec({
repoFullName: "o/r",
finding: {
title: "<!-- marker -->Follow-up: it's noisy",
detail: "Detail <!-- hidden --> stays public-safe.",
},
});
expect(s.command).toContain("--title 'Follow-up: it'\\''s noisy'");
expect(s.command).not.toContain("<!--");
expect(s.command).not.toContain("--label");
expect(s.inputs).toMatchObject({ labels: [] });
expect(s.inputs.finding).toEqual({ title: "Follow-up: it's noisy", detail: "Detail stays public-safe." });
expect(s.inputs).not.toHaveProperty("pullNumber");
expect(s.description).toContain("Follow-up: it's noisy");
});

it("bounds an over-long finding title before delegating to gh issue create", () => {
const longTitle = "x".repeat(140);
const s = buildFollowUpIssueSpec({ repoFullName: "o/r", finding: { title: longTitle, detail: "short detail" } });
const titleMatch = s.command.match(/--title '([^']|'\\'')*'/);
expect(titleMatch).not.toBeNull();
const titleArg = titleMatch![0].replace(/^--title '/, "").replace(/'$/, "").replace(/'\\''/g, "'");
expect(titleArg.startsWith("Follow-up: ")).toBe(true);
expect(titleArg.length).toBeLessThanOrEqual(120);
});

it("preserves an existing Follow-up prefix and bounds an over-long composed body", () => {
const s = buildFollowUpIssueSpec({
repoFullName: "o/r",
finding: {
title: "Follow-up: tighten null handling",
detail: "d".repeat(5000),
},
});
expect(s.description).toContain("Follow-up: tighten null handling");
expect(s.command.length).toBeLessThan(7000);
expect(s.command.endsWith("'")).toBe(true);
});
});